[RFC]Tuning selinux_file_permission
Yuichi Nakamura <
ynakam@...>
2007-09-03 08:04:46 GMT
Hi.
As I posted before,
I found big overhead in read/write on some CPUs.
http://marc.info/?t=118845343400001&r=1&w=2
I tried tuning based on manual inlining of selinux_file_permission,
and it works to some extent,
but Stephen suggested better idea.
Stephen Smalley wrote:
> I'd also like to separately explore a different approach for improving
> the overhead on read/write that has come up previously, namely don't
> recheck at all unless one of the following conditions is met:
> 1) process SID has changed since open-time check (i.e. exec with SID
> transition or setcon),
> 2) file SID has changed since open-time check (i.e. setxattr) ,
> 3) policy seqno has changed since open-time check (i.e. policy reload or
> boolean change).
I tried tuning of selinux_file_permission based on this idea.
I wrote a patch that shows the concept, and it can reduce much overhead.
I want comments from community.
1. Detail of patch
1) Prepared global serial number, u32 sid_serial.
It is initialized as 1,
is incremented when sid is changed in the system:
- policy is reloaded
- boolean is changed
- domain transition/setcon happend
- setxattr happend
2) Added "sid_serial" member to struct file_secuirty.
3) In file open, file_security->sid_serial is set as global sid_serial.
4) In file read/write, selinux_file_permission is called.
file_security->sid_serial and global sid_serial is compared
before permission check.
If sid_serial is not changed, permission check is skipped.
If it is different, this means some relabel could happen after file open,
then call do_selinux_file_permission and permission recheck happen.
5) If sid_serial is incremented too much,
and returned to zero.
permission recheck in selinux_file_permission is forced by notify_sid_serial_end().
This is to avoid following situation.
* sid_serial = 10 at file open
* sid_serial is incremented 2^32+10 times, then sid_serial returns to 10
* In file read, selinux_file_permission is called.
sid_serial is unchanged(=10), then permission check is skipped.
* sid may be changed, but check is skipped
2. Benchmark
lmbench simple read/write.
1) Result for x86(Pentium 4)
Base SELinux Overhead(%)
Simple read 1.1034 1.116 1.16(before 12.3)
Simple write 0.9989 1.018 1.97(before 14.0)
* Base = SELinux disabled kernel
2) Result for SH(SuperH, processor for embedded devices)
Base SELinux Overhead(%)
Simple read 2.6781 2.67 -0.37(before 141.5)
Simple write 2.0781 2.34 12.5(before 155.9)
Overhead is reduced a lot in both PC and embedded devices.
Below is a patch.
---
security/selinux/avc.c | 49 +++++++++++++++++++++++++
security/selinux/hooks.c | 74 +++++++++++++++++++++++++++++++++-----
security/selinux/include/avc.h | 4 ++
security/selinux/include/objsec.h | 2 +
security/selinux/selinuxfs.c | 3 +
5 files changed, 124 insertions(+), 8 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.000000000 +0900
+++ linux-2.6.22/security/selinux/avc.c 2007-09-03 14:22:22.000000000 +0900
<at> <at> -29,8 +29,10 <at> <at>
#include <linux/audit.h>
#include <linux/ipv6.h>
#include <net/ipv6.h>
+#include <linux/file.h>
#include "avc.h"
#include "avc_ss.h"
+#include "objsec.h"
static const struct av_perm_to_string av_perm_to_string[] = {
#define S_(c, v, s) { c, v, s },
<at> <at> -126,6 +128,15 <at> <at> static struct avc_cache avc_cache;
static struct avc_callback_node *avc_callbacks;
static struct kmem_cache *avc_node_cachep;
+/*This number is incremented when sids are changed
+ - policy reload
+ - boolean change
+ - domain transition
+ - setxattr
+*/
+static u32 sid_serial = 1;
+
+
static inline int avc_hash(u32 ssid, u32 tsid, u16 tclass)
{
return (ssid ^ (tsid<<2) ^ (tclass<<4)) & (AVC_CACHE_SLOTS - 1);
<at> <at> -913,3 +924,41 <at> <at> int avc_has_perm(u32 ssid, u32 tsid, u16
avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata);
return rc;
}
+
+/*
+ Notify all processes that sid_serial returned to zero.
+*/
+void notify_sid_serial_end()
+{
+ struct task_struct *p;
+ struct files_struct *files;
+ struct file *file;
+ struct file_security_struct *fsec;
+ int i;
+
+ /*Mutex is not considered yet!*/
+ for_each_process(p) {
+ files = p->files;
+ for (i = 0; i < atomic_read(&(files->count)); i++) {
+ file = files->fd_array[i];
+ if (file) {
+ fsec = file->f_security;
+ if (fsec)
+ fsec->force_file_check = 1;
+ }
+ }
+ }
+}
+
+void sid_serial_incr()
+{
+ sid_serial++;
+ if (sid_serial == 0)
+ notify_sid_serial_end();
+
+}
+
+u32 read_sid_serial()
+{
+ return sid_serial;
+}
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.000000000 +0900
+++ linux-2.6.22/security/selinux/hooks.c 2007-09-03 14:31:54.000000000 +0900
<at> <at> -220,7 +220,9 <at> <at> static int file_alloc_security(struct fi
fsec->file = file;
fsec->sid = tsec->sid;
+ fsec->force_file_check = 0;
fsec->fown_sid = tsec->sid;
+ fsec->sid_serial = read_sid_serial();
file->f_security = fsec;
return 0;
<at> <at> -991,6 +993,7 <at> <at> out_unlock:
out:
if (isec->sclass == SECCLASS_FILE)
isec->sclass = inode_mode_to_security_class(inode->i_mode);
+
return rc;
}
<at> <at> -1691,6 +1694,8 <at> <at> static int selinux_bprm_set_security(str
/* Set the security field to the new SID. */
bsec->sid = newsid;
+
+ sid_serial_incr();
}
bsec->set = 1;
<at> <at> -2289,7 +2294,7 <at> <at> static int selinux_inode_getattr(struct
return dentry_has_perm(current, mnt, dentry, FILE__GETATTR);
}
-static int selinux_inode_setxattr(struct dentry *dentry, char *name, void *value, size_t size, int flags)
+static inline int do_selinux_inode_setxattr(struct dentry *dentry, char *name, void *value, size_t
size, int flags)
{
struct task_security_struct *tsec = current->security;
struct inode *inode = dentry->d_inode;
<at> <at> -2348,6 +2353,17 <at> <at> static int selinux_inode_setxattr(struct
FILESYSTEM__ASSOCIATE,
&ad);
}
+static int selinux_inode_setxattr(struct dentry *dentry, char *name, void *value, size_t size, int flags)
+{
+ int rc;
+
+ rc = do_selinux_inode_setxattr(dentry, name, value, size, flags);
+ if (rc)
+ return rc;
+
+ sid_serial_incr();
+ return 0;
+}
static void selinux_inode_post_setxattr(struct dentry *dentry, char *name,
void *value, size_t size, int flags)
<at> <at> -2457,17 +2473,11 <at> <at> 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;
- if (!mask) {
- /* No permission to check. Existence test. */
- return 0;
- }
-
/* file_mask_to_av won't add FILE__WRITE if MAY_APPEND is set */
if ((file->f_flags & O_APPEND) && (mask & MAY_WRITE))
mask |= MAY_APPEND;
<at> <at> -2480,6 +2490,53 <at> <at> static int selinux_file_permission(struc
return selinux_netlbl_inode_permission(inode, mask);
}
+static int selinux_file_permission(struct file *file, int mask)
+{
+
+ struct task_security_struct *tsec = current->security;
+ struct file_security_struct *fsec = file->f_security;
+ int rc;
+ u32 current_sid_serial;
+
+ if (!mask) {
+ /* No permission to check. Existence test. */
+ return 0;
+ }
+
+ /*Check FS__USE*/
+ if (tsec->sid != fsec->sid) {
+ struct vfsmount *mnt = file->f_path.mnt;
+ struct dentry *dentry = file->f_path.dentry;
+ struct avc_audit_data ad;
+ 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;
+ }
+
+ /*Skip permission check
+ when sids are not changed after open*/
+ current_sid_serial = read_sid_serial();
+ if (fsec->sid_serial == current_sid_serial &&
+ !(fsec->force_file_check))
+ return 0;
+
+ rc = do_selinux_file_permission(file, mask);
+ if (rc)
+ return rc;
+
+ fsec->sid_serial = current_sid_serial;
+ fsec->force_file_check = 0;
+
+ return 0;
+}
+
+
static int selinux_file_alloc_security(struct file *file)
{
return file_alloc_security(file);
<at> <at> -4642,6 +4699,7 <at> <at> static int selinux_setprocattr(struct ta
else
return -EINVAL;
+ sid_serial_incr();
return size;
}
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.000000000 +0900
+++ linux-2.6.22/security/selinux/include/avc.h 2007-09-03 09:05:47.000000000 +0900
<at> <at> -127,6 +127,10 <at> <at> int avc_add_callback(int (*callback)(u32
/* Exported to selinuxfs */
int avc_get_hash_stats(char *page);
+
+void sid_serial_incr();
+u32 read_sid_serial();
+
extern unsigned int avc_cache_threshold;
#ifdef CONFIG_SECURITY_SELINUX_AVC_STATS
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
--- linux-2.6.22.orig/security/selinux/include/objsec.h 2007-07-09 08:32:17.000000000 +0900
+++ linux-2.6.22/security/selinux/include/objsec.h 2007-09-03 14:28:18.000000000 +0900
<at> <at> -53,6 +53,8 <at> <at> 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 sid_serial; /* sid_serial at open time*/
+ bool force_file_check; /* It is set when sid_serial returns zero*/
};
struct superblock_security_struct {
diff -purN -X linux-2.6.22/Documentation/dontdiff
linux-2.6.22.orig/security/selinux/selinuxfs.c linux-2.6.22/security/selinux/selinuxfs.c
--- linux-2.6.22.orig/security/selinux/selinuxfs.c 2007-07-09 08:32:17.000000000 +0900
+++ linux-2.6.22/security/selinux/selinuxfs.c 2007-09-03 09:10:38.000000000 +0900
<at> <at> -294,6 +294,7 <at> <at> static ssize_t sel_write_load(struct fil
audit_log(current->audit_context, GFP_KERNEL, AUDIT_MAC_POLICY_LOAD,
"policy loaded auid=%u",
audit_get_loginuid(current->audit_context));
+ sid_serial_incr();
out:
mutex_unlock(&sel_mutex);
vfree(data);
<at> <at> -872,6 +873,8 <at> <at> static ssize_t sel_write_bool(struct fil
bool_pending_values[inode->i_ino&SEL_INO_MASK] = new_value;
length = count;
+ sid_serial_incr();
+
out:
mutex_unlock(&sel_mutex);
if (page)
Regards,
--
--
Yuichi Nakamura
Hitachi Software Engineering Co., Ltd.
Japan SELinux Users Group(JSELUG): http://www.selinux.gr.jp/
SELinux Policy Editor: http://seedit.sourceforge.net/
) the awstats cgi script
was allowed to manage awstats_lib_t files which is _not_ necessary. I
allowed that because awstats_t executes the
httpd_awstats_script_exec_t to update the statistic files. But the
cgi script called from a web interface only needs read permissions so
I added an interface awstats_cgi_exec to handle this situation. Now
awstats_t executes httpd_awstats_script_exec_t in its domain which is
the preferred and more secure way.
In the end the untrustworthy cgi script has only read access to
awstats_lib_t and awstats_t which is called from a trustworthy cron-
job has write permissions to awstats_lib_t
Please substitute the old ("insecure") awstats policy with the new one.
RSS Feed