selinux: Improving SELinux read/write performance

Hello.

I would like to propose patch that reduces overhead in read/write by SELinux. I sent RFC in previous thread.
http://lkml.org/lkml/2007/9/6/14
As a result of discussion in previous thread,
quality of code has improved, so I would like to submit patch here.

1. Background
Look at benchmark result below.
lmbench simple read/write(average of 5 run). Big overhead exists especially on SH(SuperH) arch.

1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22

                Base  SELinux  Overhead(%)
Simple read     1.10  1.24     12.3
Simple write    1.02  1.14     14.0

* Base: kernel compiled without SELinux support

2) Result for SH(SH4, SH7751R), kernel 2.6.22

                Base    SELinux   Overhead(%)
Simple read     2.39    5.49      130.5
Simple write    2.07    5.10      146.6

2. About patch
It reduces the selinux overhead on read/write by only revalidating permissions in selinux_file_permission if the task or inode labels have changed or the policy has changed since the open-time check. A new LSM hook, security_dentry_open, is added to capture the necessary state at open time to allow this optimization.

3. Result of benchmark after applying patch
1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22

                Base  SELinux  Overhead(%)
Simple read     1.10  1.13     2.3(Before 12.3)
Simple write    1.02  1.024    0.6(Before 14.0)

* Base: kernel compiled without SELinux support

2) Result for SH(SH4, SH7751R), kernel 2.6.22

                Base    SELinux   Overhead(%)
Simple read     2.39    2.63      10.4(Before 130.5)
Simple write    2.07    2.34      13.1(Before 146.6)

Overhead in read/write is reduced a lot.
This patch adds permission check at open time(in __dentry_open), but open/close performance does not get worse as shown below.

* Lmbench simple open/close
Pentium 4(before patch):

                Base  SELinux  Overhead(%)
open/close      5.97  7.45     24.9

after patch:
open/close 5.97 7.48 25.3

SH(before patch):
Base SELinux Overhead(%)
open/close 32.6 62.8 93.0
after patch:
open/close 32.6 58.7 80.2

Next is a patch for 2.6.22.

It reduces the selinux overhead on read/write by only revalidating permissions in selinux_file_permission if the task or inode labels have changed or the policy has changed since the open-time check. A new LSM hook, security_dentry_open, is added to capture the necessary state at open time to allow this optimization.

Signed-off-by Yuichi Nakamura<[email protected]>

---
 fs/open.c                         |    4 ++
 include/linux/security.h          |   18 ++++++++++++
 security/dummy.c                  |    6 ++++
 security/selinux/avc.c            |    5 +++
 security/selinux/hooks.c          |   53 +++++++++++++++++++++++++++++++++++++-
 security/selinux/include/avc.h    |    2 +
 security/selinux/include/objsec.h |    2 +
 7 files changed, 89 insertions(+), 1 deletion(-)

diff -purN -X linux-2.6.22/Documentation/dontdiff linux-2.6.22.orig/security/selinux/avc.c linux-2.6.22/security/selinux/avc.c

security/selinux/avc.c +5 -0

@@ -913,3 +913,8 @@ int avc_has_perm(u32 ssid, u32 tsid, u16
 	avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata);
 	return rc;
 }
+
+u32 avc_policy_seqno(void)
+{
+	return avc_cache.latest_notif;
+}

diff -purN -X linux-2.6.22/Documentation/dontdiff linux-2.6.22.orig/security/selinux/hooks.c linux-2.6.22/security/selinux/hooks.c

security/selinux/hooks.c +52 -1

@@ -14,6 +14,8 @@
  *                          <[email protected]>
  *  Copyright (C) 2006 Hewlett-Packard Development Company, L.P.
  *                     Paul Moore, <[email protected]>
+ *  Copyright (C) 2007 Hitachi Software Engineering Co., Ltd.
+ *                     Yuichi Nakamura <[email protected]>
  *
  *	This program is free software; you can redistribute it and/or modify
  *	it under the terms of the GNU General Public License version 2,

