Skip to content

Commit

Permalink
UPSTREAM: selinux: sidtab reverse lookup hash table
Browse files Browse the repository at this point in the history
This replaces the reverse table lookup and reverse cache with a
hashtable which improves cache-miss reverse-lookup times from
O(n) to O(1)* and maintains the same performance as a reverse
cache hit.

This reduces the time needed to add a new sidtab entry from ~500us
to 5us on a Pixel 3 when there are ~10,000 sidtab entries.

The implementation uses the kernel's generic hashtable API,
It uses the context's string represtation as the hash source,
and the kernels generic string hashing algorithm full_name_hash()
to reduce the string to a 32 bit value.

This change also maintains the improvement introduced in
commit ee1a84fdfeed ("selinux: overhaul sidtab to fix bug and improve
performance") which removed the need to keep the current sidtab
locked during policy reload. It does however introduce periodic
locking of the target sidtab while converting the hashtable. Sidtab
entries are never modified or removed, so the context struct stored
in the sid_to_context tree can also be used for the context_to_sid
hashtable to reduce memory usage.

This bug was reported by:
- On the selinux bug tracker.
  BUG: kernel softlockup due to too many SIDs/contexts #37
  SELinuxProject/selinux-kernel#37
- Jovana Knezevic on Android's bugtracker.
  Bug: 140252993
  "During multi-user performance testing, we create and remove users
  many times. selinux_android_restorecon_pkgdir goes from 1ms to over
  20ms after about 200 user creations and removals. Accumulated over
  ~280 packages, that adds a significant time to user creation,
  making perf benchmarks unreliable."

* Hashtable lookup is only O(1) when n < the number of buckets.

Signed-off-by: Jeff Vander Stoep <jeffv@google.com>
Reported-by: Stephen Smalley <sds@tycho.nsa.gov>
Reported-by: Jovana Knezevic <jovanak@google.com>
Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>
Tested-by: Stephen Smalley <sds@tycho.nsa.gov>
[PM: subj tweak, removed changelog from patch description]
Signed-off-by: Paul Moore <paul@paul-moore.com>

Change-Id: Id7fa6405deec30604ffdf5c315e4bdbf305963ba
(Cherry picked from commit 66f8e2f03c02e812002f8e9e465681cc62edda5b)
Bug: 140252993
Signed-off-by: Jeff Vander Stoep <jeffv@google.com>
Signed-off-by: Rapherion Rollerscaperers <rapherion@raphielgang.org>
Signed-off-by: Chenyang Zhong <zhongcy95@gmail.com>
  • Loading branch information
jeffvanderstoep authored and Sorayukii committed Jun 2, 2024
1 parent 37d386b commit e2988f2
Show file tree
Hide file tree
Showing 9 changed files with 304 additions and 162 deletions.
12 changes: 12 additions & 0 deletions security/selinux/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,15 @@ config SECURITY_SELINUX_CHECKREQPROT_VALUE
via /selinux/checkreqprot if authorized by policy.

If you are unsure how to answer this question, answer 0.

config SECURITY_SELINUX_SIDTAB_HASH_BITS
int "NSA SELinux sidtab hashtable size"
depends on SECURITY_SELINUX
range 8 13
default 9
help
This option sets the number of buckets used in the sidtab hashtable
to 2^SECURITY_SELINUX_SIDTAB_HASH_BITS buckets. The number of hash
collisions may be viewed at /sys/fs/selinux/ss/sidtab_hash_stats. If
chain lengths are high (e.g. > 20) then selecting a higher value here
will ensure that lookups times are short and stable.
1 change: 1 addition & 0 deletions security/selinux/include/security.h
Original file line number Diff line number Diff line change
Expand Up @@ -365,5 +365,6 @@ extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
extern void avtab_cache_init(void);
extern void ebitmap_cache_init(void);
extern void hashtab_cache_init(void);
extern int security_sidtab_hash_stats(struct selinux_state *state, char *page);

#endif /* _SELINUX_SECURITY_H_ */
65 changes: 65 additions & 0 deletions security/selinux/selinuxfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1486,6 +1486,32 @@ static ssize_t sel_read_avc_hash_stats(struct file *filp, char __user *buf,
return length;
}

static ssize_t sel_read_sidtab_hash_stats(struct file *filp, char __user *buf,
size_t count, loff_t *ppos)
{
struct selinux_fs_info *fsi = file_inode(filp)->i_sb->s_fs_info;
struct selinux_state *state = fsi->state;
char *page;
ssize_t length;

page = (char *)__get_free_page(GFP_KERNEL);
if (!page)
return -ENOMEM;

length = security_sidtab_hash_stats(state, page);
if (length >= 0)
length = simple_read_from_buffer(buf, count, ppos, page,
length);
free_page((unsigned long)page);

return length;
}

static const struct file_operations sel_sidtab_hash_stats_ops = {
.read = sel_read_sidtab_hash_stats,
.llseek = generic_file_llseek,
};

static const struct file_operations sel_avc_cache_threshold_ops = {
.read = sel_read_avc_cache_threshold,
.write = sel_write_avc_cache_threshold,
Expand Down Expand Up @@ -1602,6 +1628,37 @@ static int sel_make_avc_files(struct dentry *dir)
return 0;
}

static int sel_make_ss_files(struct dentry *dir)
{
struct super_block *sb = dir->d_sb;
struct selinux_fs_info *fsi = sb->s_fs_info;
int i;
static struct tree_descr files[] = {
{ "sidtab_hash_stats", &sel_sidtab_hash_stats_ops, S_IRUGO },
};

for (i = 0; i < ARRAY_SIZE(files); i++) {
struct inode *inode;
struct dentry *dentry;

dentry = d_alloc_name(dir, files[i].name);
if (!dentry)
return -ENOMEM;

inode = sel_make_inode(dir->d_sb, S_IFREG|files[i].mode);
if (!inode) {
dput(dentry);
return -ENOMEM;
}

inode->i_fop = files[i].ops;
inode->i_ino = ++fsi->last_ino;
d_add(dentry, inode);
}

return 0;
}

static ssize_t sel_read_initcon(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
{
Expand Down Expand Up @@ -1956,6 +2013,14 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
}

ret = sel_make_avc_files(dentry);

dentry = sel_make_dir(sb->s_root, "ss", &fsi->last_ino);
if (IS_ERR(dentry)) {
ret = PTR_ERR(dentry);
goto err;
}

ret = sel_make_ss_files(dentry);
if (ret)
goto err;

Expand Down
11 changes: 10 additions & 1 deletion security/selinux/ss/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ struct context {
u32 len; /* length of string in bytes */
struct mls_range range;
char *str; /* string representation if context cannot be mapped. */
u32 hash; /* a hash of the string representation */
};

static inline void mls_context_init(struct context *c)
Expand Down Expand Up @@ -135,12 +136,13 @@ static inline int context_cpy(struct context *dst, struct context *src)
kfree(dst->str);
return rc;
}
dst->hash = src->hash;
return 0;
}

