selinux: try security xattr after genfs for kernfs filesystems
authorOndrej Mosnacek <omosnace@redhat.com>
Fri, 22 Feb 2019 14:57:14 +0000 (15:57 +0100)
committerPaul Moore <paul@paul-moore.com>
Thu, 21 Mar 2019 01:53:04 +0000 (21:53 -0400)
Since kernfs supports the security xattr handlers, we can simply use
these to determine the inode's context, dropping the need to update it
from kernfs explicitly using a security_inode_notifysecctx() call.

We achieve this by setting a new sbsec flag SE_SBGENFS_XATTR to all
mounts that are known to use kernfs under the hood and then fetching the
xattrs after determining the fallback genfs sid in
inode_doinit_with_dentry() when this flag is set.

This will allow implementing full security xattr support in kernfs and
removing the ...notifysecctx() call in a subsequent patch.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
[PM: more manual merge fixups]
Signed-off-by: Paul Moore <paul@paul-moore.com>
security/selinux/hooks.c
security/selinux/include/security.h

index 1d0b37af2444df6e3358dceb1e1c03522de4a731..085409b367949b637e265a2436567e18b3e82c60 100644 (file)
@@ -751,11 +751,13 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 
        if (!strcmp(sb->s_type->name, "debugfs") ||
            !strcmp(sb->s_type->name, "tracefs") ||
-           !strcmp(sb->s_type->name, "sysfs") ||
-           !strcmp(sb->s_type->name, "pstore") ||
+           !strcmp(sb->s_type->name, "pstore"))
+               sbsec->flags |= SE_SBGENFS;
+
+       if (!strcmp(sb->s_type->name, "sysfs") ||
            !strcmp(sb->s_type->name, "cgroup") ||
            !strcmp(sb->s_type->name, "cgroup2"))
-               sbsec->flags |= SE_SBGENFS;
+               sbsec->flags |= SE_SBGENFS | SE_SBGENFS_XATTR;
 
        if (!sbsec->behavior) {
                /*
@@ -1354,6 +1356,67 @@ static int selinux_genfs_get_sid(struct dentry *dentry,
        return rc;
 }
 
+static int inode_doinit_use_xattr(struct inode *inode, struct dentry *dentry,
+                                 u32 def_sid, u32 *sid)
+{
+#define INITCONTEXTLEN 255
+       char *context;
+       unsigned int len;
+       int rc;
+
+       len = INITCONTEXTLEN;
+       context = kmalloc(len + 1, GFP_NOFS);
+       if (!context)
+               return -ENOMEM;
+
+       context[len] = '\0';
+       rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len);
+       if (rc == -ERANGE) {
+               kfree(context);
+
+               /* Need a larger buffer.  Query for the right size. */
+               rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, NULL, 0);
+               if (rc < 0)
+                       return rc;
+
+               len = rc;
+               context = kmalloc(len + 1, GFP_NOFS);
+               if (!context)
+                       return -ENOMEM;
+
+               context[len] = '\0';
+               rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX,
+                                   context, len);
+       }
+       if (rc < 0) {
+               kfree(context);
+               if (rc != -ENODATA) {
+                       pr_warn("SELinux: %s:  getxattr returned %d for dev=%s ino=%ld\n",
+                               __func__, -rc, inode->i_sb->s_id, inode->i_ino);
+                       return rc;
+               }
+               *sid = def_sid;
+               return 0;
+       }
+
+       rc = security_context_to_sid_default(&selinux_state, context, rc, sid,
+                                            def_sid, GFP_NOFS);
+       if (rc) {
+               char *dev = inode->i_sb->s_id;
+               unsigned long ino = inode->i_ino;
+
+               if (rc == -EINVAL) {
+                       pr_notice_ratelimited("SELinux: inode=%lu on dev=%s was found to have an invalid context=%s.  This indicates you may need to relabel the inode or the filesystem in question.\n",
+                                             ino, dev, context);
+               } else {
+                       pr_warn("SELinux: %s:  context_to_sid(%s) returned %d for dev=%s ino=%ld\n",
+                               __func__, context, -rc, dev, ino);
+               }
+       }
+       kfree(context);
+       return 0;
+}
+
 /* The inode's security attributes must be initialized before first use. */
 static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dentry)
 {
@@ -1362,9 +1425,6 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
        u32 task_sid, sid = 0;
        u16 sclass;
        struct dentry *dentry;
-#define INITCONTEXTLEN 255
-       char *context = NULL;
-       unsigned len = 0;
        int rc = 0;
 
        if (isec->initialized == LABEL_INITIALIZED)
@@ -1432,72 +1492,11 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
                        goto out;
                }
 
