| Message ID | 20190214095015.16032-2-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Allow initializing the kernfs node's secctx based on its parent | expand |
On 2/14/19 4:50 AM, Ondrej Mosnacek wrote: > 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> > --- > security/selinux/hooks.c | 160 +++++++++++++++------------- > security/selinux/include/security.h | 1 + > 2 files changed, 88 insertions(+), 73 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 81e012c66d95..7dea5b1a89a3 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -793,11 +793,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) { > /* > @@ -1392,6 +1394,71 @@ 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 = NULL; > + unsigned int len = 0; No need to initialize here since no uses or gotos prior to first assignment? > + int rc; > + > + *sid = def_sid; > + > + if (!(inode->i_opflags & IOP_XATTR)) > + return 0; Is this a change in behavior from before the patch? Would we have previously called __vfs_getxattr -> xattr_resolve_name and returned either -EIO (is_bad_inode) or -EOPNOTSUPP to the caller? Perhaps it is fine to return 0 with the default SID here, but wanted to check. > + > + 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; > + } > + 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) > { > @@ -1400,9 +1467,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) > @@ -1470,72 +1534,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; > @@ -1586,9 +1589,20 @@ 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) { > + rc = inode_doinit_use_xattr(inode, dentry, > + sid, &sid); > + if (rc) { > + dput(dentry); > + goto out; > + } > + } > + dput(dentry); > } > break; > } > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > index f68fb25b5702..6e5928f951da 100644 > --- a/security/selinux/include/security.h > +++ b/security/selinux/include/security.h > @@ -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=" >
On Thu, Feb 14, 2019 at 9:49 PM Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 2/14/19 4:50 AM, Ondrej Mosnacek wrote: > > 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> > > --- > > security/selinux/hooks.c | 160 +++++++++++++++------------- > > security/selinux/include/security.h | 1 + > > 2 files changed, 88 insertions(+), 73 deletions(-) > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index 81e012c66d95..7dea5b1a89a3 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -793,11 +793,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) { > > /* > > @@ -1392,6 +1394,71 @@ 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 = NULL; > > + unsigned int len = 0; > > No need to initialize here since no uses or gotos prior to first assignment? Yes, fixed locally. > > > + int rc; > > + > > + *sid = def_sid; > > + > > + if (!(inode->i_opflags & IOP_XATTR)) > > + return 0; > > Is this a change in behavior from before the patch? Would we have > previously called __vfs_getxattr -> xattr_resolve_name and returned > either -EIO (is_bad_inode) or -EOPNOTSUPP to the caller? Perhaps it is > fine to return 0 with the default SID here, but wanted to check. In the 'case SECURITY_FS_USE_XATTR' block (where this code was originally), there is the same check actually duplicated at the beginning (it is not visible in the diff context), for that code path this check is a no-op. Since moving the check into the function would possibly change the behavior there, I chose to duplicate the check inside the new function (and in the new call site we want to leave the default (genfs sid) and return 0). I realized now that checking for IOP_XATTR along with SE_SBGENFS_XATTR and removing the check from inode_doinit_use_xattr() actually makes more sense. Will do that in the next respin. > > > + > > + 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; > > + } > > + 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) > > { > > @@ -1400,9 +1467,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) > > @@ -1470,72 +1534,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; > > @@ -1586,9 +1589,20 @@ 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) { > > + rc = inode_doinit_use_xattr(inode, dentry, > > + sid, &sid); > > + if (rc) { > > + dput(dentry); > > + goto out; > > + } > > + } > > + dput(dentry); > > } > > break; > > } > > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > > index f68fb25b5702..6e5928f951da 100644 > > --- a/security/selinux/include/security.h > > +++ b/security/selinux/include/security.h > > @@ -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=" > > > -- Ondrej Mosnacek <omosnace at redhat dot com> Associate Software Engineer, Security Technologies Red Hat, Inc.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 81e012c66d95..7dea5b1a89a3 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -793,11 +793,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) { /* @@ -1392,6 +1394,71 @@ 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 = NULL; + unsigned int len = 0; + int rc; + + *sid = def_sid; + + if (!(inode->i_opflags & IOP_XATTR)) + return 0; + + 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; + } + 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) { @@ -1400,9 +1467,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) @@ -1470,72 +1534,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; @@ -1586,9 +1589,20 @@ 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) { + rc = inode_doinit_use_xattr(inode, dentry, + sid, &sid); + if (rc) { + dput(dentry); + goto out; + } + } + dput(dentry); } break; } diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index f68fb25b5702..6e5928f951da 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -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="
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> --- security/selinux/hooks.c | 160 +++++++++++++++------------- security/selinux/include/security.h | 1 + 2 files changed, 88 insertions(+), 73 deletions(-)