static inline void context_destroy(struct context *c)
{
c->user = c->role = c->type = 0;
c->user = c->role = c->type = c->hash = 0;
kfree(c->str);
c->str = NULL;
c->len = 0;
Expand All @@ -149,6 +151,8 @@ static inline void context_destroy(struct context *c)

static inline int context_cmp(struct context *c1, struct context *c2)
{
if (c1->hash && c2->hash && (c1->hash != c2->hash))
return 0;
if (c1->len && c2->len)
return (c1->len == c2->len && !strcmp(c1->str, c2->str));
if (c1->len || c2->len)
Expand All @@ -159,5 +163,10 @@ static inline int context_cmp(struct context *c1, struct context *c2)
mls_context_cmp(c1, c2));
}

static inline unsigned int context_compute_hash(const char *s)
{
return full_name_hash(NULL, s, strlen(s));
}

#endif /* _SS_CONTEXT_H_ */

5 changes: 5 additions & 0 deletions security/selinux/ss/policydb.c
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,11 @@ int policydb_load_isids(struct policydb *p, struct sidtab *s)
sidtab_destroy(s);
goto out;
}
rc = context_add_hash(p, &c->context[0]);
if (rc) {
sidtab_destroy(s);
goto out;
}

rc = sidtab_set_initial(s, c->sid[0], &c->context[0]);
if (rc) {
Expand Down
89 changes: 70 additions & 19 deletions security/selinux/ss/services.c
Original file line number Diff line number Diff line change
Expand Up @@ -1272,6 +1272,17 @@ static int context_struct_to_string(struct policydb *p,

#include "initial_sid_to_string.h"

int security_sidtab_hash_stats(struct selinux_state *state, char *page)
{
int rc;

read_lock(&state->ss->policy_rwlock);
rc = sidtab_hash_stats(state->ss->sidtab, page);
read_unlock(&state->ss->policy_rwlock);

return rc;
}

const char *security_get_initial_sid_context(u32 sid)
{
if (unlikely(sid > SECINITSID_NUM))
Expand Down Expand Up @@ -1440,6 +1451,42 @@ static int string_to_context_struct(struct policydb *pol,
return rc;
}

int context_add_hash(struct policydb *policydb,
struct context *context)
{
int rc;
char *str;
int len;

if (context->str) {
context->hash = context_compute_hash(context->str);
} else {
rc = context_struct_to_string(policydb, context,
&str, &len);
if (rc)
return rc;
context->hash = context_compute_hash(str);
kfree(str);
}
return 0;
}

static int context_struct_to_sid(struct selinux_state *state,
struct context *context, u32 *sid)
{
int rc;
struct sidtab *sidtab = state->ss->sidtab;
struct policydb *policydb = &state->ss->policydb;

if (!context->hash) {
rc = context_add_hash(policydb, context);
if (rc)
return rc;
}

return sidtab_context_to_sid(sidtab, context, sid);
}

static int security_context_to_sid_core(struct selinux_state *state,
const char *scontext, u32 scontext_len,
u32 *sid, u32 def_sid, gfp_t gfp_flags,
Expand Down Expand Up @@ -1492,7 +1539,7 @@ static int security_context_to_sid_core(struct selinux_state *state,
str = NULL;
} else if (rc)
goto out_unlock;
rc = sidtab_context_to_sid(sidtab, &context, sid);
rc = context_struct_to_sid(state, &context, sid);
context_destroy(&context);
out_unlock:
read_unlock(&state->ss->policy_rwlock);
Expand Down Expand Up @@ -1793,7 +1840,7 @@ static int security_compute_sid(struct selinux_state *state,
goto out_unlock;
}
/* Obtain the sid for the context. */
rc = sidtab_context_to_sid(sidtab, &newcontext, out_sid);
rc = context_struct_to_sid(state, &newcontext, out_sid);
out_unlock:
read_unlock(&state->ss->policy_rwlock);
context_destroy(&newcontext);
Expand Down Expand Up @@ -1945,6 +1992,7 @@ static int convert_context(struct context *oldc, struct context *newc, void *p)
context_init(newc);
newc->str = s;
newc->len = oldc->len;
newc->hash = oldc->hash;
return 0;
}
kfree(s);
Expand Down Expand Up @@ -2021,6 +2069,10 @@ static int convert_context(struct context *oldc, struct context *newc, void *p)
goto bad;
}

rc = context_add_hash(args->newp, newc);
if (rc)
goto bad;

return 0;
bad:
/* Map old representation to string and save it. */
Expand All @@ -2030,6 +2082,7 @@ static int convert_context(struct context *oldc, struct context *newc, void *p)
context_destroy(newc);
newc->str = s;
newc->len = len;
newc->hash = context_compute_hash(s);
pr_info("SELinux: Context %s became invalid (unmapped).\n",
newc->str);
return 0;
Expand Down Expand Up @@ -2268,8 +2321,7 @@ int security_port_sid(struct selinux_state *state,

if (c) {
if (!c->sid[0]) {
rc = sidtab_context_to_sid(sidtab,
&c->context[0],
rc = context_struct_to_sid(state, &c->context[0],
&c->sid[0]);
if (rc)
goto out;
Expand Down Expand Up @@ -2311,13 +2363,11 @@ int security_netif_sid(struct selinux_state *state,

if (c) {
if (!c->sid[0] || !c->sid[1]) {
rc = sidtab_context_to_sid(sidtab,
&c->context[0],
&c->sid[0]);
rc = context_struct_to_sid(state, &c->context[0],
&c->sid[0]);
if (rc)
goto out;
rc = sidtab_context_to_sid(sidtab,
&c->context[1],
rc = context_struct_to_sid(state, &c->context[1],
&c->sid[1]);
if (rc)
goto out;
Expand Down Expand Up @@ -2358,14 +2408,12 @@ int security_node_sid(struct selinux_state *state,
u32 *out_sid)
{
struct policydb *policydb;
struct sidtab *sidtab;
int rc;
struct ocontext *c;

read_lock(&state->ss->policy_rwlock);

policydb = &state->ss->policydb;
sidtab = state->ss->sidtab;

switch (domain) {
case AF_INET: {
Expand Down Expand Up @@ -2407,7 +2455,7 @@ int security_node_sid(struct selinux_state *state,

if (c) {
if (!c->sid[0]) {
rc = sidtab_context_to_sid(sidtab,
rc = context_struct_to_sid(state,
&c->context[0],
&c->sid[0]);
if (rc)
Expand Down Expand Up @@ -2491,12 +2539,17 @@ int security_get_user_sids(struct selinux_state *state,
usercon.role = i + 1;
ebitmap_for_each_positive_bit(&role->types, tnode, j) {
usercon.type = j + 1;
/*
* The same context struct is reused here so the hash
* must be reset.
*/
usercon.hash = 0;

if (mls_setup_user_range(policydb, fromcon, user,
&usercon))
continue;

rc = sidtab_context_to_sid(sidtab, &usercon, &sid);
rc = context_struct_to_sid(state, &usercon, &sid);
if (rc)
goto out_unlock;
if (mynel < maxnel) {
Expand Down Expand Up @@ -2567,7 +2620,6 @@ static inline int __security_genfs_sid(struct selinux_state *state,
u32 *sid)
{
struct policydb *policydb = &state->ss->policydb;
struct sidtab *sidtab = state->ss->sidtab;
int len;
u16 sclass;
struct genfs *genfs;
Expand Down Expand Up @@ -2602,7 +2654,7 @@ static inline int __security_genfs_sid(struct selinux_state *state,
goto out;

if (!c->sid[0]) {
rc = sidtab_context_to_sid(sidtab, &c->context[0], &c->sid[0]);
rc = context_struct_to_sid(state, &c->context[0], &c->sid[0]);
if (rc)
goto out;
}
Expand Down Expand Up @@ -2665,7 +2717,7 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb)
if (c) {
sbsec->behavior = c->v.behavior;
if (!c->sid[0]) {
rc = sidtab_context_to_sid(sidtab, &c->context[0],
rc = context_struct_to_sid(state, &c->context[0],
&c->sid[0]);
if (rc)
goto out;
Expand Down Expand Up @@ -2912,8 +2964,7 @@ int security_sid_mls_copy(struct selinux_state *state,
goto out_unlock;
}
}

rc = sidtab_context_to_sid(sidtab, &newcon, new_sid);
rc = context_struct_to_sid(state, &newcon, new_sid);
out_unlock:
read_unlock(&state->ss->policy_rwlock);
context_destroy(&newcon);
Expand Down Expand Up @@ -3501,7 +3552,7 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state,
if (!mls_context_isvalid(policydb, &ctx_new))
goto out_free;

rc = sidtab_context_to_sid(sidtab, &ctx_new, sid);
rc = context_struct_to_sid(state, &ctx_new, sid);
if (rc)
goto out_free;

Expand Down
Loading

0 comments on commit e2988f2

Please sign in to comment.