Skip to content

Commit ee1a84f

Browse files
WOnder93pcmoore
authored andcommitted
selinux: overhaul sidtab to fix bug and improve performance
Before this patch, during a policy reload the sidtab would become frozen and trying to map a new context to SID would be unable to add a new entry to sidtab and fail with -ENOMEM. Such failures are usually propagated into userspace, which has no way of distignuishing them from actual allocation failures and thus doesn't handle them gracefully. Such situation can be triggered e.g. by the following reproducer: while true; do load_policy; echo -n .; sleep 0.1; done & for (( i = 0; i < 1024; i++ )); do runcon -l s0:c$i echo -n x || break # or: # chcon -l s0:c$i <some_file> || break done This patch overhauls the sidtab so it doesn't need to be frozen during policy reload, thus solving the above problem. The new SID table leverages the fact that SIDs are allocated sequentially and are never invalidated and stores them in linear buckets indexed by a tree structure. This brings several advantages: 1. Fast SID -> context lookup - this lookup can now be done in logarithmic time complexity (usually in less than 4 array lookups) and can still be done safely without locking. 2. No need to re-search the whole table on reverse lookup miss - after acquiring the spinlock only the newly added entries need to be searched, which means that reverse lookups that end up inserting a new entry are now about twice as fast. 3. No need to freeze sidtab during policy reload - it is now possible to handle insertion of new entries even during sidtab conversion. The tree structure of the new sidtab is able to grow automatically to up to about 2^31 entries (at which point it should not have more than about 4 tree levels). The old sidtab had a theoretical capacity of almost 2^32 entries, but half of that is still more than enough since by that point the reverse table lookups would become unusably slow anyway... The number of entries per tree node is selected automatically so that each node fits into a single page, which should be the easiest size for kmalloc() to handle. Note that the cache for reverse lookup is preserved with equivalent logic. The only difference is that instead of storing pointers to the hash table nodes it stores just the indices of the cached entries. The new cache ensures that the indices are loaded/stored atomically, but it still has the drawback that concurrent cache updates may mess up the contents of the cache. Such situation however only reduces its effectivity, not the correctness of lookups. Tested by selinux-testsuite and thoroughly tortured by this simple stress test: ``` function rand_cat() { echo $(( $RANDOM % 1024 )) } function do_work() { while true; do echo -n "system_u:system_r:kernel_t:s0:c$(rand_cat),c$(rand_cat)" \ >/sys/fs/selinux/context 2>/dev/null || true done } do_work >/dev/null & do_work >/dev/null & do_work >/dev/null & while load_policy; do echo -n .; sleep 0.1; done kill %1 kill %2 kill %3 ``` Link: SELinuxProject/selinux-kernel#38 Reported-by: Orion Poplawski <[email protected]> Reported-by: Li Kun <[email protected]> Signed-off-by: Ondrej Mosnacek <[email protected]> Reviewed-by: Stephen Smalley <[email protected]> [PM: most of sidtab.c merged by hand due to conflicts] [PM: checkpatch fixes in mls.c, services.c, sidtab.c] Signed-off-by: Paul Moore <[email protected]>
1 parent 24ed7fd commit ee1a84f

File tree

5 files changed

+468
-324
lines changed

5 files changed

+468
-324
lines changed

security/selinux/ss/mls.c

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -436,16 +436,17 @@ int mls_setup_user_range(struct policydb *p,
436436

437437
/*
438438
* Convert the MLS fields in the security context
439-
* structure `c' from the values specified in the
440-
* policy `oldp' to the values specified in the policy `newp'.
439+
* structure `oldc' from the values specified in the
440+
* policy `oldp' to the values specified in the policy `newp',
441+
* storing the resulting context in `newc'.
441442
*/
442443
int mls_convert_context(struct policydb *oldp,
443444
struct policydb *newp,
444-
struct context *c)
445+
struct context *oldc,
446+
struct context *newc)
445447
{
446448
struct level_datum *levdatum;
447449
struct cat_datum *catdatum;
448-
struct ebitmap bitmap;
449450
struct ebitmap_node *node;
450451
int l, i;
451452

@@ -455,28 +456,25 @@ int mls_convert_context(struct policydb *oldp,
455456
for (l = 0; l < 2; l++) {
456457
levdatum = hashtab_search(newp->p_levels.table,
457458
sym_name(oldp, SYM_LEVELS,
458-
c->range.level[l].sens - 1));
459+
oldc->range.level[l].sens - 1));
459460

460461
if (!levdatum)
461462
return -EINVAL;
462-
c->range.level[l].sens = levdatum->level->sens;
463+
newc->range.level[l].sens = levdatum->level->sens;
463464

464-
ebitmap_init(&bitmap);
465-
ebitmap_for_each_positive_bit(&c->range.level[l].cat, node, i) {
465+
ebitmap_for_each_positive_bit(&oldc->range.level[l].cat,
466+
node, i) {
466467
int rc;
467468

468469
catdatum = hashtab_search(newp->p_cats.table,
469470
sym_name(oldp, SYM_CATS, i));
470471
if (!catdatum)
471472
return -EINVAL;
472-
rc = ebitmap_set_bit(&bitmap, catdatum->value - 1, 1);
473+
rc = ebitmap_set_bit(&newc->range.level[l].cat,
474+
catdatum->value - 1, 1);
473475
if (rc)
474476
return rc;
475-
476-
cond_resched();
477477
}
478-
ebitmap_destroy(&c->range.level[l].cat);
479-
c->range.level[l].cat = bitmap;
480478
}
481479

482480
return 0;

security/selinux/ss/mls.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ int mls_range_set(struct context *context, struct mls_range *range);
4646

4747
int mls_convert_context(struct policydb *oldp,
4848
struct policydb *newp,
49-
struct context *context);
49+
struct context *oldc,
50+
struct context *newc);
5051

