path: root/security/selinux/ss/services.c
diff options
authorJann Horn <jannh@google.com>2018-08-06 23:19:32 +0200
committerPaul Moore <paul@paul-moore.com>2018-09-05 17:47:09 -0400
commit95ffe194204ae3cef88d0b59be209204bbe9b3be (patch)
treea31906d241c0bc898d412423cf97ce8905f0347c /security/selinux/ss/services.c
parentselinux: fix mounting of cgroup2 under older policies (diff)
selinux: refactor mls_context_to_sid() and make it stricter
The intended behavior change for this patch is to reject any MLS strings that contain (trailing) garbage if p->mls_enabled is true. As suggested by Paul Moore, change mls_context_to_sid() so that the two parts of the range are extracted before the rest of the parsing. Because now we don't have to scan for two different separators simultaneously everywhere, we can actually switch to strchr() everywhere instead of the open-coded loops that scan for two separators at once. mls_context_to_sid() used to signal how much of the input string was parsed by updating `*scontext`. However, there is actually no case in which mls_context_to_sid() only parses a subset of the input and still returns a success (other than the buggy case with a second '-' in which it incorrectly claims to have consumed the entire string). Turn `scontext` into a simple pointer argument and stop redundantly checking whether the entire input was consumed in string_to_context_struct(). This also lets us remove the `scontext_len` argument from `string_to_context_struct()`. Signed-off-by: Jann Horn <jannh@google.com> [PM: minor merge fuzz in convert_context()] Signed-off-by: Paul Moore <paul@paul-moore.com>
Diffstat (limited to 'security/selinux/ss/services.c')
1 files changed, 4 insertions, 8 deletions
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index f3def298a90e..12e414394530 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1365,7 +1365,6 @@ int security_sid_to_context_force(struct selinux_state *state, u32 sid,
static int string_to_context_struct(struct policydb *pol,
struct sidtab *sidtabp,
char *scontext,
- u32 scontext_len,
struct context *ctx,
u32 def_sid)
@@ -1426,15 +1425,12 @@ static int string_to_context_struct(struct policydb *pol,
ctx->type = typdatum->value;
- rc = mls_context_to_sid(pol, oldc, &p, ctx, sidtabp, def_sid);
+ rc = mls_context_to_sid(pol, oldc, p, ctx, sidtabp, def_sid);
if (rc)
goto out;
- rc = -EINVAL;
- if ((p - scontext) < scontext_len)
- goto out;
/* Check the validity of the new context. */
+ rc = -EINVAL;
if (!policydb_context_isvalid(pol, ctx))
goto out;
rc = 0;
@@ -1489,7 +1485,7 @@ static int security_context_to_sid_core(struct selinux_state *state,
policydb = &state->ss->policydb;
sidtab = &state->ss->sidtab;
rc = string_to_context_struct(policydb, sidtab, scontext2,
- scontext_len, &context, def_sid);
+ &context, def_sid);
if (rc == -EINVAL && force) {
context.str = str;
context.len = strlen(str) + 1;
@@ -1958,7 +1954,7 @@ static int convert_context(u32 key,
goto out;
rc = string_to_context_struct(args->newp, NULL, s,
- c->len, &ctx, SECSID_NULL);
+ &ctx, SECSID_NULL);
if (!rc) {
pr_info("SELinux: Context %s became valid (mapped).\n",