* [PATCH] selinux: parse contexts for mount options early
@ 2022-02-02 12:55 Ondrej Mosnacek
2022-02-04 19:30 ` Paul Moore
0 siblings, 1 reply; 2+ messages in thread
From: Ondrej Mosnacek @ 2022-02-02 12:55 UTC (permalink / raw)
To: Paul Moore; +Cc: selinux, Scott Mayhew
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@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] selinux: parse contexts for mount options early
2022-02-02 12:55 [PATCH] selinux: parse contexts for mount options early Ondrej Mosnacek
@ 2022-02-04 19:30 ` Paul Moore
0 siblings, 0 replies; 2+ messages in thread
From: Paul Moore @ 2022-02-04 19:30 UTC (permalink / raw)
To: Ondrej Mosnacek; +Cc: selinux, Scott Mayhew
On Wed, Feb 2, 2022 at 7:55 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> 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@redhat.com>
> ---
> security/selinux/hooks.c | 202 ++++++++++-----------------------------
> 1 file changed, 53 insertions(+), 149 deletions(-)
Merged into selinux/next, thanks.
Please keep an eye on line length in the future; I understand it is
considered poor form to split long error messages, but some of the
messages in this patch are unnecessarily long and wordy. You may have
inherited some of those messages from the current code, but that
doesn't mean you can't make them more concise.
--
paul-moore.com
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-02-04 19:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-02 12:55 [PATCH] selinux: parse contexts for mount options early Ondrej Mosnacek
2022-02-04 19:30 ` Paul Moore
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.