Re: [RFC]selinux: Improving SELinux read/write performance
On Thu, 13 Sep 2007 08:58:32 -0400
Stephen Smalley wrote:
> On Wed, 2007-09-12 at 17:51 +0900, Yuichi Nakamura wrote:
> Thanks, a few comments below.
Thanks for comments!
> >
> > * Description of patch
> > This patch improves performance of read/write in SELinux.
> > It improves performance by skipping permission check in
> > selinux_file_permission. Permission is only checked when
> > sid change or policy load is detected after file open.
> > To detect sid change, new LSM hook securiy_dentry_open is added.
>
> I think I'd reword this a little, e.g.
>
> 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.
I see, I will use that.
> >
> > Signed-off-by: Yuichi Nakamura<[EMAIL PROTECTED]>
> > ---
> > fs/open.c |5
> > include/linux/security.h | 16 ++
> > security/dummy.c |6 +
> > security/selinux/avc.c|5
> > security/selinux/hooks.c | 43
> > +-
> > security/selinux/include/avc.h|2 +
> > security/selinux/include/objsec.h |2 +
> > 7 files changed, 78 insertions(+), 1 deletion(-)
>
> > 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
> > --- linux-2.6.22.orig/security/selinux/hooks.c 2007-07-09
> > 08:32:17.0 +0900
> > +++ linux-2.6.22/security/selinux/hooks.c 2007-09-12 08:42:49.0
> > +0900
> > @@ -80,6 +82,7 @@
> >
> > #define XATTR_SELINUX_SUFFIX "selinux"
> > #define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
> > +#define ACC_MODE(x) ("\000\004\002\006"[(x)_ACCMODE])
>
> Leftover from prior version of the patch, no longer needed.
Fixed.
>
>
> > @@ -2715,6 +2737,23 @@ 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;
>
> I'd add a comment here, e.g.
> /*
> * 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.
> */
Fixed.
>
> > + fsec->isid = isec->sid;
> > + fsec->pseqno = avc_policy_seqno();
> > +
> > + /*Permission has to be rechecked here.
> > + Policy load of inode sid can happen between
> > + may_open and selinux_dentry_open.*/
>
> Typo in the comment (s/of/or/), coding style isn't right for a
> multi-line comment, and likely needs clarification, e.g.
> /*
>* 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.
>*/
Fixed.
>
> > + return inode_has_perm(current, inode, file_to_av(file), NULL);
> > +}
> > +
> > /* task security operations */
> >
> > static int selinux_task_create(unsigned long clone_flags)
>
> > diff -purN -X linux-2.6.22/Documentation/dontdiff
> > linux-2.6.22.orig/fs/open.c linux-2.6.22/fs/open.c
> > --- linux-2.6.22.orig/fs/open.c 2007-07-09 08:32:17.0 +0900
> > +++ linux-2.6.22/fs/open.c 2007-09-12 08:31:24.0 +0900
> > @@ -696,8 +696,13 @@ static struct file *__dentry_open(struct
> > f->f_op = fops_get(inode->i_fop);
> > file_move(f, >i_sb->s_files);
> >
> > + error = security_dentry_open(f);
> > + if (error)
> > + goto cleanup_all;
> > +
> > if (!open && f->f_op)
> > open = f->f_op->open;
> > +
>
> Extraneous whitespace leftover from prior version of the patch.
Fixed.
>
> > if (open) {
> > error = open(inode, f);
> > if (error)
> > 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
> > --- linux-2.6.22.orig/include/linux/security.h 2007-07-09
> > 08:32:17.0 +0900
> > +++ linux-2.6.22/include/linux/security.h 2007-09-12 08:30:16.0
> > +0900
> > @@ -503,6 +503,11 @@ struct request_sock;
> > * @file contains the file structure being received.
> > * Return 0 if
Re: [RFC]selinux: Improving SELinux read/write performance
On Wed, 2007-09-12 at 17:51 +0900, Yuichi Nakamura wrote:
> Hi.
>
> Stephen Smalley pointed out possibility of race condition
> in off-list discussion.
> Stephen Smalley said:
> > One other observation about the patch: it presently leaves open a
> > (small) race window in which the file could get relabeled or policy gets
> > reloaded between the time of the normal permission check (from
> > selinux_inode_permission) and the time you copy the inode SID and policy
> > seqno to the file security struct. In which case you might end up never
> > revalidating access upon read/write even though conditions changed since
> > the open-time permission check. Not sure how to cleanly fix in a
> > lock-free manner, and adding locks here will only make matters worse.
>
> To fix that, permission has to be checked in selinux_dentry_open.
> Therefore, in open, number of permission checks increased.
> As shown in benchmark result below, it does not affect open/close
> performance so much.
>
> Following is benchmark result.
> * Benchmark
> lmbench simple read,write,open/close.
>
> * Before tuning
> 1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
> Base SELinux Overhead(%)
> Simple read 1.10 1.24 12.3
> Simple write1.02 1.14 14.0
> open/close 5.97 7.45 24.9
> * Base: kernel compiled without SELinux support
>
> 2) Result for SH(SH4, SH7751R), kernel 2.6.22
> BaseSELinux Overhead(%)
> Simple read 2.395.49 130.5
> Simple write2.075.10 146.6
> open/close 32.662.8 93.0
>
> * After tuning
> 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 write1.02 1.0240.6(Before 14.0)
> open/close 5.97 7.48 25.3(Before 24.9)
> * Base: kernel compiled without SELinux support
>
> 2) Result for SH(SH4, SH7751R), kernel 2.6.22
> BaseSELinux Overhead(%)
> Simple read 2.392.63 10.4(Before 130.5)
> Simple write2.072.34 13.1(Before 146.6)
> open/close 32.658.7 80.2(before 93.0)
>
> Next is a patch.
Thanks, a few comments below.
>
> * Description of patch
> This patch improves performance of read/write in SELinux.
> It improves performance by skipping permission check in
> selinux_file_permission. Permission is only checked when
> sid change or policy load is detected after file open.
> To detect sid change, new LSM hook securiy_dentry_open is added.
I think I'd reword this a little, e.g.
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 |5
> include/linux/security.h | 16 ++
> security/dummy.c |6 +
> security/selinux/avc.c|5
> security/selinux/hooks.c | 43
> +-
> security/selinux/include/avc.h|2 +
> security/selinux/include/objsec.h |2 +
> 7 files changed, 78 insertions(+), 1 deletion(-)
> 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
> --- linux-2.6.22.orig/security/selinux/hooks.c2007-07-09
> 08:32:17.0 +0900
> +++ linux-2.6.22/security/selinux/hooks.c 2007-09-12 08:42:49.0
> +0900
> @@ -80,6 +82,7 @@
>
> #define XATTR_SELINUX_SUFFIX "selinux"
> #define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
> +#define ACC_MODE(x) ("\000\004\002\006"[(x)_ACCMODE])
Leftover from prior version of the patch, no longer needed.
> @@ -2715,6 +2737,23 @@ 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;
I'd add a comment here, e.g.
/*
* 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();
> +
> + /*Permission has to be rechecked here.
> + Policy load of inode sid can happen between
> + may_open and selinux_dentry_open.*/
Typo in the comment
Re: [RFC]selinux: Improving SELinux read/write performance
On Wed, 2007-09-12 at 17:51 +0900, Yuichi Nakamura wrote:
Hi.
Stephen Smalley pointed out possibility of race condition
in off-list discussion.
Stephen Smalley said:
One other observation about the patch: it presently leaves open a
(small) race window in which the file could get relabeled or policy gets
reloaded between the time of the normal permission check (from
selinux_inode_permission) and the time you copy the inode SID and policy
seqno to the file security struct. In which case you might end up never
revalidating access upon read/write even though conditions changed since
the open-time permission check. Not sure how to cleanly fix in a
lock-free manner, and adding locks here will only make matters worse.
To fix that, permission has to be checked in selinux_dentry_open.
Therefore, in open, number of permission checks increased.
As shown in benchmark result below, it does not affect open/close
performance so much.
Following is benchmark result.
* Benchmark
lmbench simple read,write,open/close.
* Before tuning
1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
Base SELinux Overhead(%)
Simple read 1.10 1.24 12.3
Simple write1.02 1.14 14.0
open/close 5.97 7.45 24.9
* Base: kernel compiled without SELinux support
2) Result for SH(SH4, SH7751R), kernel 2.6.22
BaseSELinux Overhead(%)
Simple read 2.395.49 130.5
Simple write2.075.10 146.6
open/close 32.662.8 93.0
* After tuning
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 write1.02 1.0240.6(Before 14.0)
open/close 5.97 7.48 25.3(Before 24.9)
* Base: kernel compiled without SELinux support
2) Result for SH(SH4, SH7751R), kernel 2.6.22
BaseSELinux Overhead(%)
Simple read 2.392.63 10.4(Before 130.5)
Simple write2.072.34 13.1(Before 146.6)
open/close 32.658.7 80.2(before 93.0)
Next is a patch.
Thanks, a few comments below.
* Description of patch
This patch improves performance of read/write in SELinux.
It improves performance by skipping permission check in
selinux_file_permission. Permission is only checked when
sid change or policy load is detected after file open.
To detect sid change, new LSM hook securiy_dentry_open is added.
I think I'd reword this a little, e.g.
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 |5
include/linux/security.h | 16 ++
security/dummy.c |6 +
security/selinux/avc.c|5
security/selinux/hooks.c | 43
+-
security/selinux/include/avc.h|2 +
security/selinux/include/objsec.h |2 +
7 files changed, 78 insertions(+), 1 deletion(-)
snip
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
--- linux-2.6.22.orig/security/selinux/hooks.c2007-07-09
08:32:17.0 +0900
+++ linux-2.6.22/security/selinux/hooks.c 2007-09-12 08:42:49.0
+0900
@@ -80,6 +82,7 @@
#define XATTR_SELINUX_SUFFIX selinux
#define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
+#define ACC_MODE(x) (\000\004\002\006[(x)O_ACCMODE])
Leftover from prior version of the patch, no longer needed.
snip
@@ -2715,6 +2737,23 @@ 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;
I'd add a comment here, e.g.
/*
* 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();
+
+ /*Permission has to be rechecked here.
+ Policy load of inode sid can happen between
+ may_open and selinux_dentry_open.*/
Typo in the comment (s/of/or/), coding style isn't right for a
multi-line comment, and likely needs clarification, e.g.
/*
Re: [RFC]selinux: Improving SELinux read/write performance
On Thu, 13 Sep 2007 08:58:32 -0400
Stephen Smalley wrote:
On Wed, 2007-09-12 at 17:51 +0900, Yuichi Nakamura wrote:
snip
Thanks, a few comments below.
Thanks for comments!
* Description of patch
This patch improves performance of read/write in SELinux.
It improves performance by skipping permission check in
selinux_file_permission. Permission is only checked when
sid change or policy load is detected after file open.
To detect sid change, new LSM hook securiy_dentry_open is added.
I think I'd reword this a little, e.g.
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.
I see, I will use that.
Signed-off-by: Yuichi Nakamura[EMAIL PROTECTED]
---
fs/open.c |5
include/linux/security.h | 16 ++
security/dummy.c |6 +
security/selinux/avc.c|5
security/selinux/hooks.c | 43
+-
security/selinux/include/avc.h|2 +
security/selinux/include/objsec.h |2 +
7 files changed, 78 insertions(+), 1 deletion(-)
snip
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
--- linux-2.6.22.orig/security/selinux/hooks.c 2007-07-09
08:32:17.0 +0900
+++ linux-2.6.22/security/selinux/hooks.c 2007-09-12 08:42:49.0
+0900
@@ -80,6 +82,7 @@
#define XATTR_SELINUX_SUFFIX selinux
#define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
+#define ACC_MODE(x) (\000\004\002\006[(x)O_ACCMODE])
Leftover from prior version of the patch, no longer needed.
Fixed.
snip
@@ -2715,6 +2737,23 @@ 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;
I'd add a comment here, e.g.
/*
* 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.
*/
Fixed.
+ fsec-isid = isec-sid;
+ fsec-pseqno = avc_policy_seqno();
+
+ /*Permission has to be rechecked here.
+ Policy load of inode sid can happen between
+ may_open and selinux_dentry_open.*/
Typo in the comment (s/of/or/), coding style isn't right for a
multi-line comment, and likely needs clarification, e.g.
/*
* 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.
*/
Fixed.
+ return inode_has_perm(current, inode, file_to_av(file), NULL);
+}
+
/* task security operations */
static int selinux_task_create(unsigned long clone_flags)
diff -purN -X linux-2.6.22/Documentation/dontdiff
linux-2.6.22.orig/fs/open.c linux-2.6.22/fs/open.c
--- linux-2.6.22.orig/fs/open.c 2007-07-09 08:32:17.0 +0900
+++ linux-2.6.22/fs/open.c 2007-09-12 08:31:24.0 +0900
@@ -696,8 +696,13 @@ 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;
+
Extraneous whitespace leftover from prior version of the patch.
Fixed.
if (open) {
error = open(inode, f);
if (error)
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
--- linux-2.6.22.orig/include/linux/security.h 2007-07-09
08:32:17.0 +0900
+++ linux-2.6.22/include/linux/security.h 2007-09-12 08:30:16.0
+0900
@@ -503,6 +503,11 @@ struct request_sock;
* @file contains the file structure being received.
* Return 0 if permission is granted.
*
+ * Security hook for dentry
+ *
+ * @dentry_open
+ * Check permission or get additional information before opening dentry.
+ *
More precisely, Save open-time
Re: [RFC]selinux: Improving SELinux read/write performance
Hi.
Stephen Smalley pointed out possibility of race condition
in off-list discussion.
Stephen Smalley said:
> One other observation about the patch: it presently leaves open a
> (small) race window in which the file could get relabeled or policy gets
> reloaded between the time of the normal permission check (from
> selinux_inode_permission) and the time you copy the inode SID and policy
> seqno to the file security struct. In which case you might end up never
> revalidating access upon read/write even though conditions changed since
> the open-time permission check. Not sure how to cleanly fix in a
> lock-free manner, and adding locks here will only make matters worse.
To fix that, permission has to be checked in selinux_dentry_open.
Therefore, in open, number of permission checks increased.
As shown in benchmark result below, it does not affect open/close
performance so much.
Following is benchmark result.
* Benchmark
lmbench simple read,write,open/close.
* Before tuning
1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
Base SELinux Overhead(%)
Simple read 1.10 1.24 12.3
Simple write1.02 1.14 14.0
open/close 5.97 7.45 24.9
* Base: kernel compiled without SELinux support
2) Result for SH(SH4, SH7751R), kernel 2.6.22
BaseSELinux Overhead(%)
Simple read 2.395.49 130.5
Simple write2.075.10 146.6
open/close 32.662.8 93.0
* After tuning
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 write1.02 1.0240.6(Before 14.0)
open/close 5.97 7.48 25.3(Before 24.9)
* Base: kernel compiled without SELinux support
2) Result for SH(SH4, SH7751R), kernel 2.6.22
BaseSELinux Overhead(%)
Simple read 2.392.63 10.4(Before 130.5)
Simple write2.072.34 13.1(Before 146.6)
open/close 32.658.7 80.2(before 93.0)
Next is a patch.
* Description of patch
This patch improves performance of read/write in SELinux.
It improves performance by skipping permission check in
selinux_file_permission. Permission is only checked when
sid change or policy load is detected after file open.
To detect sid change, new LSM hook securiy_dentry_open is added.
Signed-off-by: Yuichi Nakamura<[EMAIL PROTECTED]>
---
fs/open.c |5
include/linux/security.h | 16 ++
security/dummy.c |6 +
security/selinux/avc.c|5
security/selinux/hooks.c | 43 +-
security/selinux/include/avc.h|2 +
security/selinux/include/objsec.h |2 +
7 files changed, 78 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
--- linux-2.6.22.orig/security/selinux/avc.c2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/security/selinux/avc.c 2007-09-12 08:24:27.0 +0900
@@ -913,3 +913,8 @@ int avc_has_perm(u32 ssid, u32 tsid, u16
avc_audit(ssid, tsid, tclass, requested, , 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
--- linux-2.6.22.orig/security/selinux/hooks.c 2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/security/selinux/hooks.c 2007-09-12 08:42:49.0
+0900
@@ -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,
@@ -80,6 +82,7 @@
#define XATTR_SELINUX_SUFFIX "selinux"
#define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
+#define ACC_MODE(x) ("\000\004\002\006"[(x)_ACCMODE])
extern unsigned int policydb_loaded_version;
extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
@@ -2458,7 +2461,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 +2483,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;
+
Re: [RFC]selinux: Improving SELinux read/write performance
Hi.
Stephen Smalley pointed out possibility of race condition
in off-list discussion.
Stephen Smalley said:
One other observation about the patch: it presently leaves open a
(small) race window in which the file could get relabeled or policy gets
reloaded between the time of the normal permission check (from
selinux_inode_permission) and the time you copy the inode SID and policy
seqno to the file security struct. In which case you might end up never
revalidating access upon read/write even though conditions changed since
the open-time permission check. Not sure how to cleanly fix in a
lock-free manner, and adding locks here will only make matters worse.
To fix that, permission has to be checked in selinux_dentry_open.
Therefore, in open, number of permission checks increased.
As shown in benchmark result below, it does not affect open/close
performance so much.
Following is benchmark result.
* Benchmark
lmbench simple read,write,open/close.
* Before tuning
1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
Base SELinux Overhead(%)
Simple read 1.10 1.24 12.3
Simple write1.02 1.14 14.0
open/close 5.97 7.45 24.9
* Base: kernel compiled without SELinux support
2) Result for SH(SH4, SH7751R), kernel 2.6.22
BaseSELinux Overhead(%)
Simple read 2.395.49 130.5
Simple write2.075.10 146.6
open/close 32.662.8 93.0
* After tuning
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 write1.02 1.0240.6(Before 14.0)
open/close 5.97 7.48 25.3(Before 24.9)
* Base: kernel compiled without SELinux support
2) Result for SH(SH4, SH7751R), kernel 2.6.22
BaseSELinux Overhead(%)
Simple read 2.392.63 10.4(Before 130.5)
Simple write2.072.34 13.1(Before 146.6)
open/close 32.658.7 80.2(before 93.0)
Next is a patch.
* Description of patch
This patch improves performance of read/write in SELinux.
It improves performance by skipping permission check in
selinux_file_permission. Permission is only checked when
sid change or policy load is detected after file open.
To detect sid change, new LSM hook securiy_dentry_open is added.
Signed-off-by: Yuichi Nakamura[EMAIL PROTECTED]
---
fs/open.c |5
include/linux/security.h | 16 ++
security/dummy.c |6 +
security/selinux/avc.c|5
security/selinux/hooks.c | 43 +-
security/selinux/include/avc.h|2 +
security/selinux/include/objsec.h |2 +
7 files changed, 78 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
--- linux-2.6.22.orig/security/selinux/avc.c2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/security/selinux/avc.c 2007-09-12 08:24:27.0 +0900
@@ -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
--- linux-2.6.22.orig/security/selinux/hooks.c 2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/security/selinux/hooks.c 2007-09-12 08:42:49.0
+0900
@@ -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,
@@ -80,6 +82,7 @@
#define XATTR_SELINUX_SUFFIX selinux
#define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
+#define ACC_MODE(x) (\000\004\002\006[(x)O_ACCMODE])
extern unsigned int policydb_loaded_version;
extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
@@ -2458,7 +2461,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 +2483,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
Re: [RFC]selinux: Improving SELinux read/write performance
On Mon, 2007-09-10 at 10:31 +0900, Yuichi Nakamura wrote:
> Next is updated patch.
Thanks.
Please include the short description of the patch though when
re-submitting.
> Signed-off-by: Yuichi Nakamura<[EMAIL PROTECTED]>
> ---
> fs/open.c |5 +
> include/linux/security.h | 16
> security/selinux/avc.c|5 +
> security/selinux/hooks.c | 36 +++-
> security/selinux/include/avc.h|2 ++
> security/selinux/include/objsec.h |2 ++
> 6 files changed, 65 insertions(+), 1 deletion(-)
Still missing the necessary changes to security/dummy.c (add
dummy_dentry_open() and update security_fixup_ops()). For
CONFIG_SECURITY=y but SELinux disabled.
Also, have you re-run your benchmarks with this version of the patch?
> 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
> --- linux-2.6.22.orig/security/selinux/avc.c 2007-07-09 08:32:17.0
> +0900
> +++ linux-2.6.22/security/selinux/avc.c 2007-09-10 09:56:22.0
> +0900
> @@ -913,3 +913,8 @@ int avc_has_perm(u32 ssid, u32 tsid, u16
> avc_audit(ssid, tsid, tclass, requested, , 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
> --- linux-2.6.22.orig/security/selinux/hooks.c2007-07-09
> 08:32:17.0 +0900
> +++ linux-2.6.22/security/selinux/hooks.c 2007-09-10 10:11:13.0
> +0900
> @@ -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,17 @@ 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, int flags)
> +{
> + struct file_security_struct *fsec;
> + struct inode_security_struct *isec;
> + fsec = file->f_security;
> + isec = file->f_path.dentry->d_inode->i_security;
> + fsec->isid = isec->sid;
> + fsec->pseqno = avc_policy_seqno();
> + return 0;
> +}
> +
> /* task security operations */
>
> static int selinux_task_create(unsigned long clone_flags)
> @@ -4780,6 +4812,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
> --- linux-2.6.22.orig/security/selinux/include/avc.h 2007-07-09
> 08:32:17.0 +0900
> +++ linux-2.6.22/security/selinux/include/avc.h 2007-09-10
> 09:56:22.0 +0900
> @@ -110,6 +110,8 @@ int avc_has_perm(u32 ssid, u32 tsid,
> u16 tclass, u32 requested,
>
Re: [RFC]selinux: Improving SELinux read/write performance
On Mon, 2007-09-10 at 10:31 +0900, Yuichi Nakamura wrote:
Next is updated patch.
Thanks.
Please include the short description of the patch though when
re-submitting.
Signed-off-by: Yuichi Nakamura[EMAIL PROTECTED]
---
fs/open.c |5 +
include/linux/security.h | 16
security/selinux/avc.c|5 +
security/selinux/hooks.c | 36 +++-
security/selinux/include/avc.h|2 ++
security/selinux/include/objsec.h |2 ++
6 files changed, 65 insertions(+), 1 deletion(-)
Still missing the necessary changes to security/dummy.c (add
dummy_dentry_open() and update security_fixup_ops()). For
CONFIG_SECURITY=y but SELinux disabled.
Also, have you re-run your benchmarks with this version of the patch?
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
--- linux-2.6.22.orig/security/selinux/avc.c 2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/security/selinux/avc.c 2007-09-10 09:56:22.0
+0900
@@ -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
--- linux-2.6.22.orig/security/selinux/hooks.c2007-07-09
08:32:17.0 +0900
+++ linux-2.6.22/security/selinux/hooks.c 2007-09-10 10:11:13.0
+0900
@@ -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,17 @@ 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, int flags)
+{
+ struct file_security_struct *fsec;
+ struct inode_security_struct *isec;
+ fsec = file-f_security;
+ isec = file-f_path.dentry-d_inode-i_security;
+ fsec-isid = isec-sid;
+ fsec-pseqno = avc_policy_seqno();
+ return 0;
+}
+
/* task security operations */
static int selinux_task_create(unsigned long clone_flags)
@@ -4780,6 +4812,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
--- linux-2.6.22.orig/security/selinux/include/avc.h 2007-07-09
08:32:17.0 +0900
+++ linux-2.6.22/security/selinux/include/avc.h 2007-09-10
09:56:22.0 +0900
@@ -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
Re: [RFC]selinux: Improving SELinux read/write performance
On Thu, 06 Sep 2007 09:47:15 -0400
Stephen Smalley wrote:
> >
> > @@ -431,8 +432,10 @@ static int avc_latest_notif_update(int s
> > ret = -EAGAIN;
> > }
> > } else {
> > - if (seqno > avc_cache.latest_notif)
> > + if (seqno > avc_cache.latest_notif) {
> > avc_cache.latest_notif = seqno;
> > + policy_seqno = seqno;
> > + }
>
> I would have just provided an avc interface for obtaining the seqno,
> e.g.
> u32 avc_policy_seqno(void)
> {
> return avc_cache.latest_notif;
> }
Fixed.
>
> > }
> > spin_unlock_irqrestore(_lock, flag);
> >
> > 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
> > --- linux-2.6.22.orig/security/selinux/hooks.c 2007-07-09
> > 08:32:17.0 +0900
> > +++ linux-2.6.22/security/selinux/hooks.c 2007-09-06 16:08:36.0
> > +0900
> > @@ -84,6 +84,7 @@
> > extern unsigned int policydb_loaded_version;
> > extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
> > extern int selinux_compat_net;
> > +extern u32 policy_seqno;
>
> I think that they frown upon extern declarations in .c files (versus
> in .h files), so we don't want to add more - we ultimately should clean
> up what we presently have.
Removed policy_seqno.
>
> >
> > #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
> > int selinux_enforcing = 0;
> > @@ -220,6 +221,8 @@ static int file_alloc_security(struct fi
> >
> > fsec->file = file;
> > fsec->sid = tsec->sid;
> > + fsec->tsid = tsec->sid;
>
> I'm not sure why we need the separate field here, as fsec->sid already
> holds the allocating task SID and doesn't change.
I see.
removed fsec->tsid.
> > + fsec->pseqno = policy_seqno;
>
> Not sure that you want to set the seqno here versus from your new hook,
> as it could conceivably change in between.
Fixed, pseqno is set in selinux_dentry_open.
> > fsec->fown_sid = tsec->sid;
> > file->f_security = fsec;
> >
> > @@ -2458,7 +2461,7 @@ static int selinux_inode_listsecurity(st
> >
> > /* file security operations */
> >
> > -static int selinux_file_permission(struct file *file, int mask)
> > +static int do_selinux_file_permission(struct file *file, int mask)
>
> Might want to rename for clarity, e.g.
> selinux_revalidate_file_permission or
> selinux_file_permission_slow (slow path) or
> something similar.
Renamed to selinux_revalidate_file_permission.
>
> > {
> > int rc;
> > struct inode *inode = file->f_path.dentry->d_inode;
> > @@ -2480,6 +2483,43 @@ 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->tsid) {
> > + if (tsec->sid != fsec->sid) {
> > + struct vfsmount *mnt = file->f_path.mnt;
> > + struct dentry *dentry = file->f_path.dentry;
> > + struct avc_audit_data ad;
> > + int rc;
> > + AVC_AUDIT_DATA_INIT(, FS);
> > + ad.u.fs.mnt = mnt;
> > + ad.u.fs.dentry = dentry;
> > + rc = avc_has_perm(tsec->sid, fsec->sid,
> > + SECCLASS_FD,
> > + FD__USE,
> > + );
> > + if (rc)
> > + return rc;
> > + }
>
> Why inline the FD_USE check here given that you are falling back to the
> full function call anyway? I also don't see why you separate this from
> the rest of the comparison - I'd just make it something like:
> if (tsec->sid == fsec->sid && isec->sid == fsec->isid &&
> avc_seqno() == fsec->seqno)
> return selinux_netlbl_inode_permission(inode, mask);
> return selinux_revalidate_file_permission(file, mask);
Thanks, I missed that.
FD_USE check is called in file_permission..
Fixed like you pointed out.
> > static int selinux_file_alloc_security(struct file *file)
> > {
> > return file_alloc_security(file);
> > @@ -2715,6 +2755,16 @@ 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, int flags)
> > +{
> > + struct file_security_struct *fsec;
> > + struct inode_security_struct *isec;
> > + fsec = file->f_security;
> > + isec =
Re: [RFC]selinux: Improving SELinux read/write performance
On Thu, 06 Sep 2007 09:47:15 -0400
Stephen Smalley wrote:
snip
@@ -431,8 +432,10 @@ static int avc_latest_notif_update(int s
ret = -EAGAIN;
}
} else {
- if (seqno avc_cache.latest_notif)
+ if (seqno avc_cache.latest_notif) {
avc_cache.latest_notif = seqno;
+ policy_seqno = seqno;
+ }
I would have just provided an avc interface for obtaining the seqno,
e.g.
u32 avc_policy_seqno(void)
{
return avc_cache.latest_notif;
}
Fixed.
}
spin_unlock_irqrestore(notif_lock, flag);
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
--- linux-2.6.22.orig/security/selinux/hooks.c 2007-07-09
08:32:17.0 +0900
+++ linux-2.6.22/security/selinux/hooks.c 2007-09-06 16:08:36.0
+0900
@@ -84,6 +84,7 @@
extern unsigned int policydb_loaded_version;
extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
extern int selinux_compat_net;
+extern u32 policy_seqno;
I think that they frown upon extern declarations in .c files (versus
in .h files), so we don't want to add more - we ultimately should clean
up what we presently have.
Removed policy_seqno.
#ifdef CONFIG_SECURITY_SELINUX_DEVELOP
int selinux_enforcing = 0;
@@ -220,6 +221,8 @@ static int file_alloc_security(struct fi
fsec-file = file;
fsec-sid = tsec-sid;
+ fsec-tsid = tsec-sid;
I'm not sure why we need the separate field here, as fsec-sid already
holds the allocating task SID and doesn't change.
I see.
removed fsec-tsid.
+ fsec-pseqno = policy_seqno;
Not sure that you want to set the seqno here versus from your new hook,
as it could conceivably change in between.
Fixed, pseqno is set in selinux_dentry_open.
fsec-fown_sid = tsec-sid;
file-f_security = fsec;
@@ -2458,7 +2461,7 @@ static int selinux_inode_listsecurity(st
/* file security operations */
-static int selinux_file_permission(struct file *file, int mask)
+static int do_selinux_file_permission(struct file *file, int mask)
Might want to rename for clarity, e.g.
selinux_revalidate_file_permission or
selinux_file_permission_slow (slow path) or
something similar.
Renamed to selinux_revalidate_file_permission.
{
int rc;
struct inode *inode = file-f_path.dentry-d_inode;
@@ -2480,6 +2483,43 @@ 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-tsid) {
+ if (tsec-sid != fsec-sid) {
+ struct vfsmount *mnt = file-f_path.mnt;
+ struct dentry *dentry = file-f_path.dentry;
+ struct avc_audit_data ad;
+ int rc;
+ AVC_AUDIT_DATA_INIT(ad, FS);
+ ad.u.fs.mnt = mnt;
+ ad.u.fs.dentry = dentry;
+ rc = avc_has_perm(tsec-sid, fsec-sid,
+ SECCLASS_FD,
+ FD__USE,
+ ad);
+ if (rc)
+ return rc;
+ }
Why inline the FD_USE check here given that you are falling back to the
full function call anyway? I also don't see why you separate this from
the rest of the comparison - I'd just make it something like:
if (tsec-sid == fsec-sid isec-sid == fsec-isid
avc_seqno() == fsec-seqno)
return selinux_netlbl_inode_permission(inode, mask);
return selinux_revalidate_file_permission(file, mask);
Thanks, I missed that.
FD_USE check is called in file_permission..
Fixed like you pointed out.
static int selinux_file_alloc_security(struct file *file)
{
return file_alloc_security(file);
@@ -2715,6 +2755,16 @@ 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, int flags)
+{
+ struct file_security_struct *fsec;
+ struct inode_security_struct *isec;
+ fsec = file-f_security;
+ isec = file-f_path.dentry-d_inode-i_security;
+ fsec-isid = isec-sid;
Set the seqno here too. Ideally, it would come straight from the AVC
entry that was used for the open-time check, but that is a bit more
invasive and there will always
Re: [RFC]selinux: Improving SELinux read/write performance
On Thu, 2007-09-06 at 16:27 +0900, Yuichi Nakamura wrote:
> Hello.
>
> As I posted before in selinux list,
> I found big overhead of SELinux in read/write on some CPUs,
> and trying tuning.
> There were discussion in previous threads.
> Part 1:
> http://marc.info/?t=11884534341=1=2
> Part 2:
> http://marc.info/?t=11888074984=1=2
>
> I would like to RFC again about this topic.
Thanks for your work on this, as this is clearly an important area to
improve.
> 1. Background
> Look at benchmark result below.
> lmbench simple read/write.
> 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 write1.00 1.14 14.0
> * Base: kernel compiled without SELinux support
>
> 2) Result for SH(SH4, SH7751R), kernel 2.6.22
> BaseSELinux Overhead(%)
> Simple read 2.395.49 130.5
> Simple write2.075.10 146.6
> # This result is a little different from previous threads,
> # because I changed some kernel configs.
>
> Overhead more than 100%
> I also found about 70-90% overhead in ARM.
>
> 2. About patch
> I found a overhead in selinux_file_permission function.
> This is a function that is called in read/write calls,
> and does SELinux permission check.
> SELinux checks permission both in open and read/write time.
> Stephen Smalley sugessted that we can usually skip permission check
> in selinux_file_permission.
> By this patch,
> permission check in selinux_file_permssion is done only when
> - sid of task has changed after file open
> - sid of inode has changed after file open
> - policy load or boolean change happen after file open
>
> To detect these changes,
> I added entries in file_security struct and saving these values at file open.
>
> And to save sid of inode at the time of file open,
> I had to add new LSM hook in __dentry_open function.
>
> 3. Benchmark after applying this patch
> 1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
> Base SELinux Overhead(%)
> Simple read 1.10 1.12 1.6(Before 12.3)
> Simple write1.00 1.03 3.6(Before 14.0)
>
> 2) Result for SH(SH4, SH7751R), kernel 2.6.22
> BaseSELinux Overhead(%)
> Simple read 2.392.65 11.1(Before 130.5)
> Simple write2.072.28 10.5(Before 146.6)
>
> Performance has improved a lot.
> I want comments from community.
>
> Signed-off-by: Yuichi Nakamura<[EMAIL PROTECTED]>
> ---
> fs/open.c |5 +++
> include/linux/security.h | 11 +++
> security/selinux/avc.c|5 ++-
> security/selinux/hooks.c | 54
> +-
> security/selinux/include/objsec.h |3 ++
> 5 files changed, 76 insertions(+), 2 deletions(-)
> 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
> --- linux-2.6.22.orig/security/selinux/avc.c 2007-07-09 08:32:17.0
> +0900
> +++ linux-2.6.22/security/selinux/avc.c 2007-09-06 14:33:35.0
> +0900
> @@ -123,6 +123,7 @@ DEFINE_PER_CPU(struct avc_cache_stats, a
> #endif
>
> static struct avc_cache avc_cache;
> +u32 policy_seqno = 0;
> static struct avc_callback_node *avc_callbacks;
> static struct kmem_cache *avc_node_cachep;
>
> @@ -431,8 +432,10 @@ static int avc_latest_notif_update(int s
> ret = -EAGAIN;
> }
> } else {
> - if (seqno > avc_cache.latest_notif)
> + if (seqno > avc_cache.latest_notif) {
> avc_cache.latest_notif = seqno;
> + policy_seqno = seqno;
> + }
I would have just provided an avc interface for obtaining the seqno,
e.g.
u32 avc_policy_seqno(void)
{
return avc_cache.latest_notif;
}
> }
> spin_unlock_irqrestore(_lock, flag);
>
> 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
> --- linux-2.6.22.orig/security/selinux/hooks.c2007-07-09
> 08:32:17.0 +0900
> +++ linux-2.6.22/security/selinux/hooks.c 2007-09-06 16:08:36.0
> +0900
> @@ -84,6 +84,7 @@
> extern unsigned int policydb_loaded_version;
> extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
> extern int selinux_compat_net;
> +extern u32 policy_seqno;
I think that they frown upon extern declarations in .c files (versus
in .h files), so we don't want to add more - we ultimately should clean
up what we presently have.
>
> #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
> int selinux_enforcing = 0;
> @@ -220,6 +221,8 @@ static int file_alloc_security(struct fi
>
> fsec->file = file;
> fsec->sid = tsec->sid;
> + fsec->tsid = tsec->sid;
I'm not sure why we need the separate field here, as
[RFC]selinux: Improving SELinux read/write performance
Hello.
As I posted before in selinux list,
I found big overhead of SELinux in read/write on some CPUs,
and trying tuning.
There were discussion in previous threads.
Part 1:
http://marc.info/?t=11884534341=1=2
Part 2:
http://marc.info/?t=11888074984=1=2
I would like to RFC again about this topic.
1. Background
Look at benchmark result below.
lmbench simple read/write.
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 write1.00 1.14 14.0
* Base: kernel compiled without SELinux support
2) Result for SH(SH4, SH7751R), kernel 2.6.22
BaseSELinux Overhead(%)
Simple read 2.395.49 130.5
Simple write2.075.10 146.6
# This result is a little different from previous threads,
# because I changed some kernel configs.
Overhead more than 100%
I also found about 70-90% overhead in ARM.
2. About patch
I found a overhead in selinux_file_permission function.
This is a function that is called in read/write calls,
and does SELinux permission check.
SELinux checks permission both in open and read/write time.
Stephen Smalley sugessted that we can usually skip permission check
in selinux_file_permission.
By this patch,
permission check in selinux_file_permssion is done only when
- sid of task has changed after file open
- sid of inode has changed after file open
- policy load or boolean change happen after file open
To detect these changes,
I added entries in file_security struct and saving these values at file open.
And to save sid of inode at the time of file open,
I had to add new LSM hook in __dentry_open function.
3. Benchmark after applying this patch
1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
Base SELinux Overhead(%)
Simple read 1.10 1.12 1.6(Before 12.3)
Simple write1.00 1.03 3.6(Before 14.0)
2) Result for SH(SH4, SH7751R), kernel 2.6.22
BaseSELinux Overhead(%)
Simple read 2.392.65 11.1(Before 130.5)
Simple write2.072.28 10.5(Before 146.6)
Performance has improved a lot.
I want comments from community.
Signed-off-by: Yuichi Nakamura<[EMAIL PROTECTED]>
---
fs/open.c |5 +++
include/linux/security.h | 11 +++
security/selinux/avc.c|5 ++-
security/selinux/hooks.c | 54 +-
security/selinux/include/objsec.h |3 ++
5 files changed, 76 insertions(+), 2 deletions(-)
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
--- linux-2.6.22.orig/security/selinux/avc.c2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/security/selinux/avc.c 2007-09-06 14:33:35.0 +0900
@@ -123,6 +123,7 @@ DEFINE_PER_CPU(struct avc_cache_stats, a
#endif
static struct avc_cache avc_cache;
+u32 policy_seqno = 0;
static struct avc_callback_node *avc_callbacks;
static struct kmem_cache *avc_node_cachep;
@@ -431,8 +432,10 @@ static int avc_latest_notif_update(int s
ret = -EAGAIN;
}
} else {
- if (seqno > avc_cache.latest_notif)
+ if (seqno > avc_cache.latest_notif) {
avc_cache.latest_notif = seqno;
+ policy_seqno = seqno;
+ }
}
spin_unlock_irqrestore(_lock, flag);
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
--- linux-2.6.22.orig/security/selinux/hooks.c 2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/security/selinux/hooks.c 2007-09-06 16:08:36.0
+0900
@@ -84,6 +84,7 @@
extern unsigned int policydb_loaded_version;
extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
extern int selinux_compat_net;
+extern u32 policy_seqno;
#ifdef CONFIG_SECURITY_SELINUX_DEVELOP
int selinux_enforcing = 0;
@@ -220,6 +221,8 @@ static int file_alloc_security(struct fi
fsec->file = file;
fsec->sid = tsec->sid;
+ fsec->tsid = tsec->sid;
+ fsec->pseqno = policy_seqno;
fsec->fown_sid = tsec->sid;
file->f_security = fsec;
@@ -2458,7 +2461,7 @@ static int selinux_inode_listsecurity(st
/* file security operations */
-static int selinux_file_permission(struct file *file, int mask)
+static int do_selinux_file_permission(struct file *file, int mask)
{
int rc;
struct inode *inode = file->f_path.dentry->d_inode;
@@ -2480,6 +2483,43 @@ 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;
+
Re: [RFC]selinux: Improving SELinux read/write performance
On Thu, 2007-09-06 at 16:27 +0900, Yuichi Nakamura wrote:
Hello.
As I posted before in selinux list,
I found big overhead of SELinux in read/write on some CPUs,
and trying tuning.
There were discussion in previous threads.
Part 1:
http://marc.info/?t=11884534341r=1w=2
Part 2:
http://marc.info/?t=11888074984r=1w=2
I would like to RFC again about this topic.
Thanks for your work on this, as this is clearly an important area to
improve.
1. Background
Look at benchmark result below.
lmbench simple read/write.
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 write1.00 1.14 14.0
* Base: kernel compiled without SELinux support
2) Result for SH(SH4, SH7751R), kernel 2.6.22
BaseSELinux Overhead(%)
Simple read 2.395.49 130.5
Simple write2.075.10 146.6
# This result is a little different from previous threads,
# because I changed some kernel configs.
Overhead more than 100%
I also found about 70-90% overhead in ARM.
2. About patch
I found a overhead in selinux_file_permission function.
This is a function that is called in read/write calls,
and does SELinux permission check.
SELinux checks permission both in open and read/write time.
Stephen Smalley sugessted that we can usually skip permission check
in selinux_file_permission.
By this patch,
permission check in selinux_file_permssion is done only when
- sid of task has changed after file open
- sid of inode has changed after file open
- policy load or boolean change happen after file open
To detect these changes,
I added entries in file_security struct and saving these values at file open.
And to save sid of inode at the time of file open,
I had to add new LSM hook in __dentry_open function.
3. Benchmark after applying this patch
1) Result for x86(Pentium 4 2.6Ghz), kernel 2.6.22
Base SELinux Overhead(%)
Simple read 1.10 1.12 1.6(Before 12.3)
Simple write1.00 1.03 3.6(Before 14.0)
2) Result for SH(SH4, SH7751R), kernel 2.6.22
BaseSELinux Overhead(%)
Simple read 2.392.65 11.1(Before 130.5)
Simple write2.072.28 10.5(Before 146.6)
Performance has improved a lot.
I want comments from community.
Signed-off-by: Yuichi Nakamura[EMAIL PROTECTED]
---
fs/open.c |5 +++
include/linux/security.h | 11 +++
security/selinux/avc.c|5 ++-
security/selinux/hooks.c | 54
+-
security/selinux/include/objsec.h |3 ++
5 files changed, 76 insertions(+), 2 deletions(-)
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
--- linux-2.6.22.orig/security/selinux/avc.c 2007-07-09 08:32:17.0
+0900
+++ linux-2.6.22/security/selinux/avc.c 2007-09-06 14:33:35.0
+0900
@@ -123,6 +123,7 @@ DEFINE_PER_CPU(struct avc_cache_stats, a
#endif
static struct avc_cache avc_cache;
+u32 policy_seqno = 0;
static struct avc_callback_node *avc_callbacks;
static struct kmem_cache *avc_node_cachep;
@@ -431,8 +432,10 @@ static int avc_latest_notif_update(int s
ret = -EAGAIN;
}
} else {
- if (seqno avc_cache.latest_notif)
+ if (seqno avc_cache.latest_notif) {
avc_cache.latest_notif = seqno;
+ policy_seqno = seqno;
+ }
I would have just provided an avc interface for obtaining the seqno,
e.g.
u32 avc_policy_seqno(void)
{
return avc_cache.latest_notif;
}
}
spin_unlock_irqrestore(notif_lock, flag);
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
--- linux-2.6.22.orig/security/selinux/hooks.c2007-07-09
08:32:17.0 +0900
+++ linux-2.6.22/security/selinux/hooks.c 2007-09-06 16:08:36.0
+0900
@@ -84,6 +84,7 @@
extern unsigned int policydb_loaded_version;
extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
extern int selinux_compat_net;
+extern u32 policy_seqno;
I think that they frown upon extern declarations in .c files (versus
in .h files), so we don't want to add more - we ultimately should clean
up what we presently have.
#ifdef CONFIG_SECURITY_SELINUX_DEVELOP
int selinux_enforcing = 0;
@@ -220,6 +221,8 @@ static int file_alloc_security(struct fi
fsec-file = file;
fsec-sid = tsec-sid;
+ fsec-tsid = tsec-sid;
I'm not sure why we need the separate field here, as fsec-sid already
holds the allocating task SID and doesn't change.
+ fsec-pseqno = policy_seqno;
Not sure that you want