5152
int mls_compute_sid(struct policydb *p,
5253
struct context *scontext,

security/selinux/ss/services.c

Lines changed: 51 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1907,19 +1907,16 @@ struct convert_context_args {
19071907

19081908
/*
19091909
* Convert the values in the security context
1910-
* structure `c' from the values specified
1910+
* structure `oldc' from the values specified
19111911
* in the policy `p->oldp' to the values specified
1912-
* in the policy `p->newp'. Verify that the
1913-
* context is valid under the new policy.
1912+
* in the policy `p->newp', storing the new context
1913+
* in `newc'. Verify that the context is valid
1914+
* under the new policy.
19141915
*/
1915-
static int convert_context(u32 key,
1916-
struct context *c,
1917-
void *p)
1916+
static int convert_context(struct context *oldc, struct context *newc, void *p)
19181917
{
19191918
struct convert_context_args *args;
1920-
struct context oldc;
19211919
struct ocontext *oc;
1922-
struct mls_range *range;
19231920
struct role_datum *role;
19241921
struct type_datum *typdatum;
19251922
struct user_datum *usrdatum;
@@ -1929,76 +1926,65 @@ static int convert_context(u32 key,
19291926

19301927
args = p;
19311928

1932-
if (c->str) {
1933-
struct context ctx;
1934-
1935-
rc = -ENOMEM;
1936-
s = kstrdup(c->str, GFP_KERNEL);
1929+
if (oldc->str) {
1930+
s = kstrdup(oldc->str, GFP_KERNEL);
19371931
if (!s)
1938-
goto out;
1932+
return -ENOMEM;
19391933

19401934
rc = string_to_context_struct(args->newp, NULL, s,
1941-
&ctx, SECSID_NULL);
1942-
kfree(s);
1943-
if (!rc) {
1944-
pr_info("SELinux: Context %s became valid (mapped).\n",
1945-
c->str);
1946-
/* Replace string with mapped representation. */
1947-
kfree(c->str);
1948-
memcpy(c, &ctx, sizeof(*c));
1949-
goto out;
1950-
} else if (rc == -EINVAL) {
1935+
newc, SECSID_NULL);
1936+
if (rc == -EINVAL) {
19511937
/* Retain string representation for later mapping. */
1952-
rc = 0;
1953-
goto out;
1954-
} else {
1938+
context_init(newc);
1939+
newc->str = s;
1940+
newc->len = oldc->len;
1941+
return 0;
1942+
}
1943+
kfree(s);
1944+
if (rc) {
19551945
/* Other error condition, e.g. ENOMEM. */
19561946
pr_err("SELinux: Unable to map context %s, rc = %d.\n",
1957-
c->str, -rc);
1958-
goto out;
1947+
oldc->str, -rc);
1948+
return rc;
19591949
}
1950+
pr_info("SELinux: Context %s became valid (mapped).\n",
1951+
oldc->str);
1952+
return 0;
19601953
}
19611954

1962-
rc = context_cpy(&oldc, c);
1963-
if (rc)
1964-
goto out;
1955+
context_init(newc);
19651956

19661957
/* Convert the user. */
19671958
rc = -EINVAL;
19681959
usrdatum = hashtab_search(args->newp->p_users.table,
1969-
sym_name(args->oldp, SYM_USERS, c->user - 1));
1960+
sym_name(args->oldp,
1961+
SYM_USERS, oldc->user - 1));
19701962
if (!usrdatum)
19711963
goto bad;
1972-
c->user = usrdatum->value;
1964+
newc->user = usrdatum->value;
19731965

19741966
/* Convert the role. */
19751967
rc = -EINVAL;
19761968
role = hashtab_search(args->newp->p_roles.table,
1977-
sym_name(args->oldp, SYM_ROLES, c->role - 1));
1969+
sym_name(args->oldp, SYM_ROLES, oldc->role - 1));
19781970
if (!role)
19791971
goto bad;
1980-
c->role = role->value;
1972+
newc->role = role->value;
19811973

19821974
/* Convert the type. */
19831975
rc = -EINVAL;
19841976
typdatum = hashtab_search(args->newp->p_types.table,
1985-
sym_name(args->oldp, SYM_TYPES, c->type - 1));
1977+
sym_name(args->oldp,
1978+
SYM_TYPES, oldc->type - 1));
19861979
if (!typdatum)
19871980
goto bad;
1988-
c->type = typdatum->value;
1981+
newc->type = typdatum->value;
19891982

19901983
/* Convert the MLS fields if dealing with MLS policies */
19911984
if (args->oldp->mls_enabled && args->newp->mls_enabled) {
1992-
rc = mls_convert_context(args->oldp, args->newp, c);
1985+
rc = mls_convert_context(args->oldp, args->newp, oldc, newc);
19931986
if (rc)
19941987
goto bad;
1995-
} else if (args->oldp->mls_enabled && !args->newp->mls_enabled) {
1996-
/*
1997-
* Switching between MLS and non-MLS policy:
1998-
* free any storage used by the MLS fields in the
1999-
* context for all existing entries in the sidtab.
2000-
*/
2001-
mls_context_destroy(c);
20021988
} else if (!args->oldp->mls_enabled && args->newp->mls_enabled) {
20031989
/*
20041990
* Switching between non-MLS and MLS policy:
@@ -2016,38 +2002,30 @@ static int convert_context(u32 key,
20162002
" the initial SIDs list\n");
20172003
goto bad;
20182004
}
2019-
range = &oc->context[0].range;
2020-
rc = mls_range_set(c, range);
2005+
rc = mls_range_set(newc, &oc->context[0].range);
20212006
if (rc)
20222007
goto bad;
20232008
}
20242009

20252010
/* Check the validity of the new context. */
2026-
if (!policydb_context_isvalid(args->newp, c)) {
2027-
rc = convert_context_handle_invalid_context(args->state,
2028-
&oldc);
2011+
if (!policydb_context_isvalid(args->newp, newc)) {
2012+
rc = convert_context_handle_invalid_context(args->state, oldc);
20292013
if (rc)
20302014
goto bad;
20312015
}
20322016

2033-
context_destroy(&oldc);
2034-
2035-
rc = 0;
2036-
out:
2037-
return rc;
2017+
return 0;
20382018
bad:
20392019
/* Map old representation to string and save it. */
2040-
rc = context_struct_to_string(args->oldp, &oldc, &s, &len);
2020+
rc = context_struct_to_string(args->oldp, oldc, &s, &len);
20412021
if (rc)
20422022
return rc;
2043-
context_destroy(&oldc);
2044-
context_destroy(c);
2045-
c->str = s;
2046-
c->len = len;
2023+
context_destroy(newc);
2024+
newc->str = s;
2025+
newc->len = len;
20472026
pr_info("SELinux: Context %s became invalid (unmapped).\n",
2048-
c->str);
2049-
rc = 0;
2050-
goto out;
2027+
newc->str);
2028+
return 0;
20512029
}
20522030

20532031
static void security_load_policycaps(struct selinux_state *state)
@@ -2091,6 +2069,7 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len)
20912069
struct policydb *oldpolicydb, *newpolicydb;
20922070
struct selinux_mapping *oldmapping;
20932071
struct selinux_map newmap;
2072+
struct sidtab_convert_params convert_params;
20942073
struct convert_context_args args;
20952074
u32 seqno;
20962075
int rc = 0;
@@ -2147,12 +2126,6 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len)
21472126
goto out;
21482127
}
21492128

2150-
oldsidtab = state->ss->sidtab;
2151-
2152-
#if 0
2153-
sidtab_hash_eval(oldsidtab, "sids");
2154-
#endif
2155-
21562129
rc = policydb_read(newpolicydb, fp);
21572130
if (rc) {
21582131
kfree(newsidtab);
@@ -2184,14 +2157,21 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len)
21842157
goto err;
21852158
}
21862159

2160+
oldsidtab = state->ss->sidtab;
2161+
21872162
/*
21882163
* Convert the internal representations of contexts
21892164
* in the new SID table.
21902165
*/
21912166
args.state = state;
21922167
args.oldp = policydb;
21932168
args.newp = newpolicydb;
2194-
rc = sidtab_convert(oldsidtab, newsidtab, convert_context, &args);
2169+
2170+
convert_params.func = convert_context;
2171+
convert_params.args = &args;
2172+
convert_params.target = newsidtab;
2173+
2174+
rc = sidtab_convert(oldsidtab, &convert_params);
21952175
if (rc) {
21962176
pr_err("SELinux: unable to convert the internal"
21972177
" representation of contexts in the new SID"

0 commit comments

Comments
 (0)