SLIM is an LSM module which provides an enhanced low water-mark
integrity and high water-mark secrecy mandatory access control
model.
For simplicity, this version:
- does not require stacker
- uses the integrity service (dummy provider)
Major fixes based on prior review comments:
- more cleanup based on review comments
- now only supports builtin
- more locking cleanups
- cleanups in the demotion code
SLIM now performs a generic revocation operation, including revoking
mmap and shared memory access. Note that during demotion or promotion
of a process, SLIM needs only revoke write access to files with higher
integrity, or lower secrecy. Read and execute permissions are blocked
as needed, not revoked. SLIM hopefully uses d_instantiate correctly now.
SLIM inherently deals with dynamic labels, which is a feature not
currently available in selinux. While it might be possible to
add support for this to selinux, it would not appear to be simple,
and it is not clear if the added complexity would be desirable
just to support this one model. (Isn't choice what LSM is all about? :-)
Comments on the model:
Some of the prior comments questioned the usefulness of the
low water-mark model itself. Two major questions raised concerned
a potential progression of the entire system to a fully demoted
state, and the security issues surrounding the guard processes.
In normal operation, the system seems to stabilize with a roughly
equal mixture of SYSTEM, USER, and UNTRUSTED processes. Most
applications seem to do a fixed set of operations in a fixed domain,
and stabilize at their appropriate level. Some applications, like
firefox and evolution, which inherently deal with untrusted data,
immediately go to the UNTRUSTED level, which is where they belong.
In a couple of cases, including cups and Notes, the applications
did not handle their demotions well, as they occured well into their
startup. For these applications, we simply force them to start up
as UNTRUSTED, so demotion is not an issue. The one application
that does tend to get demoted over time are shells, such as bash.
These are not problems, as new ones can be created with the
windowing system, or with su, as needed. To help with the associated
user interface issue, the user space package README shows how to
display the SLIM level in window titles, so it is always clear at
what level the process is currently running.
As for the issue of guard processes, SLIM defines three types of
guard processes: Unlimited Guards, Limited Guards, and Untrusted
Guards. Unlimited Guards are the most security sensitive, as they
allow less trusted process to acquire a higher level of trust.
On my current system there are two unlimited guards, passwd and
userhelper. These two applications inherently have to be trusted
this way regardless of the MAC model used. In SLIM, the policy
clearly and simply labels them as having this level of trust.
Limited Guards are programs which cannot give away higher
trust, but which can keep their existing level despite reading
less trusted data. On my system I have seven limited guards:
yum, which is trusted to verify the signature on an (untrusted)
downloaded RPM file, and to install it, login and sshd, which read
untrusted user supplied login data, for authentication, dhclient
which reads untrusted network data, and updates they system
file /etc/resolv.conf, dbus-daemon, which accepts data from
potentially untrusted processes, Xorg, which has to accept data
from all Xwindow clients, regardless of level, and postfix which
delivers untrusted mail. Again, these applications inherently
must cross trust levels, and SLIM properly identifies them.
As mentioned earlier, cupsd and notes are applications which are
always run directly in untrusted mode, regardless of the level of
the invoking process.
The bottom line is that SLIM guard programs inherently do security
sensitive things, and have to be trusted. There are only a small
number of them, and they are clearly identified by their labels.
Signed-off-by: Mimi Zohar <[email protected]>
Signed-off-by: Kylene Hall <[email protected]>
---
security/slim/slm_main.c | 1378 +++++++++++++++++++++++++++++++++++++
1 files changed, 1377 insertions(+)
--- linux-2.6.18/security/slim/slm_main.c 1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.17-working/security/slim/slm_main.c 2006-09-06 11:49:09.000000000 -0700
@@ -0,0 +1,1378 @@
+/*
+ * SLIM - Simple Linux Integrity Module
+ *
+ * Copyright (C) 2005,2006 IBM Corporation
+ * Author: Mimi Zohar <[email protected]>
+ * Kylene Hall <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/mman.h>
+#include <linux/config.h>
+#include <linux/kernel.h>
+#include <linux/security.h>
+#include <linux/integrity.h>
+#include <linux/proc_fs.h>
+#include <linux/socket.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+#include <linux/namei.h>
+#include <linux/mm.h>
+#include <linux/shm.h>
+#include <linux/ipc.h>
+#include <linux/errno.h>
+#include <linux/xattr.h>
+#include <net/sock.h>
+
+#include "slim.h"
+
+#define XATTR_NAME "security.slim.level"
+
+#define ZERO_STR "0"
+#define UNTRUSTED_STR "UNTRUSTED"
+#define USER_STR "USER"
+#define SYSTEM_STR "SYSTEM"
+
+char *slm_iac_str[] = {
+ ZERO_STR,
+ UNTRUSTED_STR,
+ USER_STR,
+ SYSTEM_STR
+};
+
+static char *get_token(char *buf_start, char *buf_end, char delimiter,
+ int *token_len)
+{
+ char *bufp = buf_start;
+ char *token = NULL;
+
+ while (!token && (bufp < buf_end)) { /* Get start of token */
+ switch (*bufp) {
+ case ' ':
+ case '\n':
+ case '\t':
+ bufp++;
+ break;
+ case '#':
+ while ((*bufp != '\n') && (bufp++ < buf_end)) ;
+ bufp++;
+ break;
+ default:
+ token = bufp;
+ break;
+ }
+ }
+ if (!token)
+ return NULL;
+
+ *token_len = 0;
+ while ((*token_len == 0) && (bufp <= buf_end)) {
+ if ((*bufp == delimiter) || (*bufp == '\n'))
+ *token_len = bufp - token;
+ if (bufp == buf_end)
+ *token_len = bufp - token;
+ bufp++;
+ }
+ if (*token_len == 0)
+ token = NULL;
+ return token;
+}
+
+static int is_guard_integrity(struct slm_file_xattr *level)
+{
+ if ((level->guard.iac_r != SLM_IAC_NOTDEFINED)
+ && (level->guard.iac_wx != SLM_IAC_NOTDEFINED))
+ return 1;
+ return 0;
+}
+
+static int is_lower_integrity(struct slm_file_xattr *task_level,
+ struct slm_file_xattr *obj_level)
+{
+ if (task_level->iac_level < obj_level->iac_level)
+ return 1;
+ return 0;
+}
+static int is_isec_defined(struct slm_isec_data *isec)
+{
+ if (isec && isec->level.iac_level != SLM_IAC_NOTDEFINED)
+ return 1;
+ return 0;
+}
+
+/*
+ * Called with current->files->file_lock. There is not a great lock to grab
+ * for demotion of this type. The only place f_mode is changed after install
+ * is in mark_files_ro in the filesystem code. That function is also changing
+ * taking away write rights so even if we race the outcome is the same.
+ */
+static inline int mark_has_file_wperm(struct file *file,
+ struct slm_file_xattr *cur_level)
+{
+ struct inode *inode;
+ struct slm_isec_data *isec;
+ int rc = 0;
+
+ inode = file->f_dentry->d_inode;
+ if (!S_ISREG(inode->i_mode) || !(file->f_mode & FMODE_WRITE))
+ return 0;
+
+ isec = inode->i_security;
+ spin_lock(&isec->lock);
+ if (is_lower_integrity(cur_level, &isec->level))
+ rc = 1;
+ spin_unlock(&isec->lock);
+ return rc;
+}
+
+/*
+ * Determine if a file is opened with write permissions.
+ */
+static int has_file_wperm(struct slm_file_xattr *cur_level)
+{
+ int i, j = 0;
+ struct files_struct *files = current->files;
+ unsigned long fd = 0;
+ struct fdtable *fdt;
+ struct file *file;
+ int rc = 0;
+
+ if (is_kernel_thread(current))
+ return 0;
+
+ if (!files || !cur_level)
+ return 0;
+
+ spin_lock(&files->file_lock);
+ fdt = files_fdtable(files);
+
+ for (;;) {
+ i = j * __NFDBITS;
+ if (i >= fdt->max_fdset || i >= fdt->max_fds)
+ break;
+ fd = fdt->open_fds->fds_bits[j++];
+ while (fd) {
+ if (fd & 1) {
+ file = fdt->fd[i++];
+ if (file)
+ rc = mark_has_file_wperm(file,
+ cur_level);
+ }
+ fd >>= 1;
+ }
+ }
+ spin_unlock(&files->file_lock);
+ return rc;
+}
+
+static inline void do_revoke_mmap_wperm(struct vm_area_struct *mpnt,
+ struct slm_isec_data *isec,
+ struct slm_file_xattr *cur_level)
+{
+ unsigned long start = mpnt->vm_start;
+ unsigned long end = mpnt->vm_end;
+ size_t len = end - start;
+
+ if ((mpnt->vm_flags & (VM_WRITE | VM_MAYWRITE))
+ && (mpnt->vm_flags & VM_SHARED)
+ && (cur_level->iac_level < isec->level.iac_level))
+ do_mprotect(start, len, PROT_READ);
+}
+
+/*
+ * Revoke write permission to underlying mmap file (MAP_SHARED)
+ */
+static void revoke_mmap_wperm(struct slm_file_xattr *cur_level)
+{
+ struct vm_area_struct *mpnt;
+ struct file *file;
+ struct dentry *dentry;
+ struct slm_isec_data *isec;
+
+ flush_cache_mm(current->mm);
+
+ down_write(¤t->mm->mmap_sem);
+ for (mpnt = current->mm->mmap; mpnt; mpnt = mpnt->vm_next) {
+ file = mpnt->vm_file;
+ if (!file)
+ continue;
+
+ dentry = file->f_dentry;
+ if (!dentry || !dentry->d_inode)
+ continue;
+
+ isec = dentry->d_inode->i_security;
+ do_revoke_mmap_wperm(mpnt, isec, cur_level);
+ }
+ up_write(¤t->mm->mmap_sem);
+}
+
+/*
+ * revoke write permission for shared memory for demoted threads
+ */
+static void revoke_permissions(struct slm_file_xattr *cur_level)
+{
+ if (!is_kernel_thread(current))
+ revoke_mmap_wperm(cur_level);
+}
+
+#define EXEMPT_STR "EXEMPT"
+static enum slm_iac_level parse_iac(char *token)
+{
+ int iac;
+
+ if (strncmp(token, EXEMPT_STR, strlen(EXEMPT_STR)) == 0)
+ return SLM_IAC_EXEMPT;
+ for (iac = 0; iac < sizeof(slm_iac_str) / sizeof(char *); iac++) {
+ if (strncmp(token, slm_iac_str[iac], strlen(slm_iac_str[iac]))
+ == 0)
+ return iac;
+ }
+ return SLM_IAC_ERROR;
+}
+
+#define UNLIMITED_STR "UNLIMITED"
+static inline int set_bounds(char *token)
+{
+ if (strncmp(token, UNLIMITED_STR, strlen(UNLIMITED_STR)) == 0)
+ return 1;
+ return 0;
+}
+
+/*
+ * Get the 7 access class levels from the extended attribute
+ * Format: INTEGRITY [INTEGRITY_GUARD INTEGRITY_GUARD] [GUARD_TYPE]
+ */
+static int slm_parse_xattr(char *xattr_value, int xattr_len,
+ struct slm_file_xattr *level)
+{
+ char *token;
+ int token_len;
+ char *buf, *buf_end;
+ int fieldno = 0;
+ int rc = 0;
+
+ buf = xattr_value;
+ buf_end = xattr_value + xattr_len;
+
+ while ((token = get_token(buf, buf_end, ' ', &token_len)) != NULL) {
+ buf = token + token_len;
+ switch (++fieldno) {
+ case 1:
+ level->iac_level = parse_iac(token);
+ if (level->iac_level == SLM_IAC_ERROR) {
+ rc = -EINVAL;
+ level->iac_level = SLM_IAC_UNTRUSTED;
+ }
+ break;
+ case 2:
+ level->guard.iac_r = parse_iac(token);
+ if (level->guard.iac_r == SLM_IAC_ERROR) {
+ rc = -EINVAL;
+ level->iac_level = SLM_IAC_UNTRUSTED;
+ }
+ break;
+ case 3:
+ level->guard.iac_wx = parse_iac(token);
+ if (level->guard.iac_wx == SLM_IAC_ERROR) {
+ rc = -EINVAL;
+ level->iac_level = SLM_IAC_UNTRUSTED;
+ }
+ break;
+ case 4:
+ level->guard.unlimited = set_bounds(token);
+ default:
+ break;
+ }
+ }
+ return rc;
+}
+
+/*
+ * Possible return codes: INTEGRITY_PASS, INTEGRITY_FAIL, INTEGRITY_NOLABEL,
+ * or -EINVAL
+ */
+static int slm_get_xattr(struct dentry *dentry,
+ struct slm_file_xattr *level, int *status)
+{
+ int xattr_len;
+ char *xattr_value = NULL;
+ int rc;
+
+ memset(level, 0, sizeof(struct slm_file_xattr));
+ rc = integrity_verify_metadata(dentry, XATTR_NAME,
+ &xattr_value, &xattr_len, status);
+
+ if (rc >=0 && *status == INTEGRITY_PASS && xattr_value) {
+ rc = slm_parse_xattr(xattr_value, xattr_len, level);
+ kfree(xattr_value);
+ if (rc == 0 && level->iac_level != SLM_IAC_UNTRUSTED)
+ rc = integrity_verify_data(dentry, status);
+ }
+ return rc;
+}
+
+/* Caller responsible for necessary locking */
+static inline void set_level(struct slm_file_xattr *level,
+ enum slm_iac_level iac)
+{
+ level->iac_level = iac;
+}
+static inline void set_level_exempt(struct slm_file_xattr *level)
+{
+ set_level(level, SLM_IAC_EXEMPT);
+}
+
+static inline void set_level_untrusted(struct slm_file_xattr *level)
+{
+ set_level(level, SLM_IAC_UNTRUSTED);
+}
+
+static inline void set_level_tsec_write(struct slm_file_xattr *level,
+ struct slm_tsec_data *tsec)
+{
+ set_level(level, tsec->iac_wx);
+}
+
+static inline void set_level_tsec_read(struct slm_file_xattr *level,
+ struct slm_tsec_data *tsec)
+{
+ set_level(level, tsec->iac_r);
+}
+
+static void update_sock_level(struct dentry *dentry, struct slm_file_xattr *level)
+{
+ struct slm_tsec_data *cur_tsec = current->security;
+ int rc, status = 0;
+
+ rc = slm_get_xattr(dentry, level, &status);
+ if (rc == -EOPNOTSUPP)
+ set_level_exempt(level);
+ else
+ set_level_tsec_read(level, cur_tsec);
+}
+
+static void update_level(struct dentry *dentry, struct slm_file_xattr *level)
+{
+ int rc, status = 0;
+
+ rc = slm_get_xattr(dentry, level, &status);
+ if (rc < 0) {
+ switch (rc) {
+ case -EOPNOTSUPP:
+ set_level_exempt(level);
+ break;
+ case -EINVAL: /* improperly formatted */
+ default:
+ set_level_untrusted(level);
+ break;
+ }
+ } else {
+ switch(status) {
+ case INTEGRITY_FAIL:
+ case INTEGRITY_NOLABEL:
+ set_level_untrusted(level);
+ break;
+ }
+ }
+}
+
+static struct slm_isec_data *slm_alloc_security(gfp_t flags)
+{
+ struct slm_isec_data *isec;
+
+ isec = kzalloc(sizeof(struct slm_isec_data), flags);
+ if (!isec)
+ return NULL;
+
+ spin_lock_init(&isec->lock);
+ return isec;
+}
+
+/*
+ * Exempt objects without extended attribute support
+ * for fastpath. Others will be handled generically
+ * by the other functions.
+ */
+static int is_exempt_fastpath(struct inode *inode)
+{
+ if ((inode->i_sb->s_magic == PROC_SUPER_MAGIC)
+ || S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode))
+ return 1;
+ return 0;
+}
+
+/*
+ * All directories with xattr support should be labeled, but just in case
+ * recursively traverse path (dentry->parent) until level info is found.
+ */
+static void slm_get_level(struct dentry *dentry, struct slm_file_xattr *level)
+{
+ struct inode *inode = dentry->d_inode;
+ struct slm_isec_data *isec = inode->i_security;
+
+ if (is_isec_defined(isec)) {
+ spin_lock(&isec->lock);
+ memcpy(level, &isec->level, sizeof(struct slm_file_xattr));
+ spin_unlock(&isec->lock);
+ return;
+ }
+
+ if (is_exempt_fastpath(inode)) {
+ memset(level, 0, sizeof(struct slm_file_xattr));
+ set_level_exempt(level);
+ } else if (S_ISSOCK(inode->i_mode))
+ update_sock_level(dentry, level);
+ else
+ update_level(dentry, level);
+
+ spin_lock(&isec->lock);
+ memcpy(&isec->level, level, sizeof(struct slm_file_xattr));
+ spin_unlock(&isec->lock);
+}
+
+/*
+ * new tsk->security inherits from current->security
+ */
+static struct slm_tsec_data *slm_init_task(struct task_struct *tsk, gfp_t flags)
+{
+ struct slm_tsec_data *tsec, *cur_tsec = current->security;
+
+ tsec = kzalloc(sizeof(struct slm_tsec_data), flags);
+ if (!tsec)
+ return NULL;
+ tsec->lock = SPIN_LOCK_UNLOCKED;
+ if (!cur_tsec) {
+ tsec->iac_r = SLM_IAC_HIGHEST - 1;
+ tsec->iac_wx = SLM_IAC_HIGHEST - 1;
+ } else
+ memcpy(tsec, cur_tsec, sizeof(struct slm_tsec_data));
+
+ return tsec;
+}
+
+static int is_iac_level_exempt(struct slm_file_xattr *level)
+{
+ if (level->iac_level == SLM_IAC_EXEMPT)
+ return 1;
+ return 0;
+}
+
+static int is_iac_less_than_or_exempt(struct slm_file_xattr *level,
+ enum slm_iac_level iac)
+{
+ if (iac <= level->iac_level)
+ return 1;
+ return is_iac_level_exempt(level);
+}
+
+static int is_iac_greater_than_or_exempt(struct slm_file_xattr *level,
+ enum slm_iac_level iac)
+{
+ if (iac >= level->iac_level)
+ return 1;
+ return is_iac_level_exempt(level);
+}
+
+/*
+ * enforce: IRAC(process) <= IAC(object)
+ * Permit process to read file of equal or greater integrity
+ * otherwise, demote the process.
+ */
+static int enforce_integrity_read(struct slm_file_xattr *level)
+{
+ struct slm_tsec_data *cur_tsec = current->security;
+ int rc = 0;
+
+ spin_lock(&cur_tsec->lock);
+ if (!is_iac_less_than_or_exempt(level, cur_tsec->iac_r)) {
+ rc = has_file_wperm(level);
+ if (atomic_read(¤t->mm->mm_users) != 1)
+ rc = 1;
+ if (rc)
+ spin_unlock(&cur_tsec->lock);
+ else {
+ /* Reading lower integrity, demote process */
+ /* Even in the case of a integrity guard process. */
+ cur_tsec->iac_r = level->iac_level;
+ cur_tsec->iac_wx = level->iac_level;
+ spin_unlock(&cur_tsec->lock);
+ revoke_permissions(level);
+ }
+ return rc ? -EACCES : 0;
+ }
+ spin_unlock(&cur_tsec->lock);
+ return 0;
+}
+
+static int do_task_may_read(struct slm_file_xattr *level)
+{
+ return enforce_integrity_read(level);
+}
+
+/*
+ * enforce: IWXAC(process) >= IAC(object)
+ * Permit process to write a file of equal or lesser integrity.
+ */
+static int enforce_integrity_write(struct slm_file_xattr *level)
+{
+ struct slm_tsec_data *cur_tsec = current->security;
+ int rc = 0;
+
+ spin_lock(&cur_tsec->lock);
+ if (!(is_iac_greater_than_or_exempt(level, cur_tsec->iac_wx)
+ || (level->iac_level == SLM_IAC_NOTDEFINED)))
+ /* can't write higher integrity */
+ rc = -EACCES;
+ spin_unlock(&cur_tsec->lock);
+ return rc;
+}
+
+static int do_task_may_write(struct slm_file_xattr *level)
+{
+ return enforce_integrity_write(level);
+}
+
+static int slm_set_taskperm(int mask, struct slm_file_xattr *level)
+{
+ int rc = 0;
+
+ if (mask & MAY_READ)
+ rc = do_task_may_read(level);
+ if ((mask & MAY_WRITE) || (mask & MAY_APPEND))
+ rc |= do_task_may_write(level);
+
+ return rc;
+}
+
+/*
+ * file changes invalidate isec
+ */
+static int slm_file_permission(struct file *file, int mask)
+{
+ struct slm_isec_data *isec = file->f_dentry->d_inode->i_security;
+
+ if (((mask & MAY_WRITE) || (mask & MAY_APPEND)) && isec) {
+ spin_lock(&isec->lock);
+ isec->level.iac_level = SLM_IAC_NOTDEFINED;
+ spin_unlock(&isec->lock);
+ }
+ return 0;
+}
+
+static int is_untrusted_blk_access(struct inode *inode)
+{
+ struct slm_tsec_data *cur_tsec = current->security;
+ int rc = 0;
+
+ spin_lock(&cur_tsec->lock);
+ if (cur_tsec && (cur_tsec->iac_wx == SLM_IAC_UNTRUSTED)
+ && S_ISBLK(inode->i_mode))
+ rc = 1;
+ spin_unlock(&cur_tsec->lock);
+ return rc;
+}
+
+/*
+ * Premise:
+ * Can't write or execute higher integrity, can't read lower integrity
+ * Can't read or execute higher secrecy, can't write lower secrecy
+ */
+static int slm_inode_permission(struct inode *inode, int mask,
+ struct nameidata *nd)
+{
+ struct dentry *dentry = NULL;
+ struct slm_file_xattr level;
+
+ if (S_ISDIR(inode->i_mode) && (mask & MAY_WRITE))
+ return 0;
+
+ dentry = (!nd || !nd->dentry) ? d_find_alias(inode) : nd->dentry;
+ if (!dentry)
+ return 0;
+
+ if (is_untrusted_blk_access(inode))
+ return -EPERM;
+
+ slm_get_level(dentry, &level);
+
+ /* measure all SYSTEM level integrity objects */
+ if (level.iac_level == SLM_IAC_SYSTEM)
+ integrity_measure(dentry, NULL, mask);
+
+ return slm_set_taskperm(mask, &level);
+}
+
+/*
+ * This hook is called holding the inode mutex.
+ */
+static int slm_inode_unlink(struct inode *dir, struct dentry *dentry)
+{
+ struct slm_file_xattr level;
+
+ if (!dentry || !dentry->d_name.name)
+ return 0;
+
+ slm_get_level(dentry, &level);
+ return slm_set_taskperm(MAY_WRITE, &level);
+}
+
+static void slm_inode_free_security(struct inode *inode)
+{
+ struct slm_isec_data *isec = inode->i_security;
+
+ inode->i_security = NULL;
+ kfree(isec);
+}
+
+/*
+ * Check integrity permission to create a regular file.
+ */
+static int slm_inode_create(struct inode *parent_dir,
+ struct dentry *dentry, int mask)
+{
+ struct slm_tsec_data *cur_tsec = current->security;
+ struct slm_isec_data *parent_isec = parent_dir->i_security;
+ struct slm_file_xattr *parent_level = &parent_isec->level;
+ int rc = 0;
+
+ /*
+ * enforce: IWXAC(process) >= IAC(object)
+ * Permit process to write a file of equal or lesser integrity.
+ */
+ spin_lock(&cur_tsec->lock);
+ spin_lock(&parent_isec->lock);
+ if (!is_iac_greater_than_or_exempt(parent_level, cur_tsec->iac_wx))
+ rc = -EPERM;
+ spin_unlock(&parent_isec->lock);
+ spin_unlock(&cur_tsec->lock);
+ return rc;
+}
+
+#define MAX_XATTR_SIZE 76
+
+static int slm_set_xattr(struct slm_file_xattr *level,
+ char **name, void **value, size_t * value_len)
+{
+ int len;
+ int xattr_len;
+ char buf[MAX_XATTR_SIZE];
+ char *bufp = buf;
+ char *xattr_val = buf;
+ char *xattr_name;
+
+ if (!level)
+ return 0;
+
+ memset(buf, 0, sizeof(buf));
+
+ if (is_iac_level_exempt(level)) {
+ memcpy(bufp, EXEMPT_STR, strlen(EXEMPT_STR));
+ bufp += strlen(EXEMPT_STR);
+ } else {
+ len = strlen(slm_iac_str[level->iac_level]);
+ memcpy(bufp, slm_iac_str[level->iac_level], len);
+ bufp += len;
+ }
+ *bufp++ = ' ';
+ xattr_len = bufp - buf;
+
+ /* point after 'security.' */
+ xattr_name = strchr(XATTR_NAME, '.');
+ if (xattr_name)
+ *name = kstrdup(xattr_name + 1, GFP_KERNEL);
+ *value = kmalloc(xattr_len + 1, GFP_KERNEL);
+ if (!*value) {
+ kfree(name);
+ return -ENOMEM;
+ }
+ memcpy(*value, xattr_val, xattr_len);
+ *value_len = xattr_len;
+ return 0;
+}
+
+/* Create the security.slim.level extended attribute */
+static int slm_inode_init_security(struct inode *inode, struct inode *dir,
+ char **name, void **value, size_t * len)
+{
+ struct slm_isec_data *isec = inode->i_security, *parent_isec =
+ dir->i_security;
+ struct slm_tsec_data *cur_tsec = current->security;
+ struct slm_file_xattr level;
+ struct xattr_data *data;
+ int rc;
+
+ if (!name || !value || !len)
+ return 0;
+
+ memset(&level, 0, sizeof(struct slm_file_xattr));
+
+ if (is_isec_defined(parent_isec)) {
+ spin_lock(&parent_isec->lock);
+ memcpy(&level, &parent_isec->level,
+ sizeof(struct slm_file_xattr));
+ spin_unlock(&parent_isec->lock);
+ }
+
+ spin_lock(&cur_tsec->lock);
+ /* low integrity process wrote into a higher level directory */
+ if (cur_tsec->iac_wx < level.iac_level)
+ set_level_tsec_write(&level, cur_tsec);
+ /* if directory is exempt, then use process level */
+ if (is_iac_level_exempt(&level)) {
+ /* When a guard process creates a directory */
+ if (S_ISDIR(inode->i_mode)
+ && (cur_tsec->iac_wx != cur_tsec->iac_r))
+ set_level_exempt(&level);
+ else
+ set_level_tsec_write(&level, cur_tsec);
+ }
+
+ /* if a guard process creates a UNIX socket, then EXEMPT it */
+ if (S_ISSOCK(inode->i_mode)
+ && (cur_tsec->iac_wx != cur_tsec->iac_r))
+ set_level_exempt(&level);
+ spin_unlock(&cur_tsec->lock);
+
+ spin_lock(&isec->lock);
+ memcpy(&isec->level, &level, sizeof(struct slm_file_xattr));
+ spin_unlock(&isec->lock);
+
+ data = kmalloc(sizeof(struct xattr_data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ /* set levels, based on parent */
+ rc = slm_set_xattr(&level, &data->name, &data->value, &data->len);
+ if (rc < 0) {
+ kfree(data);
+ return rc;
+ }
+
+ *name = data->name;
+ *value = data->value;
+ *len = data->len;
+ return 0;
+}
+
+static void slm_d_instantiate(struct dentry *dentry, struct inode *inode)
+{
+ struct slm_isec_data *isec;
+ struct slm_file_xattr level;
+
+ if (!inode)
+ return;
+
+ isec = inode->i_security;
+ if (is_exempt_fastpath(inode)) {
+ memset(&level, 0, sizeof(struct slm_file_xattr));
+ set_level_exempt(&level);
+ } else if (S_ISSOCK(inode->i_mode))
+ memset(&level, 0, sizeof(struct slm_file_xattr));
+ else
+ update_level(dentry, &level);
+
+ spin_lock(&isec->lock);
+ memcpy(&isec->level, &level, sizeof(struct slm_file_xattr));
+ spin_unlock(&isec->lock);
+}
+
+/*
+ * Check permissions to create a new directory in the existing directory
+ * associated with inode structure @dir.
+ */
+static int slm_inode_mkdir(struct inode *parent_dir,
+ struct dentry *dentry, int mask)
+{
+ struct slm_tsec_data *cur_tsec = current->security;
+ struct slm_isec_data *parent_isec = parent_dir->i_security;
+ struct slm_file_xattr *parent_level = &parent_isec->level;
+ int rc = 0;
+
+ spin_lock(&cur_tsec->lock);
+ spin_lock(&parent_isec->lock);
+ if (cur_tsec->iac_wx < parent_level->iac_level
+ && parent_level->iac_level == SLM_IAC_SYSTEM)
+ rc = -EACCES;
+ spin_unlock(&parent_isec->lock);
+ spin_unlock(&cur_tsec->lock);
+ return rc;
+}
+
+static int slm_inode_rename(struct inode *old_dir,
+ struct dentry *old_dentry,
+ struct inode *new_dir, struct dentry *new_dentry)
+{
+ struct slm_file_xattr old_level, parent_level;
+ struct dentry *parent_dentry;
+
+ if (old_dir == new_dir)
+ return 0;
+
+ slm_get_level(old_dentry, &old_level);
+
+ parent_dentry = dget_parent(new_dentry);
+ slm_get_level(parent_dentry, &parent_level);
+ dput(parent_dentry);
+
+ if (is_lower_integrity(&old_level, &parent_level))
+ return -EPERM;
+ return 0;
+}
+
+/*
+ * Limit the integrity value of an object to be no greater than that
+ * of the current process. This is especially important for objects
+ * being promoted.
+*/
+int slm_inode_setxattr(struct dentry *dentry, char *name, void *value,
+ size_t size, int flags)
+{
+ struct slm_tsec_data *cur_tsec = current->security;
+ char *data = value;
+ enum slm_iac_level iac;
+
+ if (strncmp(name, XATTR_NAME, strlen(XATTR_NAME)) != 0)
+ return 0;
+
+ if (!value)
+ return -EINVAL;
+
+ spin_lock(&cur_tsec->lock);
+ iac = cur_tsec->iac_wx;
+ spin_unlock(&cur_tsec->lock);
+
+ switch (iac) {
+ case SLM_IAC_USER:
+ if ((strncmp(data, USER_STR, strlen(USER_STR)) != 0) &&
+ (strncmp(data, UNTRUSTED_STR, strlen(UNTRUSTED_STR)) != 0))
+ return -EPERM;
+ break;
+ case SLM_IAC_SYSTEM:
+ if ((strncmp(data, SYSTEM_STR, strlen(SYSTEM_STR)) != 0) &&
+ (strncmp(data, USER_STR, strlen(USER_STR)) != 0) &&
+ (strncmp(data, UNTRUSTED_STR, strlen(UNTRUSTED_STR)) != 0)
+ && (strncmp(data, EXEMPT_STR, strlen(EXEMPT_STR)) != 0))
+ return -EPERM;
+ break;
+ default:
+ return -EPERM;
+ }
+ return 0;
+}
+
+/*
+ * SLIM extended attribute was modified, update isec.
+ */
+static void slm_inode_post_setxattr(struct dentry *dentry, char *name,
+ void *value, size_t size, int flags)
+{
+ struct slm_isec_data *slm_isec;
+ struct slm_file_xattr level;
+ int rc, status = 0;
+
+ if (strncmp(name, XATTR_NAME, strlen(XATTR_NAME)) != 0)
+ return;
+
+ rc = slm_get_xattr(dentry, &level, &status);
+ slm_isec = dentry->d_inode->i_security;
+ spin_lock(&slm_isec->lock);
+ memcpy(&slm_isec->level, &level, sizeof(struct slm_file_xattr));
+ spin_unlock(&slm_isec->lock);
+}
+
+static int slm_inode_removexattr(struct dentry *dentry, char *name)
+{
+ struct slm_isec_data *isec = dentry->d_inode->i_security;
+ struct slm_tsec_data *tsec = current->security;
+ enum slm_iac_level iac;
+ int rc = 0;
+
+ if (strncmp(name, XATTR_NAME, strlen(XATTR_NAME)) != 0)
+ return 0;
+
+ if (isec) {
+ spin_lock(&tsec->lock);
+ iac = tsec->iac_wx;
+ spin_unlock(&tsec->lock);
+
+ spin_lock(&isec->lock);
+ switch(iac) {
+ case SLM_IAC_SYSTEM:
+ isec->level.iac_level = SLM_IAC_NOTDEFINED;
+ break;
+ case SLM_IAC_USER:
+ if (isec->level.iac_level < SLM_IAC_NOTDEFINED ||
+ isec->level.iac_level > SLM_IAC_USER)
+ rc = -EPERM;
+ else
+ isec->level.iac_level = SLM_IAC_NOTDEFINED;
+ break;
+ default:
+ rc = -EPERM;
+ }
+ spin_unlock(&isec->lock);
+ }
+ return rc;
+}
+
+static int slm_inode_alloc_security(struct inode *inode)
+{
+ struct slm_isec_data *isec = slm_alloc_security(GFP_KERNEL);
+ if (!isec)
+ return -ENOMEM;
+
+ inode->i_security = isec;
+ return 0;
+}
+
+/*
+ * Opening a socket demotes the integrity of a process to untrusted.
+ */
+int slm_socket_create(int family, int type, int protocol, int kern)
+{
+ struct task_struct *parent_tsk;
+ struct slm_tsec_data *cur_tsec = current->security, *parent_tsec;
+ struct slm_file_xattr level;
+ int rc = 0;
+
+ /* demoting all but UNIX and NETLINK sockets */
+ if ((family != AF_UNIX) && (family != AF_NETLINK)) {
+ spin_lock(&cur_tsec->lock);
+ if (cur_tsec->iac_r > SLM_IAC_UNTRUSTED) {
+ parent_tsk = current->parent;
+ parent_tsec = parent_tsk->security;
+ memset(&level, 0, sizeof(struct slm_file_xattr));
+ level.iac_level = SLM_IAC_UNTRUSTED;
+ rc = has_file_wperm(&level);
+ if (atomic_read(¤t->mm->mm_users) != 1)
+ rc = 1;
+ if (rc) {
+ spin_unlock(&cur_tsec->lock);
+ return -EPERM;
+ } else {
+ cur_tsec->iac_r = SLM_IAC_UNTRUSTED;
+ cur_tsec->iac_wx = SLM_IAC_UNTRUSTED;
+ spin_unlock(&cur_tsec->lock);
+
+ revoke_permissions(&level);
+ return 0;
+ }
+ }
+ spin_unlock(&cur_tsec->lock);
+ }
+ return 0;
+}
+
+/*
+ * Didn't have the family type previously, so update the inode security now.
+ */
+static void slm_socket_post_create(struct socket *sock, int family,
+ int type, int protocol, int kern)
+{
+ struct slm_tsec_data *cur_tsec = current->security;
+ struct inode *inode = SOCK_INODE(sock);
+ struct slm_isec_data *slm_isec = inode->i_security;
+
+ spin_lock(&slm_isec->lock);
+ if (family == PF_UNIX) {
+ if (cur_tsec->iac_wx != cur_tsec->iac_r) /* guard process */
+ set_level_exempt(&slm_isec->level);
+ else
+ set_level_tsec_write(&slm_isec->level, cur_tsec);
+ } else
+ set_level_untrusted(&slm_isec->level);
+ spin_unlock(&slm_isec->lock);
+}
+
+/*
+ * When a task gets allocated, it inherits the current IAC and SAC.
+ * Set the values and store them in p->security.
+ */
+static int slm_task_alloc_security(struct task_struct *tsk)
+{
+ struct slm_tsec_data *tsec = tsk->security;
+
+ if (!tsec) {
+ tsec = slm_init_task(tsk, GFP_KERNEL);
+ if (!tsec)
+ return -ENOMEM;
+ }
+ tsk->security = tsec;
+ return 0;
+}
+
+static void slm_task_free_security(struct task_struct *tsk)
+{
+ struct slm_tsec_data *tsec;
+
+ tsec = tsk->security;
+ tsk->security = NULL;
+ kfree(tsec);
+}
+
+/* init_task is an integrity guard */
+static int slm_task_init_alloc_security(struct task_struct *tsk)
+{
+ struct slm_tsec_data *tsec = kzalloc(sizeof(struct slm_tsec_data), GFP_KERNEL);
+
+ if (!tsec)
+ return -ENOMEM;
+
+ tsec->lock = SPIN_LOCK_UNLOCKED;
+
+ tsec->iac_r = SLM_IAC_UNTRUSTED;
+ tsec->iac_wx = SLM_IAC_SYSTEM;
+
+ tsec->unlimited = 1;
+
+ tsk->security = tsec;
+ return 0;
+}
+
+static int slm_task_post_setuid(uid_t old_ruid, uid_t old_euid,
+ uid_t old_suid, int flags)
+{
+ struct slm_tsec_data *cur_tsec = current->security;
+
+ if (cur_tsec && flags == LSM_SETID_ID) {
+ /*set process to USER level integrity for everything but root */
+ spin_lock(&cur_tsec->lock);
+ if ((cur_tsec->iac_r == cur_tsec->iac_wx)
+ && (cur_tsec->iac_r == SLM_IAC_UNTRUSTED));
+ else if (current->suid != 0) {
+ cur_tsec->iac_r = SLM_IAC_USER;
+ cur_tsec->iac_wx = SLM_IAC_USER;
+ } else if ((current->uid == 0) && (old_ruid != 0)) {
+ cur_tsec->iac_r = SLM_IAC_SYSTEM;
+ cur_tsec->iac_wx = SLM_IAC_SYSTEM;
+ }
+ spin_unlock(&cur_tsec->lock);
+ }
+ return 0;
+}
+
+static inline int slm_setprocattr(struct task_struct *tsk,
+ char *name, void *value, size_t size)
+{
+ return -EACCES;
+
+}
+
+static inline int slm_getprocattr(struct task_struct *tsk,
+ char *name, void *value, size_t size)
+{
+ struct slm_tsec_data *tsec = tsk->security;
+ size_t len = 0;
+
+ if (is_kernel_thread(tsk))
+ len = snprintf(value, size, "KERNEL");
+ else {
+ spin_lock(&tsec->lock);
+ if (tsec->iac_wx != tsec->iac_r)
+ len = snprintf(value, size, "GUARD wx:%s r:%s",
+ slm_iac_str[tsec->iac_wx],
+ slm_iac_str[tsec->iac_r]);
+ else
+ len = snprintf(value, size, "%s",
+ slm_iac_str[tsec->iac_wx]);
+ spin_unlock(&tsec->lock);
+ }
+ return min(len, size);
+}
+
+/*
+ * enforce: IWXAC(process) <= IAC(object)
+ * Permit process to execute file of equal or greater integrity
+ */
+static int enforce_integrity_execute(struct linux_binprm *bprm,
+ struct slm_file_xattr *level,
+ struct slm_tsec_data *cur_tsec)
+{
+ struct task_struct *parent_tsk = current->parent;
+ struct slm_tsec_data *parent_tsec = parent_tsk->security;
+ int rc = 0;
+
+ spin_lock(&cur_tsec->lock);
+ if (is_iac_less_than_or_exempt(level, cur_tsec->iac_wx))
+ /* Being a guard process is not inherited */
+ cur_tsec->iac_r = cur_tsec->iac_wx;
+ else {
+ rc = has_file_wperm(level);
+ if (atomic_read(¤t->mm->mm_users) != 1)
+ rc = 1;
+ if (rc)
+ spin_unlock(&cur_tsec->lock);
+ else {
+ cur_tsec->iac_r = level->iac_level;
+ cur_tsec->iac_wx = level->iac_level;
+ spin_unlock(&cur_tsec->lock);
+ revoke_permissions(level);
+ }
+ return rc ? -EACCES : 0;
+ }
+ spin_unlock(&cur_tsec->lock);
+ return 0;
+}
+
+static void enforce_guard_integrity_execute(struct linux_binprm *bprm,
+ struct slm_file_xattr *level,
+ struct slm_tsec_data *cur_tsec)
+{
+ if ((strcmp(bprm->filename, bprm->interp) != 0)
+ && (level->guard.unlimited))
+ level->guard.unlimited = 0;
+
+ spin_lock(&cur_tsec->lock);
+ if (level->guard.unlimited) {
+ cur_tsec->iac_r = level->guard.iac_r;
+ cur_tsec->iac_wx = level->guard.iac_wx;
+ } else {
+ if (cur_tsec->iac_r > level->guard.iac_r)
+ cur_tsec->iac_r = level->guard.iac_r;
+ if (cur_tsec->iac_wx > level->guard.iac_wx)
+ cur_tsec->iac_wx = level->guard.iac_wx;
+ }
+ spin_unlock(&cur_tsec->lock);
+}
+
+/*
+ * Enforce process integrity & secrecy levels.
+ * - update integrity process level of integrity guard program
+ * - update secrecy process level of secrecy guard program
+ */
+static int slm_bprm_check_security(struct linux_binprm *bprm)
+{
+ struct dentry *dentry;
+ struct slm_tsec_data *cur_tsec = current->security;
+ struct slm_file_xattr level;
+ int rc, status;
+
+ /* Special case interpreters */
+ spin_lock(&cur_tsec->lock);
+ if (strcmp(bprm->filename, bprm->interp) != 0) {
+ if (!cur_tsec->script_dentry) {
+ spin_unlock(&cur_tsec->lock);
+ return 0;
+ } else
+ dentry = cur_tsec->script_dentry;
+ } else {
+ dentry = bprm->file->f_dentry;
+ cur_tsec->script_dentry = dentry;
+ }
+ spin_unlock(&cur_tsec->lock);
+
+ slm_get_level(dentry, &level);
+
+ /* slm_inode_permission measured all SYSTEM level integrity objects */
+ if (level.iac_level != SLM_IAC_SYSTEM)
+ integrity_measure(dentry, bprm->filename, MAY_EXEC);
+
+ /* Possible return codes: PERMIT, DENY, NOLABEL */
+ rc = integrity_verify_data(dentry, &status);
+ if (rc < 0)
+ return rc;
+
+ switch(status) {
+ case INTEGRITY_FAIL:
+ if (!is_kernel_thread(current))
+ return -EACCES;
+ break;
+ case INTEGRITY_NOLABEL:
+ level.iac_level = SLM_IAC_UNTRUSTED;
+ }
+
+ rc = enforce_integrity_execute(bprm, &level, cur_tsec);
+ if (rc < 0)
+ return rc;
+ if (is_guard_integrity(&level))
+ enforce_guard_integrity_execute(bprm, &level, cur_tsec);
+
+ return 0;
+}
+
+static int slm_inode_setattr(struct dentry *dentry, struct iattr *iattr)
+{
+ struct slm_tsec_data *cur_tsec = current->security;
+ struct slm_file_xattr level;
+ int rc = 0;
+
+ slm_get_level(dentry, &level);
+ spin_lock(&cur_tsec->lock);
+ if (cur_tsec->iac_wx < level.iac_level)
+ rc = -EACCES;
+ spin_unlock(&cur_tsec->lock);
+ return rc;
+}
+
+static inline int slm_capable(struct task_struct *tsk, int cap)
+{
+ struct slm_tsec_data *tsec = tsk->security;
+ int rc = 0;
+
+ /* Derived from include/linux/sched.h:capable. */
+ if (cap_raised(tsk->cap_effective, cap)) {
+ spin_lock(&tsec->lock);
+ if (tsec->iac_wx == SLM_IAC_UNTRUSTED &&
+ cap == CAP_SYS_ADMIN)
+ rc = -EACCES;
+ spin_unlock(&tsec->lock);
+ return rc;
+ }
+ return -EPERM;
+}
+
+static int slm_ptrace(struct task_struct *parent, struct task_struct *child)
+{
+ struct slm_tsec_data *parent_tsec = parent->security,
+ *child_tsec = child->security;
+ int rc = 0;
+
+ if (is_kernel_thread(parent))
+ return 0;
+ spin_lock(&parent_tsec->lock);
+ if (parent_tsec->iac_wx < child_tsec->iac_wx)
+ rc = -EPERM;
+ spin_unlock(&parent_tsec->lock);
+ return rc;
+}
+
+static int slm_shm_alloc_security(struct shmid_kernel *shp)
+{
+ struct slm_tsec_data *cur_tsec = current->security;
+ struct kern_ipc_perm *perm = &shp->shm_perm;
+ struct slm_isec_data *isec = slm_alloc_security(GFP_KERNEL);
+
+ if (!isec)
+ return -ENOMEM;
+
+ spin_lock(&cur_tsec->lock);
+ if (cur_tsec->iac_wx != cur_tsec->iac_r) /* guard process */
+ set_level_exempt(&isec->level);
+ else
+ set_level_tsec_write(&isec->level, cur_tsec);
+ spin_unlock(&cur_tsec->lock);
+ perm->security = isec;
+
+ return 0;
+}
+
+static void slm_shm_free_security(struct shmid_kernel *shp)
+{
+ struct kern_ipc_perm *perm = &shp->shm_perm;
+ struct slm_isec_data *isec = perm->security;
+
+ perm->security = NULL;
+ kfree(isec);
+}
+
+/*
+ * When shp exists called holding perm->lock
+ */
+static int slm_shm_shmctl(struct shmid_kernel *shp, int cmd)
+{
+ struct kern_ipc_perm *perm;
+ struct slm_isec_data *perm_isec;
+ struct file *file;
+ struct dentry *dentry;
+ struct inode *inode;
+ int rc;
+
+ if (!shp)
+ return 0;
+
+ perm = &shp->shm_perm;
+ perm_isec = perm->security;
+ file = shp->shm_file;
+ dentry = file->f_dentry;
+ inode = dentry->d_inode;
+
+ spin_lock(&perm_isec->lock);
+ rc = slm_set_taskperm(MAY_READ | MAY_WRITE, &perm_isec->level);
+ spin_unlock(&perm_isec->lock);
+ return rc;
+}
+
+/*
+ * Called holding perm->lock
+ */
+static int slm_shm_shmat(struct shmid_kernel *shp,
+ char __user * shmaddr, int shmflg)
+{
+ int mask = MAY_READ;
+ int rc;
+ struct kern_ipc_perm *perm = &shp->shm_perm;
+ struct file *file = shp->shm_file;
+ struct dentry *dentry = file->f_dentry;
+ struct inode *inode = dentry->d_inode;
+ struct slm_isec_data *perm_isec = perm->security,
+ *isec = inode->i_security;
+
+ if (shmflg != SHM_RDONLY)
+ mask |= MAY_WRITE;
+
+ spin_lock(&perm_isec->lock);
+ rc = slm_set_taskperm(mask, &perm_isec->level);
+
+ spin_lock(&isec->lock);
+ memcpy(&isec->level, &perm_isec->level, sizeof(struct slm_file_xattr));
+ spin_unlock(&perm_isec->lock);
+ spin_unlock(&isec->lock);
+
+ return rc;
+}
+
+static struct security_operations slm_security_ops = {
+ .bprm_check_security = slm_bprm_check_security,
+ .file_permission = slm_file_permission,
+ .inode_permission = slm_inode_permission,
+ .inode_unlink = slm_inode_unlink,
+ .inode_create = slm_inode_create,
+ .inode_mkdir = slm_inode_mkdir,
+ .inode_rename = slm_inode_rename,
+ .inode_setattr = slm_inode_setattr,
+ .inode_setxattr = slm_inode_setxattr,
+ .inode_post_setxattr = slm_inode_post_setxattr,
+ .inode_removexattr = slm_inode_removexattr,
+ .inode_alloc_security = slm_inode_alloc_security,
+ .inode_free_security = slm_inode_free_security,
+ .inode_init_security = slm_inode_init_security,
+ .socket_create = slm_socket_create,
+ .socket_post_create = slm_socket_post_create,
+ .task_alloc_security = slm_task_alloc_security,
+ .task_free_security = slm_task_free_security,
+ .task_post_setuid = slm_task_post_setuid,
+ .capable = slm_capable,
+ .ptrace = slm_ptrace,
+ .shm_alloc_security = slm_shm_alloc_security,
+ .shm_free_security = slm_shm_free_security,
+ .shm_shmat = slm_shm_shmat,
+ .shm_shmctl = slm_shm_shmctl,
+ .getprocattr = slm_getprocattr,
+ .setprocattr = slm_setprocattr,
+ .d_instantiate = slm_d_instantiate
+};
+
+#ifdef CONFIG_SECURITY_SLIM_BOOTPARAM
+int slim_enabled = CONFIG_SECURITY_SLIM_BOOTPARAM_VALUE;
+
+static int __init slim_enabled_setup(char *str)
+{
+ slim_enabled = simple_strtol(str, NULL, 0);
+ return 1;
+}
+__setup("slim=", slim_enabled_setup);
+#else
+int slim_enabled = 1;
+#endif
+static int __init init_slm(void)
+{
+ if (!slim_enabled)
+ return 0;
+ slm_task_init_alloc_security(current);
+ return register_security(&slm_security_ops);
+}
+security_initcall(init_slm);
On Tue, 12 Sep 2006 10:57:45 -0700
Kylene Jo Hall <[email protected]> wrote:
> SLIM is an LSM module which provides an enhanced low water-mark
> integrity and high water-mark secrecy mandatory access control
> model.
>
trivial things:
> --- linux-2.6.18/security/slim/slm_main.c 1969-12-31 16:00:00.000000000 -0800
> +++ linux-2.6.17-working/security/slim/slm_main.c 2006-09-06 11:49:09.000000000 -0700
> @@ -0,0 +1,1378 @@
> +/*
> + * SLIM - Simple Linux Integrity Module
> + *
> + * Copyright (C) 2005,2006 IBM Corporation
> + * Author: Mimi Zohar <[email protected]>
> + * Kylene Hall <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, version 2 of the License.
> + */
> +
> +#include <linux/mman.h>
> +#include <linux/config.h>
> +#include <linux/kernel.h>
> +#include <linux/security.h>
> +#include <linux/integrity.h>
> +#include <linux/proc_fs.h>
> +#include <linux/socket.h>
> +#include <linux/fs.h>
> +#include <linux/file.h>
> +#include <linux/namei.h>
> +#include <linux/mm.h>
> +#include <linux/shm.h>
> +#include <linux/ipc.h>
> +#include <linux/errno.h>
> +#include <linux/xattr.h>
> +#include <net/sock.h>
> +
> +#include "slim.h"
> +
> +#define XATTR_NAME "security.slim.level"
> +
> +#define ZERO_STR "0"
> +#define UNTRUSTED_STR "UNTRUSTED"
> +#define USER_STR "USER"
> +#define SYSTEM_STR "SYSTEM"
> +
> +char *slm_iac_str[] = {
> + ZERO_STR,
> + UNTRUSTED_STR,
> + USER_STR,
> + SYSTEM_STR
> +};
Could this be made static?
> +static char *get_token(char *buf_start, char *buf_end, char delimiter,
> + int *token_len)
> +{
> + char *bufp = buf_start;
> + char *token = NULL;
> +
> + while (!token && (bufp < buf_end)) { /* Get start of token */
> + switch (*bufp) {
> + case ' ':
> + case '\n':
> + case '\t':
> + bufp++;
> + break;
> + case '#':
> + while ((*bufp != '\n') && (bufp++ < buf_end)) ;
newline needed here.
> +
> +/*
> + * Determine if a file is opened with write permissions.
> + */
> +static int has_file_wperm(struct slm_file_xattr *cur_level)
> +{
> + int i, j = 0;
> + struct files_struct *files = current->files;
> + unsigned long fd = 0;
> + struct fdtable *fdt;
> + struct file *file;
> + int rc = 0;
> +
> + if (is_kernel_thread(current))
> + return 0;
> +
> + if (!files || !cur_level)
> + return 0;
> +
> + spin_lock(&files->file_lock);
> + fdt = files_fdtable(files);
> +
> + for (;;) {
> + i = j * __NFDBITS;
> + if (i >= fdt->max_fdset || i >= fdt->max_fds)
> + break;
> + fd = fdt->open_fds->fds_bits[j++];
> + while (fd) {
> + if (fd & 1) {
> + file = fdt->fd[i++];
> + if (file)
> + rc = mark_has_file_wperm(file,
> + cur_level);
> + }
> + fd >>= 1;
> + }
> + }
> + spin_unlock(&files->file_lock);
> + return rc;
> +}
Could we use a more meaningful identifier than `j' here? Perhaps `i' too?
> +#define EXEMPT_STR "EXEMPT"
> +static enum slm_iac_level parse_iac(char *token)
> +{
> + int iac;
> +
> + if (strncmp(token, EXEMPT_STR, strlen(EXEMPT_STR)) == 0)
> + return SLM_IAC_EXEMPT;
> + for (iac = 0; iac < sizeof(slm_iac_str) / sizeof(char *); iac++) {
ARRAY_SIZE()
> + if (strncmp(token, slm_iac_str[iac], strlen(slm_iac_str[iac]))
> + == 0)
> + return iac;
> + }
> + return SLM_IAC_ERROR;
> +}
> +
>
> ...
>
> +
> +static inline int slm_getprocattr(struct task_struct *tsk,
> + char *name, void *value, size_t size)
It's rather pointless to declare a function inline and to then only refer to
it via a pointer-to-function. Please review all inlinings.
On Thu, 2006-09-14 at 16:52 -0700, Andrew Morton wrote:
> On Tue, 12 Sep 2006 10:57:45 -0700
> Kylene Jo Hall <[email protected]> wrote:
>
> > SLIM is an LSM module which provides an enhanced low water-mark
> > integrity and high water-mark secrecy mandatory access control
> > model.
> >
>
> trivial things:
>
>
> > --- linux-2.6.18/security/slim/slm_main.c 1969-12-31 16:00:00.000000000 -0800
> > +++ linux-2.6.17-working/security/slim/slm_main.c 2006-09-06 11:49:09.000000000 -0700
> > @@ -0,0 +1,1378 @@
> > +/*
> > + * SLIM - Simple Linux Integrity Module
> > + *
> > + * Copyright (C) 2005,2006 IBM Corporation
> > + * Author: Mimi Zohar <[email protected]>
> > + * Kylene Hall <[email protected]>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, version 2 of the License.
> > + */
> > +
> > +#include <linux/mman.h>
> > +#include <linux/config.h>
> > +#include <linux/kernel.h>
> > +#include <linux/security.h>
> > +#include <linux/integrity.h>
> > +#include <linux/proc_fs.h>
> > +#include <linux/socket.h>
> > +#include <linux/fs.h>
> > +#include <linux/file.h>
> > +#include <linux/namei.h>
> > +#include <linux/mm.h>
> > +#include <linux/shm.h>
> > +#include <linux/ipc.h>
> > +#include <linux/errno.h>
> > +#include <linux/xattr.h>
> > +#include <net/sock.h>
> > +
> > +#include "slim.h"
> > +
> > +#define XATTR_NAME "security.slim.level"
> > +
> > +#define ZERO_STR "0"
> > +#define UNTRUSTED_STR "UNTRUSTED"
> > +#define USER_STR "USER"
> > +#define SYSTEM_STR "SYSTEM"
> > +
> > +char *slm_iac_str[] = {
> > + ZERO_STR,
> > + UNTRUSTED_STR,
> > + USER_STR,
> > + SYSTEM_STR
> > +};
>
> Could this be made static?
I don't think so because it is used in slm_secfs.c
I'll get the rest fixed up and submit a patch.
Thanks,
Kylie
Miscellaneous fixes requested when the intial patches were pulled in
last week. This patch addresses the following comments.
On Thu, 2006-09-14 at 16:52 -0700, Andrew Morton wrote:
> trivial things:
> > +char *slm_iac_str[] = {
> > + ZERO_STR,
> > + UNTRUSTED_STR,
> > + USER_STR,
> > + SYSTEM_STR
> > +};
>
> Could this be made static?
>
No because it is used in slm_secfs.c.
> > + case '#':
> > + while ((*bufp != '\n') && (bufp++ < buf_end)) ;
>
> newline needed here.
Added in the patch.
> > +static int has_file_wperm(struct slm_file_xattr *cur_level)
> > +{
> > + int i, j = 0;
> > + struct files_struct *files = current->files;
> > + unsigned long fd = 0;
> > + struct fdtable *fdt;
> > + struct file *file;
> > + int rc = 0;
> > +
> > + if (is_kernel_thread(current))
> > + return 0;
> > +
> > + if (!files || !cur_level)
> > + return 0;
> > +
> > + spin_lock(&files->file_lock);
> > + fdt = files_fdtable(files);
> > +
> > + for (;;) {
> > + i = j * __NFDBITS;
> > + if (i >= fdt->max_fdset || i >= fdt->max_fds)
> > + break;
> > + fd = fdt->open_fds->fds_bits[j++];
> > + while (fd) {
> > + if (fd & 1) {
> > + file = fdt->fd[i++];
> > + if (file)
> > + rc = mark_has_file_wperm(file,
> > + cur_level);
> > + }
> > + fd >>= 1;
> > + }
> > + }
> > + spin_unlock(&files->file_lock);
> > + return rc;
> > +}
>
> Could we use a more meaningful identifier than `j' here? Perhaps `i' too?
This function has been cleaned up with the use of find_next_bit and i
and j are no longer used in the patch below.
> > + for (iac = 0; iac < sizeof(slm_iac_str) / sizeof(char *); iac++) {
>
> ARRAY_SIZE()
Fixed below.
> > + if (strncmp(token, slm_iac_str[iac], strlen(slm_iac_str[iac]))
> > + == 0)
> > + return iac;
> > + }
> > + return SLM_IAC_ERROR;
> > +}
> > +
> >
> > ...
> >
> > +
> > +static inline int slm_getprocattr(struct task_struct *tsk,
> > + char *name, void *value, size_t size)
> It's rather pointless to declare a function inline and to then only refer to
> it via a pointer-to-function. Please review all inlinings.
All inlines have been reviewed and fixed if necessary in the patch
below. 3 hooks were affected.
Signed-off-by: Kylene Hall <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
---
security/slim/slm_main.c | 29 ++++++++++-------------------
1 files changed, 10 insertions(+), 19 deletions(-)
--- linux-2.6.18-rc6-orig/security/slim/slm_main.c 2006-09-18 16:41:51.000000000 -0500
+++ linux-2.6.18-rc6/security/slim/slm_main.c 2006-09-19 16:45:34.000000000 -0500
@@ -61,6 +60,7 @@ static char *get_token(char *buf_start,
break;
case '#':
while ((*bufp != '\n') && (bufp++ < buf_end)) ;
+
bufp++;
break;
default:
@@ -141,7 +141,6 @@ static inline int mark_has_file_wperm(st
*/
static int has_file_wperm(struct slm_file_xattr *cur_level)
{
- int i, j = 0;
struct files_struct *files = current->files;
unsigned long fd = 0;
struct fdtable *fdt;
@@ -157,21 +156,12 @@ static int has_file_wperm(struct slm_fil
spin_lock(&files->file_lock);
fdt = files_fdtable(files);
- for (;;) {
- i = j * __NFDBITS;
- if (i >= fdt->max_fdset || i >= fdt->max_fds)
- break;
- fd = fdt->open_fds->fds_bits[j++];
- while (fd) {
- if (fd & 1) {
- file = fdt->fd[i++];
- if (file)
- rc = mark_has_file_wperm(file,
- cur_level);
- }
- fd >>= 1;
- }
+ while((fd=find_next_bit(fdt->open_fds->fds_bits, fdt->max_fdset, fd)) < fdt->max_fdset) {
+ file = fdt->fd[fd++];
+ if (file)
+ rc = mark_has_file_wperm(file, cur_level);
}
+
spin_unlock(&files->file_lock);
return rc;
}
@@ -241,7 +231,7 @@ static enum slm_iac_level parse_iac(char
if (strncmp(token, EXEMPT_STR, strlen(EXEMPT_STR)) == 0)
return SLM_IAC_EXEMPT;
- for (iac = 0; iac < sizeof(slm_iac_str) / sizeof(char *); iac++) {
+ for (iac = 0; iac < ARRAY_SIZE(slm_iac_str); iac++) {
if (strncmp(token, slm_iac_str[iac], strlen(slm_iac_str[iac]))
== 0)
return iac;
@@ -1243,7 +1233,7 @@ static int slm_task_post_setuid(uid_t ol
return rc;
}
-static inline int slm_setprocattr(struct task_struct *tsk,
+static int slm_setprocattr(struct task_struct *tsk,
char *name, void *value, size_t size)
{
dprintk(SLM_BASE, "%s: %s \n", __FUNCTION__, name);
@@ -1251,7 +1241,7 @@ static inline int slm_setprocattr(struct
}
-static inline int slm_getprocattr(struct task_struct *tsk,
+static int slm_getprocattr(struct task_struct *tsk,
char *name, void *value, size_t size)
{
struct slm_tsec_data *tsec = tsk->security;
@@ -1473,7 +1463,7 @@ static int slm_inode_setattr(struct dent
return rc;
}
-static inline int slm_capable(struct task_struct *tsk, int cap)
+static int slm_capable(struct task_struct *tsk, int cap)
{
struct slm_tsec_data *tsec = tsk->security;
int rc = 0;
On Tue, 2006-09-12 at 10:57 -0700, Kylene Jo Hall wrote:
> SLIM is an LSM module which provides an enhanced low water-mark
> integrity and high water-mark secrecy mandatory access control
> model.
<snip>
> SLIM now performs a generic revocation operation, including revoking
> mmap and shared memory access. Note that during demotion or promotion
> of a process, SLIM needs only revoke write access to files with higher
> integrity, or lower secrecy. Read and execute permissions are blocked
> as needed, not revoked. SLIM hopefully uses d_instantiate correctly now.
Neither the above text nor your Documentation nor your code comments
seems to have been fully updated to reflect the changes to SLIM in this
submission, e.g. the removal of the secrecy MAC model and the change in
how you deal with "revocation". Thus they are misleading and confusing
to potential users and to subsequent maintenance of the code.
> --- linux-2.6.18/security/slim/slm_main.c 1969-12-31 16:00:00.000000000 -0800
> +++ linux-2.6.17-working/security/slim/slm_main.c 2006-09-06 11:49:09.000000000 -0700
> +/*
> + * Called with current->files->file_lock. There is not a great lock to grab
> + * for demotion of this type. The only place f_mode is changed after install
> + * is in mark_files_ro in the filesystem code. That function is also changing
> + * taking away write rights so even if we race the outcome is the same.
> + */
> +static inline int mark_has_file_wperm(struct file *file,
> + struct slm_file_xattr *cur_level)
> +{
> + struct inode *inode;
> + struct slm_isec_data *isec;
> + int rc = 0;
> +
> + inode = file->f_dentry->d_inode;
> + if (!S_ISREG(inode->i_mode) || !(file->f_mode & FMODE_WRITE))
> + return 0;
> +
> + isec = inode->i_security;
> + spin_lock(&isec->lock);
> + if (is_lower_integrity(cur_level, &isec->level))
> + rc = 1;
> + spin_unlock(&isec->lock);
> + return rc;
> +}
So rather than "revoke", you just test for the presence of an open file
with write access, and if present, you ultimately deny the read attempt
that would have caused demotion? Still calling the function "mark_..."
is confusing here. But it doesn't provide any real guarantees, as there
may be open files sitting on an AF_LOCAL socket that the process will
receive after the "demotion" (and you don't mediate file_receive, or
file_mmap, so nothing prevents receipt and then mapping with
PROT_WRITE), or another thread could be sharing the file table (and your
mm_users test is neither sufficient nor correct as a way of checking for
sharing in general), etc.
I think you would do better to just drop out the so-called "revocation"
code altogether and acknowledge this limitation in your Documentation.
Meanwhile, you could do a better job of using the permission checking
hooks that do exist to apply checks on subsequent operations after
demotion, like implementing file_receive, file_mmap, etc.
> +/*
> + * All directories with xattr support should be labeled, but just in case
> + * recursively traverse path (dentry->parent) until level info is found.
The comment is no longer accurate, right?
> + */
> +static void slm_get_level(struct dentry *dentry, struct slm_file_xattr *level)
> +{
> + struct inode *inode = dentry->d_inode;
> + struct slm_isec_data *isec = inode->i_security;
> +
> + if (is_isec_defined(isec)) {
> + spin_lock(&isec->lock);
> + memcpy(level, &isec->level, sizeof(struct slm_file_xattr));
> + spin_unlock(&isec->lock);
> + return;
> + }
> +
> + if (is_exempt_fastpath(inode)) {
> + memset(level, 0, sizeof(struct slm_file_xattr));
> + set_level_exempt(level);
> + } else if (S_ISSOCK(inode->i_mode))
> + update_sock_level(dentry, level);
> + else
> + update_level(dentry, level);
> +
> + spin_lock(&isec->lock);
> + memcpy(&isec->level, level, sizeof(struct slm_file_xattr));
> + spin_unlock(&isec->lock);
The locking doesn't make much sense to me, and that applies throughout.
Can you explain it? For example, why don't you have to recheck
is_isec_defined() after taking the lock? Why do you call this function
repeatedly in various hooks rather than just setting up the isec once
from d_instantiate and inode_init_security?
> +static int enforce_integrity_read(struct slm_file_xattr *level)
> +{
> + struct slm_tsec_data *cur_tsec = current->security;
> + int rc = 0;
> +
> + spin_lock(&cur_tsec->lock);
> + if (!is_iac_less_than_or_exempt(level, cur_tsec->iac_r)) {
> + rc = has_file_wperm(level);
> + if (atomic_read(¤t->mm->mm_users) != 1)
This only checks for users of the mm (not necessarily other forms of
sharing), and it can yield "false" positives due to transient references
(also noted by Hugh). If you look at the SELinux code, you'll see a
more complex test applied in selinux_setprocattr after this test, and
that is only checking for mm sharing. For sharing of the file table,
there is the unsafe_exec checking that happens, with the corresponding
LSM_UNSAFE_SHARE check applied by SELinux (since we are only flushing
the table on context-changing exec).
> +
> +/*
> + * file changes invalidate isec
> + */
> +static int slm_file_permission(struct file *file, int mask)
> +{
> + struct slm_isec_data *isec = file->f_dentry->d_inode->i_security;
> +
> + if (((mask & MAY_WRITE) || (mask & MAY_APPEND)) && isec) {
> + spin_lock(&isec->lock);
> + isec->level.iac_level = SLM_IAC_NOTDEFINED;
> + spin_unlock(&isec->lock);
> + }
> + return 0;
> +}
I don't understand the above - any write to a file by any process resets it to an undefined state?
And then you just reset from xattr upon the next hook that happens to
call get_level or the next d_instantiate? What is the point of that?
> +
> +static int is_untrusted_blk_access(struct inode *inode)
> +{
> + struct slm_tsec_data *cur_tsec = current->security;
> + int rc = 0;
> +
> + spin_lock(&cur_tsec->lock);
> + if (cur_tsec && (cur_tsec->iac_wx == SLM_IAC_UNTRUSTED)
> + && S_ISBLK(inode->i_mode))
> + rc = 1;
> + spin_unlock(&cur_tsec->lock);
> + return rc;
> +}
This kind of special case logic is troubling, and shouldn't be necessary.
> +/*
> + * Premise:
> + * Can't write or execute higher integrity, can't read lower integrity
> + * Can't read or execute higher secrecy, can't write lower secrecy
Misleading comment - no secrecy model within.
> + */
> +static int slm_inode_permission(struct inode *inode, int mask,
> + struct nameidata *nd)
> +{
> + struct dentry *dentry = NULL;
> + struct slm_file_xattr level;
> +
> + if (S_ISDIR(inode->i_mode) && (mask & MAY_WRITE))
> + return 0;
> +
> + dentry = (!nd || !nd->dentry) ? d_find_alias(inode) : nd->dentry;
> + if (!dentry)
> + return 0;
Relying on a dentry here is problematic.
> +
> + if (is_untrusted_blk_access(inode))
> + return -EPERM;
> +
> + slm_get_level(dentry, &level);
If the isec isn't already set up here, you have a bug.
It shouldn't be necessary to go through this processing on every
permission check.
> +
> + /* measure all SYSTEM level integrity objects */
> + if (level.iac_level == SLM_IAC_SYSTEM)
> + integrity_measure(dentry, NULL, mask);
This seems to be the wrong point to perform a measurement.
> +/* Create the security.slim.level extended attribute */
> +static int slm_inode_init_security(struct inode *inode, struct inode *dir,
> + char **name, void **value, size_t * len)
> +{
> + struct slm_isec_data *isec = inode->i_security, *parent_isec =
> + dir->i_security;
> + struct slm_tsec_data *cur_tsec = current->security;
> + struct slm_file_xattr level;
> + struct xattr_data *data;
> + int rc;
> +
> + if (!name || !value || !len)
> + return 0;
> +
> + memset(&level, 0, sizeof(struct slm_file_xattr));
> +
> + if (is_isec_defined(parent_isec)) {
> + spin_lock(&parent_isec->lock);
> + memcpy(&level, &parent_isec->level,
> + sizeof(struct slm_file_xattr));
> + spin_unlock(&parent_isec->lock);
> + }
> +
> + spin_lock(&cur_tsec->lock);
> + /* low integrity process wrote into a higher level directory */
> + if (cur_tsec->iac_wx < level.iac_level)
> + set_level_tsec_write(&level, cur_tsec);
> + /* if directory is exempt, then use process level */
> + if (is_iac_level_exempt(&level)) {
> + /* When a guard process creates a directory */
> + if (S_ISDIR(inode->i_mode)
> + && (cur_tsec->iac_wx != cur_tsec->iac_r))
> + set_level_exempt(&level);
> + else
> + set_level_tsec_write(&level, cur_tsec);
> + }
> +
> + /* if a guard process creates a UNIX socket, then EXEMPT it */
> + if (S_ISSOCK(inode->i_mode)
> + && (cur_tsec->iac_wx != cur_tsec->iac_r))
> + set_level_exempt(&level);
> + spin_unlock(&cur_tsec->lock);
> +
> + spin_lock(&isec->lock);
> + memcpy(&isec->level, &level, sizeof(struct slm_file_xattr));
> + spin_unlock(&isec->lock);
> +
> + data = kmalloc(sizeof(struct xattr_data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + /* set levels, based on parent */
> + rc = slm_set_xattr(&level, &data->name, &data->value, &data->len);
> + if (rc < 0) {
> + kfree(data);
> + return rc;
> + }
> +
> + *name = data->name;
> + *value = data->value;
> + *len = data->len;
What frees the xattr_data structure? Why does it even exist?
> +static int slm_inode_rename(struct inode *old_dir,
> + struct dentry *old_dentry,
> + struct inode *new_dir, struct dentry *new_dentry)
> +{
> + struct slm_file_xattr old_level, parent_level;
> + struct dentry *parent_dentry;
> +
> + if (old_dir == new_dir)
> + return 0;
> +
> + slm_get_level(old_dentry, &old_level);
> +
> + parent_dentry = dget_parent(new_dentry);
> + slm_get_level(parent_dentry, &parent_level);
> + dput(parent_dentry);
> +
> + if (is_lower_integrity(&old_level, &parent_level))
> + return -EPERM;
> + return 0;
> +}
Why can't you get the parent level from old_dir if that is what you need?
What is the rationale for this logic - renaming a file doesn't change its integrity.
> +static void slm_inode_post_setxattr(struct dentry *dentry, char *name,
> + void *value, size_t size, int flags)
> +{
> + struct slm_isec_data *slm_isec;
> + struct slm_file_xattr level;
> + int rc, status = 0;
> +
> + if (strncmp(name, XATTR_NAME, strlen(XATTR_NAME)) != 0)
> + return;
Not just here, but in several places you are using strncmp like this.
Are you sure that is what you mean to test (versus an exact match)?
> +
> + rc = slm_get_xattr(dentry, &level, &status);
Here you do a get_xattr, but this is post_setxattr - and you just ignore the supplied (value, size) pair?
I'm out of time for now, but there is plenty more to comment on.
--
Stephen Smalley
National Security Agency
Sorry for the delay. We have been working on the issues below. There
will be patches coming to address some of these concerns. We have
included some comments and questions inline below.
On Tue, 2006-09-26 at 14:44 -0400, Stephen Smalley wrote:
> On Tue, 2006-09-12 at 10:57 -0700, Kylene Jo Hall wrote:
> > SLIM is an LSM module which provides an enhanced low water-mark
> > integrity and high water-mark secrecy mandatory access control
> > model.
> <snip>
> > SLIM now performs a generic revocation operation, including revoking
> > mmap and shared memory access. Note that during demotion or promotion
> > of a process, SLIM needs only revoke write access to files with higher
> > integrity, or lower secrecy. Read and execute permissions are blocked
> > as needed, not revoked. SLIM hopefully uses d_instantiate correctly now.
>
> Neither the above text nor your Documentation nor your code comments
> seems to have been fully updated to reflect the changes to SLIM in this
> submission, e.g. the removal of the secrecy MAC model and the change in
> how you deal with "revocation". Thus they are misleading and confusing
> to potential users and to subsequent maintenance of the code.
I'll provide a patch that fixes the comments and in-tree documentation.
>
> > --- linux-2.6.18/security/slim/slm_main.c 1969-12-31 16:00:00.000000000 -0800
> > +++ linux-2.6.17-working/security/slim/slm_main.c 2006-09-06 11:49:09.000000000 -0700
> > +/*
> > + * Called with current->files->file_lock. There is not a great lock to grab
> > + * for demotion of this type. The only place f_mode is changed after install
> > + * is in mark_files_ro in the filesystem code. That function is also changing
> > + * taking away write rights so even if we race the outcome is the same.
> > + */
> > +static inline int mark_has_file_wperm(struct file *file,
> > + struct slm_file_xattr *cur_level)
> > +{
> > + struct inode *inode;
> > + struct slm_isec_data *isec;
> > + int rc = 0;
> > +
> > + inode = file->f_dentry->d_inode;
> > + if (!S_ISREG(inode->i_mode) || !(file->f_mode & FMODE_WRITE))
> > + return 0;
> > +
> > + isec = inode->i_security;
> > + spin_lock(&isec->lock);
> > + if (is_lower_integrity(cur_level, &isec->level))
> > + rc = 1;
> > + spin_unlock(&isec->lock);
> > + return rc;
> > +}
>
> So rather than "revoke", you just test for the presence of an open file
> with write access, and if present, you ultimately deny the read attempt
> that would have caused demotion? Still calling the function "mark_..."
> is confusing here. But it doesn't provide any real guarantees, as there
> may be open files sitting on an AF_LOCAL socket that the process will
> receive after the "demotion" (and you don't mediate file_receive, or
> file_mmap, so nothing prevents receipt and then mapping with
> PROT_WRITE), or another thread could be sharing the file table (and your
> mm_users test is neither sufficient nor correct as a way of checking for
> sharing in general), etc.
>
> I think you would do better to just drop out the so-called "revocation"
> code altogether and acknowledge this limitation in your Documentation.
> Meanwhile, you could do a better job of using the permission checking
> hooks that do exist to apply checks on subsequent operations after
> demotion, like implementing file_receive, file_mmap, etc.
>
You are right in your AF_LOCAL socket concern. We have been working
on that and have implemented the file_receive hook, however, once we
have this hook we cannot imagine the case where file_mmap is also
needed. If you still feel it is necessary can you please explain
further.
--- linux-2.6.19-rc2/security/slim/slm_main.c 2006-10-19 12:05:58.000000000 -0700
+++ linux-2.6.19-rc2/security/slim/slm_main.c 2006-10-19 12:11:37.000000000 -0700
@@ -1130,6 +1103,34 @@ static int slm_socket_post_create(struct
return 0;
}
+static int slm_file_receive(struct file *file)
+{
+ struct slm_isec_data *isec = file->f_dentry->d_inode->i_security;
+ struct slm_tsec_data *tsec = current->security;
+ struct slm_file_xattr level;
+ int rc = 0;
+
+ spin_lock(&isec->lock);
+ memcpy(&level, &isec->level, sizeof(struct slm_file_xattr));
+ spin_unlock(&isec->lock);
+
+ spin_lock(&tsec->lock);
+ if (file->f_mode & FMODE_READ) { /* IRAC(process) <= IAC(object) */
+ if (!is_iac_less_than_or_exempt(&level, tsec->iac_r))
+ rc = -EPERM;
+ }
+ if (file->f_mode & FMODE_WRITE) { /* IWXAC(process) >= IAC(object) */
+ if (!is_iac_greater_than_or_exempt(&level, tsec->iac_wx))
+ rc = -EPERM;
+ }
+ if (file->f_mode & FMODE_EXEC) { /* IWXAC(process) <= IAC(object) */
+ if (!is_iac_less_than_or_exempt(&level, tsec->iac_wx))
+ rc = -EPERM;
+ }
+ spin_unlock(&tsec->lock);
+ return rc;
+}
+
/*
* When a task gets allocated, it inherits the current IAC and SAC.
* Set the values and store them in p->security.
@@ -1607,6 +1870,7 @@ static struct security_operations slm_se
.inode_init_security = slm_inode_init_security,
.socket_create = slm_socket_create,
.socket_post_create = slm_socket_post_create,
+ .file_receive = slm_file_receive,
.task_alloc_security = slm_task_alloc_security,
.task_free_security = slm_task_free_security,
.task_post_setuid = slm_task_post_setuid,
As for removing revocation altogether (and keeping demotion) we are
concerned that checking every read and write for example in the
file_permission hook would be a signifcant performance hit over the
revocation way. Additionally, Alan Cox brought up that demoting with an
writeable open file really isnt' safe (http://marc.theaimsgroup.com/?
l=linux-kernel&m=115643804205202&w=2) and revocation would still be
necessary for mmaps as I don't know of a security hook that can be used
to double check reads and writes to mmaps.
Is it really not possible to enumerate all the possible cases and
mediate them?
>
> > +/*
> > + * All directories with xattr support should be labeled, but just in case
> > + * recursively traverse path (dentry->parent) until level info is found.
>
> The comment is no longer accurate, right?
Correct.
>
> > + */
> > +static void slm_get_level(struct dentry *dentry, struct slm_file_xattr *level)
> > +{
> > + struct inode *inode = dentry->d_inode;
> > + struct slm_isec_data *isec = inode->i_security;
> > +
> > + if (is_isec_defined(isec)) {
> > + spin_lock(&isec->lock);
> > + memcpy(level, &isec->level, sizeof(struct slm_file_xattr));
> > + spin_unlock(&isec->lock);
> > + return;
> > + }
> > +
> > + if (is_exempt_fastpath(inode)) {
> > + memset(level, 0, sizeof(struct slm_file_xattr));
> > + set_level_exempt(level);
> > + } else if (S_ISSOCK(inode->i_mode))
> > + update_sock_level(dentry, level);
> > + else
> > + update_level(dentry, level);
> > +
> > + spin_lock(&isec->lock);
> > + memcpy(&isec->level, level, sizeof(struct slm_file_xattr));
> > + spin_unlock(&isec->lock);
>
> The locking doesn't make much sense to me, and that applies throughout.
> Can you explain it? For example, why don't you have to recheck
> is_isec_defined() after taking the lock? Why do you call this function
> repeatedly in various hooks rather than just setting up the isec once
> from d_instantiate and inode_init_security?
>
You were right about all the issues with this function. It is no longer
needed thus negating the issue with the locking. Removing this and the
reason it had been around (explanation below) also negates the need for
the other is_isec_defined checks. I looked and the rest of the locking
looks sane to me. Please point out any other particular instances you
aren't sure about.
> > +static int enforce_integrity_read(struct slm_file_xattr *level)
> > +{
> > + struct slm_tsec_data *cur_tsec = current->security;
> > + int rc = 0;
> > +
> > + spin_lock(&cur_tsec->lock);
> > + if (!is_iac_less_than_or_exempt(level, cur_tsec->iac_r)) {
> > + rc = has_file_wperm(level);
> > + if (atomic_read(¤t->mm->mm_users) != 1)
>
> This only checks for users of the mm (not necessarily other forms of
> sharing), and it can yield "false" positives due to transient references
> (also noted by Hugh). If you look at the SELinux code, you'll see a
> more complex test applied in selinux_setprocattr after this test, and
> that is only checking for mm sharing. For sharing of the file table,
> there is the unsafe_exec checking that happens, with the corresponding
> LSM_UNSAFE_SHARE check applied by SELinux (since we are only flushing
> the table on context-changing exec).
Yes the mm_users check as is will cause us to fail things that might not
necessarily need to fail but at least we are failing closed.
It seems that all the checks in sys_unshare should be done and those
that can be unshared should be and if any sharing exists that can't be
undone (such as unshare_vm) then the demotion would need to be
prevented. We would implement this similar to how we split sys_mprotect
and do_mprotect creating an exported do_unshare.
> > +
> > +/*
> > + * file changes invalidate isec
> > + */
> > +static int slm_file_permission(struct file *file, int mask)
> > +{
> > + struct slm_isec_data *isec = file->f_dentry->d_inode->i_security;
> > +
> > + if (((mask & MAY_WRITE) || (mask & MAY_APPEND)) && isec) {
> > + spin_lock(&isec->lock);
> > + isec->level.iac_level = SLM_IAC_NOTDEFINED;
> > + spin_unlock(&isec->lock);
> > + }
> > + return 0;
> > +}
>
> I don't understand the above - any write to a file by any process resets it to an undefined state?
> And then you just reset from xattr upon the next hook that happens to
> call get_level or the next d_instantiate? What is the point of that?
We had thought it was necessary to recheck the integrity data/metadata
after being invalidated by a write, but on reconsideration
SLIM permitted the write in the first place, so it is unnecessary.
>
> > +
> > +static int is_untrusted_blk_access(struct inode *inode)
> > +{
> > + struct slm_tsec_data *cur_tsec = current->security;
> > + int rc = 0;
> > +
> > + spin_lock(&cur_tsec->lock);
> > + if (cur_tsec && (cur_tsec->iac_wx == SLM_IAC_UNTRUSTED)
> > + && S_ISBLK(inode->i_mode))
> > + rc = 1;
> > + spin_unlock(&cur_tsec->lock);
> > + return rc;
> > +}
>
> This kind of special case logic is troubling, and shouldn't be necessary.
Unfortunately, this does seem necessary. Block devices don't have
xattrs and thus are treated as EXEMPT. However, they shouldn't be
written to except by SYSTEM (logic updated to enforce this rather than !
UNTRUSTED). We thought about special casing them to be SYSTEM
instead of EXEMPT or that might be able to do something like SELinux and
provide a daemon to label these files at boot, however, then special
casing would be necessary to avoid measuring these devices. You don't
really want /dev/hda1 rehashed everytime you write to the filesystem.
> > +/*
> > + * Premise:
> > + * Can't write or execute higher integrity, can't read lower integrity
> > + * Can't read or execute higher secrecy, can't write lower secrecy
>
> Misleading comment - no secrecy model within.
>
> > + */
> > +static int slm_inode_permission(struct inode *inode, int mask,
> > + struct nameidata *nd)
> > +{
> > + struct dentry *dentry = NULL;
> > + struct slm_file_xattr level;
> > +
> > + if (S_ISDIR(inode->i_mode) && (mask & MAY_WRITE))
> > + return 0;
> > +
> > + dentry = (!nd || !nd->dentry) ? d_find_alias(inode) : nd->dentry;
> > + if (!dentry)
> > + return 0;
>
> Relying on a dentry here is problematic.
>
> > +
> > + if (is_untrusted_blk_access(inode))
> > + return -EPERM;
> > +
> > + slm_get_level(dentry, &level);
>
> If the isec isn't already set up here, you have a bug.
> It shouldn't be necessary to go through this processing on every
> permission check.
>
> > +
> > + /* measure all SYSTEM level integrity objects */
> > + if (level.iac_level == SLM_IAC_SYSTEM)
> > + integrity_measure(dentry, NULL, mask);
>
> This seems to be the wrong point to perform a measurement.
>
The SLIM measure policy is to measure all system files and anything executable.
Things opened writable are not measured. Executables are caught in the
bprm_check_security hook. Measurement cannot be moved to d_instantiate for
example because you don't know what flags the file will be opened with.
The measure hook is really only using the dentry to get the inode I will
provide a patch to change the hook to accept an inode instead, though the
filename will sometimes not be available without a dentry.
> > +/* Create the security.slim.level extended attribute */
> > +static int slm_inode_init_security(struct inode *inode, struct inode *dir,
> > + char **name, void **value, size_t * len)
> > +{
> > + struct slm_isec_data *isec = inode->i_security, *parent_isec =
> > + dir->i_security;
> > + struct slm_tsec_data *cur_tsec = current->security;
> > + struct slm_file_xattr level;
> > + struct xattr_data *data;
> > + int rc;
> > +
> > + if (!name || !value || !len)
> > + return 0;
> > +
> > + memset(&level, 0, sizeof(struct slm_file_xattr));
> > +
> > + if (is_isec_defined(parent_isec)) {
> > + spin_lock(&parent_isec->lock);
> > + memcpy(&level, &parent_isec->level,
> > + sizeof(struct slm_file_xattr));
> > + spin_unlock(&parent_isec->lock);
> > + }
> > +
> > + spin_lock(&cur_tsec->lock);
> > + /* low integrity process wrote into a higher level directory */
> > + if (cur_tsec->iac_wx < level.iac_level)
> > + set_level_tsec_write(&level, cur_tsec);
> > + /* if directory is exempt, then use process level */
> > + if (is_iac_level_exempt(&level)) {
> > + /* When a guard process creates a directory */
> > + if (S_ISDIR(inode->i_mode)
> > + && (cur_tsec->iac_wx != cur_tsec->iac_r))
> > + set_level_exempt(&level);
> > + else
> > + set_level_tsec_write(&level, cur_tsec);
> > + }
> > +
> > + /* if a guard process creates a UNIX socket, then EXEMPT it */
> > + if (S_ISSOCK(inode->i_mode)
> > + && (cur_tsec->iac_wx != cur_tsec->iac_r))
> > + set_level_exempt(&level);
> > + spin_unlock(&cur_tsec->lock);
> > +
> > + spin_lock(&isec->lock);
> > + memcpy(&isec->level, &level, sizeof(struct slm_file_xattr));
> > + spin_unlock(&isec->lock);
> > +
> > + data = kmalloc(sizeof(struct xattr_data), GFP_KERNEL);
> > + if (!data)
> > + return -ENOMEM;
> > +
> > + /* set levels, based on parent */
> > + rc = slm_set_xattr(&level, &data->name, &data->value, &data->len);
> > + if (rc < 0) {
> > + kfree(data);
> > + return rc;
> > + }
> > +
> > + *name = data->name;
> > + *value = data->value;
> > + *len = data->len;
>
> What frees the xattr_data structure? Why does it even exist?
Not necessary so removed.
>
> > +static int slm_inode_rename(struct inode *old_dir,
> > + struct dentry *old_dentry,
> > + struct inode *new_dir, struct dentry *new_dentry)
> > +{
> > + struct slm_file_xattr old_level, parent_level;
> > + struct dentry *parent_dentry;
> > +
> > + if (old_dir == new_dir)
> > + return 0;
> > +
> > + slm_get_level(old_dentry, &old_level);
> > +
> > + parent_dentry = dget_parent(new_dentry);
> > + slm_get_level(parent_dentry, &parent_level);
> > + dput(parent_dentry);
> > +
> > + if (is_lower_integrity(&old_level, &parent_level))
> > + return -EPERM;
> > + return 0;
> > +}
>
> Why can't you get the parent level from old_dir if that is what you need?
> What is the rationale for this logic - renaming a file doesn't change its integrity.
Fixed to compare the original file level to the level of the new
directory. The logic is that moving a low integrity file to a high
integrity directory is a bad idea for numerous reasons one being you end
up demoting your shells doing tab completions on such directories.
>
> > +static void slm_inode_post_setxattr(struct dentry *dentry, char *name,
> > + void *value, size_t size, int flags)
> > +{
> > + struct slm_isec_data *slm_isec;
> > + struct slm_file_xattr level;
> > + int rc, status = 0;
> > +
> > + if (strncmp(name, XATTR_NAME, strlen(XATTR_NAME)) != 0)
> > + return;
>
> Not just here, but in several places you are using strncmp like this.
> Are you sure that is what you mean to test (versus an exact match)?
Fixed.
>
> > +
> > + rc = slm_get_xattr(dentry, &level, &status);
>
> Here you do a get_xattr, but this is post_setxattr - and you just ignore the supplied (value, size) pair?
>
Fixed this to use the value size pair with the slm_parse_xattr function
rather than recalculating.
> I'm out of time for now, but there is plenty more to comment on.
>
On Thu, 2006-10-19 at 13:48 -0700, Kylene Jo Hall wrote:
> On Tue, 2006-09-26 at 14:44 -0400, Stephen Smalley wrote:
> > Neither the above text nor your Documentation nor your code comments
> > seems to have been fully updated to reflect the changes to SLIM in this
> > submission, e.g. the removal of the secrecy MAC model and the change in
> > how you deal with "revocation". Thus they are misleading and confusing
> > to potential users and to subsequent maintenance of the code.
> I'll provide a patch that fixes the comments and in-tree documentation.
That won't fix the actual patch descriptions, so I'd suggest just making
up a new patch set with all fixes folded into it targeted at replacing
the current patch set in -mm. That would also facilitate full review of
the updated code as a whole, which I think is needed.
> > I think you would do better to just drop out the so-called "revocation"
> > code altogether and acknowledge this limitation in your Documentation.
> > Meanwhile, you could do a better job of using the permission checking
> > hooks that do exist to apply checks on subsequent operations after
> > demotion, like implementing file_receive, file_mmap, etc.
> >
> You are right in your AF_LOCAL socket concern. We have been working
> on that and have implemented the file_receive hook, however, once we
> have this hook we cannot imagine the case where file_mmap is also
> needed. If you still feel it is necessary can you please explain
> further.
I suppose that depends on the resolution of how you handle the
revocation issue and what you want to enforce and measure.
> --- linux-2.6.19-rc2/security/slim/slm_main.c 2006-10-19 12:05:58.000000000 -0700
> +++ linux-2.6.19-rc2/security/slim/slm_main.c 2006-10-19 12:11:37.000000000 -0700
> @@ -1130,6 +1103,34 @@ static int slm_socket_post_create(struct
> return 0;
> }
>
> +static int slm_file_receive(struct file *file)
> +{
> + struct slm_isec_data *isec = file->f_dentry->d_inode->i_security;
> + struct slm_tsec_data *tsec = current->security;
> + struct slm_file_xattr level;
> + int rc = 0;
> +
> + spin_lock(&isec->lock);
> + memcpy(&level, &isec->level, sizeof(struct slm_file_xattr));
> + spin_unlock(&isec->lock);
What are you achieving with your lock?
Suppose that you just did the following here without any locking:
enum slm_iac_level iac = isec->level.iac_level;
using that iac value in your subsequent checks, and suppose that in
post_setxattr you did the following likewise without any locking:
isec->level.iac_level = level->iac_level;
What difference does it make to your checks? Under both forms, it is
possible that the checks could be applied against the old value or the
new value when another thread is performing setxattr.
> + spin_lock(&tsec->lock);
Keeping in mind that tsec is per-task (thread) and that you no longer
try to demote other threads, what purpose does this lock serve?
> + if (file->f_mode & FMODE_READ) { /* IRAC(process) <= IAC(object) */
> + if (!is_iac_less_than_or_exempt(&level, tsec->iac_r))
> + rc = -EPERM;
> + }
> + if (file->f_mode & FMODE_WRITE) { /* IWXAC(process) >= IAC(object) */
> + if (!is_iac_greater_than_or_exempt(&level, tsec->iac_wx))
> + rc = -EPERM;
> + }
> + if (file->f_mode & FMODE_EXEC) { /* IWXAC(process) <= IAC(object) */
> + if (!is_iac_less_than_or_exempt(&level, tsec->iac_wx))
> + rc = -EPERM;
> + }
> + spin_unlock(&tsec->lock);
> + return rc;
> +}
> +
> As for removing revocation altogether (and keeping demotion) we are
> concerned that checking every read and write for example in the
> file_permission hook would be a signifcant performance hit over the
> revocation way.
A file_permission hook implementation for SLIM should be quite simple
and fast in the common case, so I think performance is not the issue
here.
> Additionally, Alan Cox brought up that demoting with an
> writeable open file really isnt' safe (http://marc.theaimsgroup.com/?
> l=linux-kernel&m=115643804205202&w=2) and revocation would still be
> necessary for mmaps as I don't know of a security hook that can be used
> to double check reads and writes to mmaps.
I believe Alan's point was that you need the core revoke() functionality
if you want to do this right. Instead, you chose to back off from real
revocation, yielding a change in your security model (an application can
now get a denial upon a read attempt if demotion cannot be performed
safely), different handling for open files vs. memory mappings
(confusing semantics), and code that is still neither simple nor safe.
> Is it really not possible to enumerate all the possible cases and
> mediate them?
It can be done, but you didn't seem interested in doing it the right
way. So if you aren't willing to do that, at least document the
limitations of your implementation.
> You were right about all the issues with this function. It is no longer
> needed thus negating the issue with the locking. Removing this and the
> reason it had been around (explanation below) also negates the need for
> the other is_isec_defined checks. I looked and the rest of the locking
> looks sane to me. Please point out any other particular instances you
> aren't sure about.
See my comments about the isec and tsec locking above - they apply
throughout.
> > This only checks for users of the mm (not necessarily other forms of
> > sharing), and it can yield "false" positives due to transient references
> > (also noted by Hugh). If you look at the SELinux code, you'll see a
> > more complex test applied in selinux_setprocattr after this test, and
> > that is only checking for mm sharing. For sharing of the file table,
> > there is the unsafe_exec checking that happens, with the corresponding
> > LSM_UNSAFE_SHARE check applied by SELinux (since we are only flushing
> > the table on context-changing exec).
>
> Yes the mm_users check as is will cause us to fail things that might not
> necessarily need to fail but at least we are failing closed.
...which makes it prone to easy denial of service by another process
(just accessing the /proc/pid nodes of the target).
> It seems that all the checks in sys_unshare should be done and those
> that can be unshared should be and if any sharing exists that can't be
> undone (such as unshare_vm) then the demotion would need to be
> prevented. We would implement this similar to how we split sys_mprotect
> and do_mprotect creating an exported do_unshare.
I don't think you want to automatically unshare state without
application awareness.
> > > +static int is_untrusted_blk_access(struct inode *inode)
> > > +{
> > > + struct slm_tsec_data *cur_tsec = current->security;
> > > + int rc = 0;
> > > +
> > > + spin_lock(&cur_tsec->lock);
> > > + if (cur_tsec && (cur_tsec->iac_wx == SLM_IAC_UNTRUSTED)
> > > + && S_ISBLK(inode->i_mode))
> > > + rc = 1;
> > > + spin_unlock(&cur_tsec->lock);
> > > + return rc;
> > > +}
> >
> > This kind of special case logic is troubling, and shouldn't be necessary.
>
> Unfortunately, this does seem necessary. Block devices don't have
> xattrs and thus are treated as EXEMPT.
Um, what? They certainly can have xattrs, and SELinux assigns an
attribute to them.
> However, they shouldn't be
> written to except by SYSTEM (logic updated to enforce this rather than !
> UNTRUSTED). We thought about special casing them to be SYSTEM
> instead of EXEMPT or that might be able to do something like SELinux and
> provide a daemon to label these files at boot, however, then special
> casing would be necessary to avoid measuring these devices. You don't
> really want /dev/hda1 rehashed everytime you write to the filesystem.
I don't follow. Device nodes can certainly support xattrs, and SELinux
has always labeled them. When udev came along, it was instrumented to
likewise handle labeling of the device nodes it creates. If someone
actually does write to the raw device, then ignoring that for
measurement makes your measurements rather meaningless. If someone
writes to a file within a mounted filesystem, there is no check against
the device node.
> The SLIM measure policy is to measure all system files and anything executable.
> Things opened writable are not measured. Executables are caught in the
> bprm_check_security hook. Measurement cannot be moved to d_instantiate for
> example because you don't know what flags the file will be opened with.
> The measure hook is really only using the dentry to get the inode I will
> provide a patch to change the hook to accept an inode instead, though the
> filename will sometimes not be available without a dentry.
So how does this differ from digsig (aside from the distinction between
measurement and signature verification, and aside from dealing with more
than just executables)? Why aren't you checking on
file_mmap/mprotect()? What good does it do to measure from
inode_permission() if the actual content that is subsequently read
doesn't match what you measured? etc. Seems like all the prior
discussions of digsig and IMA apply here.
--
Stephen Smalley
National Security Agency
On Thu, 2006-10-19 at 13:48 -0700, Kylene Jo Hall wrote:
> --- linux-2.6.19-rc2/security/slim/slm_main.c 2006-10-19 12:05:58.000000000 -0700
> +++ linux-2.6.19-rc2/security/slim/slm_main.c 2006-10-19 12:11:37.000000000 -0700
> @@ -1130,6 +1103,34 @@ static int slm_socket_post_create(struct
> return 0;
> }
>
> +static int slm_file_receive(struct file *file)
> +{
> + struct slm_isec_data *isec = file->f_dentry->d_inode->i_security;
> + struct slm_tsec_data *tsec = current->security;
> + struct slm_file_xattr level;
> + int rc = 0;
> +
> + spin_lock(&isec->lock);
> + memcpy(&level, &isec->level, sizeof(struct slm_file_xattr));
> + spin_unlock(&isec->lock);
> +
> + spin_lock(&tsec->lock);
> + if (file->f_mode & FMODE_READ) { /* IRAC(process) <= IAC(object) */
> + if (!is_iac_less_than_or_exempt(&level, tsec->iac_r))
> + rc = -EPERM;
> + }
> + if (file->f_mode & FMODE_WRITE) { /* IWXAC(process) >= IAC(object) */
> + if (!is_iac_greater_than_or_exempt(&level, tsec->iac_wx))
> + rc = -EPERM;
> + }
> + if (file->f_mode & FMODE_EXEC) { /* IWXAC(process) <= IAC(object) */
> + if (!is_iac_less_than_or_exempt(&level, tsec->iac_wx))
> + rc = -EPERM;
> + }
> + spin_unlock(&tsec->lock);
> + return rc;
> +}
Also, given your security model, why do you always have this hook deny
access rather than attempting to demote the task?
--
Stephen Smalley
National Security Agency