Commit b8b87fd954b4 ("selinux: Fix selinux_sb_mnt_opts_compat()") started to parse mount options into SIDs in selinux_add_opt() if policy has already been loaded. Since it's extremely unlikely that anyone would depend on the ability to set SELinux contexts on fs_context before loading the policy and then mounting that context after simplify the logic by always parsing the options early. Note that the multi-step mounting is only possible with the new fscontext mount API and wasn't possible before its introduction. Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx> --- security/selinux/hooks.c | 202 ++++++++++----------------------------- 1 file changed, 53 insertions(+), 149 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index b60481192b38..e2a3b872c0f9 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -340,7 +340,6 @@ static void inode_free_security(struct inode *inode) } struct selinux_mnt_opts { - const char *fscontext, *context, *rootcontext, *defcontext; u32 fscontext_sid; u32 context_sid; u32 rootcontext_sid; @@ -349,12 +348,7 @@ struct selinux_mnt_opts { static void selinux_free_mnt_opts(void *mnt_opts) { - struct selinux_mnt_opts *opts = mnt_opts; - kfree(opts->fscontext); - kfree(opts->context); - kfree(opts->rootcontext); - kfree(opts->defcontext); - kfree(opts); + kfree(mnt_opts); } enum { @@ -601,17 +595,6 @@ static int bad_option(struct superblock_security_struct *sbsec, char flag, return 0; } -static int parse_sid(struct super_block *sb, const char *s, u32 *sid) -{ - int rc = security_context_str_to_sid(&selinux_state, s, - sid, GFP_KERNEL); - if (rc) - pr_warn("SELinux: security_context_str_to_sid" - "(%s) failed for (dev %s, type %s) errno=%d\n", - s, sb ? sb->s_id : "?", sb ? sb->s_type->name : "?", rc); - return rc; -} - /* * Allow filesystems with binary mount data to explicitly set mount point * labeling information. @@ -674,49 +657,29 @@ static int selinux_set_mnt_opts(struct super_block *sb, * than once with different security options. */ if (opts) { - if (opts->fscontext) { - if (opts->fscontext_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->fscontext, &fscontext_sid); - if (rc) - goto out; - } else - fscontext_sid = opts->fscontext_sid; + if (opts->fscontext_sid) { + fscontext_sid = opts->fscontext_sid; if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, fscontext_sid)) goto out_double_mount; sbsec->flags |= FSCONTEXT_MNT; } - if (opts->context) { - if (opts->context_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->context, &context_sid); - if (rc) - goto out; - } else - context_sid = opts->context_sid; + if (opts->context_sid) { + context_sid = opts->context_sid; if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, context_sid)) goto out_double_mount; sbsec->flags |= CONTEXT_MNT; } - if (opts->rootcontext) { - if (opts->rootcontext_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->rootcontext, &rootcontext_sid); - if (rc) - goto out; - } else - rootcontext_sid = opts->rootcontext_sid; + if (opts->rootcontext_sid) { + rootcontext_sid = opts->rootcontext_sid; if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, rootcontext_sid)) goto out_double_mount; sbsec->flags |= ROOTCONTEXT_MNT; } - if (opts->defcontext) { - if (opts->defcontext_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->defcontext, &defcontext_sid); - if (rc) - goto out; - } else - defcontext_sid = opts->defcontext_sid; + if (opts->defcontext_sid) { + defcontext_sid = opts->defcontext_sid; if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, defcontext_sid)) goto out_double_mount; @@ -986,6 +949,8 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) { struct selinux_mnt_opts *opts = *mnt_opts; bool is_alloc_opts = false; + u32 *dst_sid; + int rc; if (token == Opt_seclabel) /* eaten and completely ignored */ @@ -993,6 +958,11 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) if (!s) return -ENOMEM; + if (!selinux_initialized(&selinux_state)) { + pr_warn("SELinux: Unable to set superblock options before the security server is initialized\n"); + return -EINVAL; + } + if (!opts) { opts = kzalloc(sizeof(*opts), GFP_KERNEL); if (!opts) @@ -1003,36 +973,34 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) switch (token) { case Opt_context: - if (opts->context || opts->defcontext) + if (opts->context_sid || opts->defcontext_sid) goto err; - opts->context = s; - if (selinux_initialized(&selinux_state)) - parse_sid(NULL, s, &opts->context_sid); + dst_sid = &opts->context_sid; break; case Opt_fscontext: - if (opts->fscontext) + if (opts->fscontext_sid) goto err; - opts->fscontext = s; - if (selinux_initialized(&selinux_state)) - parse_sid(NULL, s, &opts->fscontext_sid); + dst_sid = &opts->fscontext_sid; break; case Opt_rootcontext: - if (opts->rootcontext) + if (opts->rootcontext_sid) goto err; - opts->rootcontext = s; - if (selinux_initialized(&selinux_state)) - parse_sid(NULL, s, &opts->rootcontext_sid); + dst_sid = &opts->rootcontext_sid; break; case Opt_defcontext: - if (opts->context || opts->defcontext) + if (opts->context_sid || opts->defcontext_sid) goto err; - opts->defcontext = s; - if (selinux_initialized(&selinux_state)) - parse_sid(NULL, s, &opts->defcontext_sid); + dst_sid = &opts->defcontext_sid; break; + default: + WARN_ON(1); + return -EINVAL; } - - return 0; + rc = security_context_str_to_sid(&selinux_state, s, dst_sid, GFP_KERNEL); + if (rc) + pr_warn("SELinux: security_context_str_to_sid (%s) failed with errno=%d\n", + s, rc); + return rc; err: if (is_alloc_opts) { @@ -2681,37 +2649,27 @@ static int selinux_sb_mnt_opts_compat(struct super_block *sb, void *mnt_opts) if (!opts) return (sbsec->flags & SE_MNTMASK) ? 1 : 0; - if (opts->fscontext) { - if (opts->fscontext_sid == SECSID_NULL) - return 1; - else if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, - opts->fscontext_sid)) + if (opts->fscontext_sid) { + if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, + opts->fscontext_sid)) return 1; } - if (opts->context) { - if (opts->context_sid == SECSID_NULL) - return 1; - else if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, - opts->context_sid)) + if (opts->context_sid) { + if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, + opts->context_sid)) return 1; } - if (opts->rootcontext) { - if (opts->rootcontext_sid == SECSID_NULL) - return 1; - else { - struct inode_security_struct *root_isec; + if (opts->rootcontext_sid) { + struct inode_security_struct *root_isec; - root_isec = backing_inode_security(sb->s_root); - if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, - opts->rootcontext_sid)) - return 1; - } - } - if (opts->defcontext) { - if (opts->defcontext_sid == SECSID_NULL) + root_isec = backing_inode_security(sb->s_root); + if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, + opts->rootcontext_sid)) return 1; - else if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, - opts->defcontext_sid)) + } + if (opts->defcontext_sid) { + if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, + opts->defcontext_sid)) return 1; } return 0; @@ -2721,7 +2679,6 @@ static int selinux_sb_remount(struct super_block *sb, void *mnt_opts) { struct selinux_mnt_opts *opts = mnt_opts; struct superblock_security_struct *sbsec = selinux_superblock(sb); - int rc; if (!(sbsec->flags & SE_SBINITIALIZED)) return 0; @@ -2729,47 +2686,24 @@ static int selinux_sb_remount(struct super_block *sb, void *mnt_opts) if (!opts) return 0; - if (opts->fscontext) { - if (opts->fscontext_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->fscontext, - &opts->fscontext_sid); - if (rc) - return rc; - } + if (opts->fscontext_sid) { if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, opts->fscontext_sid)) goto out_bad_option; } - if (opts->context) { - if (opts->context_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->context, &opts->context_sid); - if (rc) - return rc; - } + if (opts->context_sid) { if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, opts->context_sid)) goto out_bad_option; } - if (opts->rootcontext) { + if (opts->rootcontext_sid) { struct inode_security_struct *root_isec; root_isec = backing_inode_security(sb->s_root); - if (opts->rootcontext_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->rootcontext, - &opts->rootcontext_sid); - if (rc) - return rc; - } if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, opts->rootcontext_sid)) goto out_bad_option; } - if (opts->defcontext) { - if (opts->defcontext_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->defcontext, - &opts->defcontext_sid); - if (rc) - return rc; - } + if (opts->defcontext_sid) { if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, opts->defcontext_sid)) goto out_bad_option; @@ -2838,42 +2772,12 @@ static int selinux_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc) { const struct selinux_mnt_opts *src = src_fc->security; - struct selinux_mnt_opts *opts; if (!src) return 0; - fc->security = kzalloc(sizeof(struct selinux_mnt_opts), GFP_KERNEL); - if (!fc->security) - return -ENOMEM; - - opts = fc->security; - - if (src->fscontext) { - opts->fscontext = kstrdup(src->fscontext, GFP_KERNEL); - if (!opts->fscontext) - return -ENOMEM; - } - if (src->context) { - opts->context = kstrdup(src->context, GFP_KERNEL); - if (!opts->context) - return -ENOMEM; - } - if (src->rootcontext) { - opts->rootcontext = kstrdup(src->rootcontext, GFP_KERNEL); - if (!opts->rootcontext) - return -ENOMEM; - } - if (src->defcontext) { - opts->defcontext = kstrdup(src->defcontext, GFP_KERNEL); - if (!opts->defcontext) - return -ENOMEM; - } - opts->fscontext_sid = src->fscontext_sid; - opts->context_sid = src->context_sid; - opts->rootcontext_sid = src->rootcontext_sid; - opts->defcontext_sid = src->defcontext_sid; - return 0; + fc->security = kmemdup(src, sizeof(*src), GFP_KERNEL); + return fc->security ? 0 : -ENOMEM; } static const struct fs_parameter_spec selinux_fs_parameters[] = { -- 2.34.1