@@ -2458,7 +2460,7 @@ static int selinux_inode_listsecurity(st
 
 /* file security operations */
 
-static int selinux_file_permission(struct file *file, int mask)
+static int selinux_revalidate_file_permission(struct file *file, int mask)
 {
 	int rc;
 	struct inode *inode = file->f_path.dentry->d_inode;

@@ -2480,6 +2482,25 @@ static int selinux_file_permission(struc
 	return selinux_netlbl_inode_permission(inode, mask);
 }
 
+static int selinux_file_permission(struct file *file, int mask)
+{
+	struct inode *inode = file->f_path.dentry->d_inode;
+	struct task_security_struct *tsec = current->security;
+	struct file_security_struct *fsec = file->f_security;
+	struct inode_security_struct *isec = inode->i_security;
+
+	if (!mask) {
+		/* No permission to check.  Existence test. */
+		return 0;
+	}
+
+	if (tsec->sid == fsec->sid && fsec->isid == isec->sid
+	    && fsec->pseqno == avc_policy_seqno())
+		return selinux_netlbl_inode_permission(inode, mask);
+
+	return selinux_revalidate_file_permission(file, mask);
+}
+
 static int selinux_file_alloc_security(struct file *file)
 {
 	return file_alloc_security(file);

@@ -2715,6 +2736,34 @@ static int selinux_file_receive(struct f
 	return file_has_perm(current, file, file_to_av(file));
 }
 
+static int selinux_dentry_open(struct file *file)
+{
+	struct file_security_struct *fsec;
+	struct inode *inode;
+	struct inode_security_struct *isec;
+	inode = file->f_path.dentry->d_inode;
+	fsec = file->f_security;
+	isec = inode->i_security;
+	/*
+	 * Save inode label and policy sequence number
+	 * at open-time so that selinux_file_permission
+	 * can determine whether revalidation is necessary.
+	 * Task label is already saved in the file security
+	 * struct as its SID.
+	 */
+	fsec->isid = isec->sid;
+	fsec->pseqno = avc_policy_seqno();
+	/*
+	 * Since the inode label or policy seqno may have changed
+	 * between the selinux_inode_permission check and the saving
+	 * of state above, recheck that access is still permitted.
+	 * Otherwise, access might never be revalidated against the
+	 * new inode label or new policy.
+	 * This check is not redundant - do not remove.
+	 */
+	return inode_has_perm(current, inode, file_to_av(file), NULL);
+}
+
 /* task security operations */
 
 static int selinux_task_create(unsigned long clone_flags)

@@ -4780,6 +4829,8 @@ static struct security_operations selinu
 	.file_send_sigiotask =		selinux_file_send_sigiotask,
 	.file_receive =			selinux_file_receive,
 
+	.dentry_open =                  selinux_dentry_open,
+
 	.task_create =			selinux_task_create,
 	.task_alloc_security =		selinux_task_alloc_security,
 	.task_free_security =		selinux_task_free_security,

diff -purN -X linux-2.6.22/Documentation/dontdiff linux-2.6.22.orig/security/selinux/include/avc.h linux-2.6.22/security/selinux/include/avc.h

security/selinux/include/avc.h +2 -0

@@ -110,6 +110,8 @@ int avc_has_perm(u32 ssid, u32 tsid,
                  u16 tclass, u32 requested,
                  struct avc_audit_data *auditdata);
 
+u32 avc_policy_seqno(void);
+
 #define AVC_CALLBACK_GRANT		1
 #define AVC_CALLBACK_TRY_REVOKE		2
 #define AVC_CALLBACK_REVOKE		4

diff -purN -X linux-2.6.22/Documentation/dontdiff linux-2.6.22.orig/security/selinux/include/objsec.h linux-2.6.22/security/selinux/include/objsec.h

security/selinux/include/objsec.h +2 -0

@@ -53,6 +53,8 @@ struct file_security_struct {
 	struct file *file;              /* back pointer to file object */
 	u32 sid;              /* SID of open file description */
 	u32 fown_sid;         /* SID of file owner (for SIGIO) */
+	u32 isid;             /* SID of inode at the time of file open */
+	u32 pseqno;           /* Policy seqno at the time of file open */
 };
 
 struct superblock_security_struct {

diff -purN -X linux-2.6.22/Documentation/dontdiff linux-2.6.22.orig/fs/open.c linux-2.6.22/fs/open.c

fs/open.c +4 -0

@@ -696,6 +696,10 @@ static struct file *__dentry_open(struct
 	f->f_op = fops_get(inode->i_fop);
 	file_move(f, &inode->i_sb->s_files);
 
+	error = security_dentry_open(f);
+	if (error)
+		goto cleanup_all;
+
 	if (!open && f->f_op)
 		open = f->f_op->open;
 	if (open) {

diff -purN -X linux-2.6.22/Documentation/dontdiff linux-2.6.22.orig/include/linux/security.h linux-2.6.22/include/linux/security.h

include/linux/security.h +18 -0

@@ -503,6 +503,13 @@ struct request_sock;
  *	@file contains the file structure being received.
  *	Return 0 if permission is granted.
  *
+ * Security hook for dentry
+ *
+ * @dentry_open
+ *	Save open-time permission checking state for later use upon
+ *	file_permission, and recheck access if anything has changed
+ *	since inode_permission.
+ *
  * Security hooks for task operations.
  *
  * @task_create:

@@ -1253,6 +1260,7 @@ struct security_operations {
 	int (*file_send_sigiotask) (struct task_struct * tsk,
 				    struct fown_struct * fown, int sig);
 	int (*file_receive) (struct file * file);
+	int (*dentry_open)  (struct file *file);
 
 	int (*task_create) (unsigned long clone_flags);
 	int (*task_alloc_security) (struct task_struct * p);

@@ -1854,6 +1862,11 @@ static inline int security_file_receive 
 	return security_ops->file_receive (file);
 }
 
+static inline int security_dentry_open (struct file *file)
+{
+	return security_ops->dentry_open (file);
+}
+
 static inline int security_task_create (unsigned long clone_flags)
 {
 	return security_ops->task_create (clone_flags);

@@ -2529,6 +2542,11 @@ static inline int security_file_receive 
 	return 0;
 }
 
+static inline int security_dentry_open (struct file *file)
+{
+	return 0;
+}
+
 static inline int security_task_create (unsigned long clone_flags)
 {
 	return 0;

security/dummy.c +6 -0

@@ -459,6 +459,11 @@ static int dummy_file_receive (struct fi
 	return 0;
 }
 
+static int dummy_dentry_open (struct file *file)
+{
+	return 0;
+}
+
 static int dummy_task_create (unsigned long clone_flags)
 {
 	return 0;

@@ -1029,6 +1034,7 @@ void security_fixup_ops (struct security
 	set_to_dummy_if_null(ops, file_set_fowner);
 	set_to_dummy_if_null(ops, file_send_sigiotask);
 	set_to_dummy_if_null(ops, file_receive);
+	set_to_dummy_if_null(ops, dentry_open);
 	set_to_dummy_if_null(ops, task_create);
 	set_to_dummy_if_null(ops, task_alloc_security);
 	set_to_dummy_if_null(ops, task_free_security);

Regards,

Quoted 7 lines

It reduces the selinux overhead on read/write by only revalidating permissions in selinux_file_permission if the task or inode labels have changed or the policy has changed since the open-time check. A new LSM hook, security_dentry_open, is added to capture the necessary state at open time to allow this optimization.

Signed-off-by Yuichi Nakamura<[email protected]>

Thanks, looks good.

Acked-by Stephen Smalley <[email protected]>