[PATCH] security: enable atomic inode security labeling

The following patch set enables atomic security labeling of newly created
inodes by altering the fs code to invoke a new LSM hook to obtain the security
attribute to apply to a newly created inode and to set up the incore inode
security state during the inode creation transaction.  This parallels the
existing processing for setting ACLs on newly created inodes.  Otherwise, it
is possible for new inodes to be accessed by another thread via the dcache
prior to complete security setup (presently handled by the
post_create/mkdir/...  LSM hooks in the VFS) and a newly created inode may be
left unlabeled on the disk in the event of a crash.  SELinux presently works
around the issue by ensuring that the incore inode security label is
initialized to a special SID that is inaccessible to unprivileged processes
(in accordance with policy), thereby preventing inappropriate access but
potentially causing false denials on legitimate accesses.  A simple test
program demonstrates such false denials on SELinux, and the patch solves the
problem.  Similar such false denials have been encountered in real
applications.

This patch defines a new inode_init_security LSM hook to obtain the security
attribute to apply to a newly created inode and to set up the incore inode
security state for it, and adds a corresponding hook function implementation
to SELinux.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/include/linux/security.h b/include/linux/security.h
index 7aab6ab..d4f3b7a 100644 (file)
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -250,6 +250,25 @@ struct swap_info_struct;
  *     @inode contains the inode structure.
  *     Deallocate the inode security structure and set @inode->i_security to
  *     NULL. 
+ * @inode_init_security:
+ *     Obtain the security attribute name suffix and value to set on a newly
+ *     created inode and set up the incore security field for the new inode.
+ *     This hook is called by the fs code as part of the inode creation
+ *     transaction and provides for atomic labeling of the inode, unlike
+ *     the post_create/mkdir/... hooks called by the VFS.  The hook function
+ *     is expected to allocate the name and value via kmalloc, with the caller
+ *     being responsible for calling kfree after using them.
+ *     If the security module does not use security attributes or does
+ *     not wish to put a security attribute on this particular inode,
+ *     then it should return -EOPNOTSUPP to skip this processing.
+ *     @inode contains the inode structure of the newly created inode.
+ *     @dir contains the inode structure of the parent directory.
+ *     @name will be set to the allocated name suffix (e.g. selinux).
+ *     @value will be set to the allocated attribute value.
+ *     @len will be set to the length of the value.
+ *     Returns 0 if @name and @value have been successfully set,
+ *             -EOPNOTSUPP if no security attribute is needed, or
+ *             -ENOMEM on memory allocation failure.
  * @inode_create:
  *     Check permission to create a regular file.
  *     @dir contains inode structure of the parent of the new file.