-               len = INITCONTEXTLEN;
-               context = kmalloc(len+1, GFP_NOFS);
-               if (!context) {
-                       rc = -ENOMEM;
-                       dput(dentry);
-                       goto out;
-               }
-               context[len] = '\0';
-               rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len);
-               if (rc == -ERANGE) {
-                       kfree(context);
-
-                       /* Need a larger buffer.  Query for the right size. */
-                       rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, NULL, 0);
-                       if (rc < 0) {
-                               dput(dentry);
-                               goto out;
-                       }
-                       len = rc;
-                       context = kmalloc(len+1, GFP_NOFS);
-                       if (!context) {
-                               rc = -ENOMEM;
-                               dput(dentry);
-                               goto out;
-                       }
-                       context[len] = '\0';
-                       rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len);
-               }
+               rc = inode_doinit_use_xattr(inode, dentry, sbsec->def_sid,
+                                           &sid);
                dput(dentry);
-               if (rc < 0) {
-                       if (rc != -ENODATA) {
-                               pr_warn("SELinux: %s:  getxattr returned "
-                                      "%d for dev=%s ino=%ld\n", __func__,
-                                      -rc, inode->i_sb->s_id, inode->i_ino);
-                               kfree(context);
-                               goto out;
-                       }
-                       /* Map ENODATA to the default file SID */
-                       sid = sbsec->def_sid;
-                       rc = 0;
-               } else {
-                       rc = security_context_to_sid_default(&selinux_state,
-                                                            context, rc, &sid,
-                                                            sbsec->def_sid,
-                                                            GFP_NOFS);
-                       if (rc) {
-                               char *dev = inode->i_sb->s_id;
-                               unsigned long ino = inode->i_ino;
-
-                               if (rc == -EINVAL) {
-                                       if (printk_ratelimit())
-                                               pr_notice("SELinux: inode=%lu on dev=%s was found to have an invalid "
-                                                       "context=%s.  This indicates you may need to relabel the inode or the "
-                                                       "filesystem in question.\n", ino, dev, context);
-                               } else {
-                                       pr_warn("SELinux: %s:  context_to_sid(%s) "
-                                              "returned %d for dev=%s ino=%ld\n",
-                                              __func__, context, -rc, dev, ino);
-                               }
-                               kfree(context);
-                               /* Leave with the unlabeled SID */
-                               rc = 0;
-                               break;
-                       }
-               }
-               kfree(context);
+               if (rc)
+                       goto out;
                break;
        case SECURITY_FS_USE_TASK:
                sid = task_sid;
@@ -1548,9 +1547,21 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
                                goto out;
                        rc = selinux_genfs_get_sid(dentry, sclass,
                                                   sbsec->flags, &sid);
-                       dput(dentry);
-                       if (rc)
+                       if (rc) {
+                               dput(dentry);
                                goto out;
+                       }
+
+                       if ((sbsec->flags & SE_SBGENFS_XATTR) &&
+                           (inode->i_opflags & IOP_XATTR)) {
+                               rc = inode_doinit_use_xattr(inode, dentry,
+                                                           sid, &sid);
+                               if (rc) {
+                                       dput(dentry);
+                                       goto out;
+                               }
+                       }
+                       dput(dentry);
                }
                break;
        }
index b5b7c5aade8cfa0f206c6b2a4c70fc3e44f15426..111121281c4750dce8223bfedb4d7a5762da34ab 100644 (file)
@@ -58,6 +58,7 @@
 #define SE_SBINITIALIZED       0x0100
 #define SE_SBPROC              0x0200
 #define SE_SBGENFS             0x0400
+#define SE_SBGENFS_XATTR       0x0800
 
 #define CONTEXT_STR    "context"
 #define FSCONTEXT_STR  "fscontext"