@@ -1080,6 +1099,8 @@ struct security_operations {
 
        int (*inode_alloc_security) (struct inode *inode);      
        void (*inode_free_security) (struct inode *inode);
+       int (*inode_init_security) (struct inode *inode, struct inode *dir,
+                                   char **name, void **value, size_t *len);
        int (*inode_create) (struct inode *dir,
                             struct dentry *dentry, int mode);
        void (*inode_post_create) (struct inode *dir,
@@ -1442,6 +1463,17 @@ static inline void security_inode_free (struct inode *inode)
                return;
        security_ops->inode_free_security (inode);
 }
+
+static inline int security_inode_init_security (struct inode *inode,
+                                               struct inode *dir,
+                                               char **name,
+                                               void **value,
+                                               size_t *len)
+{
+       if (unlikely (IS_PRIVATE (inode)))
+               return -EOPNOTSUPP;
+       return security_ops->inode_init_security (inode, dir, name, value, len);
+}
        
 static inline int security_inode_create (struct inode *dir,
                                         struct dentry *dentry,
@@ -2171,6 +2203,15 @@ static inline int security_inode_alloc (struct inode *inode)
 
 static inline void security_inode_free (struct inode *inode)
 { }
+
+static inline int security_inode_init_security (struct inode *inode,
+                                               struct inode *dir,
+                                               char **name,
+                                               void **value,
+                                               size_t *len)
+{
+       return -EOPNOTSUPP;
+}
        
 static inline int security_inode_create (struct inode *dir,
                                         struct dentry *dentry,

diff --git a/security/dummy.c b/security/dummy.c
index 6ff8875..e8a00fa 100644 (file)
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -258,6 +258,12 @@ static void dummy_inode_free_security (struct inode *inode)
        return;
 }
 
+static int dummy_inode_init_security (struct inode *inode, struct inode *dir,
+                                     char **name, void **value, size_t *len)
+{
+       return -EOPNOTSUPP;
+}
+
 static int dummy_inode_create (struct inode *inode, struct dentry *dentry,
                               int mask)
 {
@@ -886,6 +892,7 @@ void security_fixup_ops (struct security_operations *ops)
        set_to_dummy_if_null(ops, sb_post_pivotroot);
        set_to_dummy_if_null(ops, inode_alloc_security);
        set_to_dummy_if_null(ops, inode_free_security);
+       set_to_dummy_if_null(ops, inode_init_security);
        set_to_dummy_if_null(ops, inode_create);
        set_to_dummy_if_null(ops, inode_post_create);
        set_to_dummy_if_null(ops, inode_link);

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 8641f88..63701fe 100644 (file)
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1274,6 +1274,7 @@ static int post_create(struct inode *dir,
        struct inode *inode;
        struct inode_security_struct *dsec;
        struct superblock_security_struct *sbsec;
+       struct inode_security_struct *isec;
        u32 newsid;
        char *context;
        unsigned int len;
@@ -1293,6 +1294,11 @@ static int post_create(struct inode *dir,
                return 0;
        }
 
+       isec = inode->i_security;
+
+       if (isec->security_attr_init)
+               return 0;
+
        if (tsec->create_sid && sbsec->behavior != SECURITY_FS_USE_MNTPOINT) {
                newsid = tsec->create_sid;
        } else {
@@ -2018,6 +2024,58 @@ static void selinux_inode_free_security(struct inode *inode)
        inode_free_security(inode);
 }
 
+static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
+                                      char **name, void **value,
+                                      size_t *len)
+{
+       struct task_security_struct *tsec;
+       struct inode_security_struct *dsec;
+       struct superblock_security_struct *sbsec;
+       struct inode_security_struct *isec;
+       u32 newsid;
+       int rc;
+       char *namep, *context;
+
+       tsec = current->security;
+       dsec = dir->i_security;
+       sbsec = dir->i_sb->s_security;
+       isec = inode->i_security;
+
+       if (tsec->create_sid && sbsec->behavior != SECURITY_FS_USE_MNTPOINT) {
+               newsid = tsec->create_sid;
+       } else {
+               rc = security_transition_sid(tsec->sid, dsec->sid,
+                                            inode_mode_to_security_class(inode->i_mode),
+                                            &newsid);
+               if (rc) {
+                       printk(KERN_WARNING "%s:  "
+                              "security_transition_sid failed, rc=%d (dev=%s "
+                              "ino=%ld)\n",
+                              __FUNCTION__,
+                              -rc, inode->i_sb->s_id, inode->i_ino);
+                       return rc;
+               }
+       }
+
+       inode_security_set_sid(inode, newsid);
+
+       namep = kstrdup(XATTR_SELINUX_SUFFIX, GFP_KERNEL);
+       if (!namep)
+               return -ENOMEM;
+       *name = namep;
+
+       rc = security_sid_to_context(newsid, &context, len);
+       if (rc) {
+               kfree(namep);
+               return rc;
+       }
+       *value = context;
+
+       isec->security_attr_init = 1;
+
+       return 0;
+}
+
 static int selinux_inode_create(struct inode *dir, struct dentry *dentry, int mask)
 {
        return may_create(dir, dentry, SECCLASS_FILE);
@@ -4298,6 +4356,7 @@ static struct security_operations selinux_ops = {
 
        .inode_alloc_security =         selinux_inode_alloc_security,
        .inode_free_security =          selinux_inode_free_security,
+       .inode_init_security =          selinux_inode_init_security,
        .inode_create =                 selinux_inode_create,
        .inode_post_create =            selinux_inode_post_create,
        .inode_link =                   selinux_inode_link,

diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 887937c..c515bc0 100644 (file)
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -46,6 +46,7 @@ struct inode_security_struct {
        unsigned char initialized;     /* initialization flag */
        struct semaphore sem;
        unsigned char inherit;         /* inherit SID from parent entry */
+       unsigned char security_attr_init; /* security attributes init flag */
 };
 
 struct file_security_struct {