Skip to content
This repository has been archived by the owner on Oct 28, 2022. It is now read-only.

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>

(cherry picked from commit 66f8e2f03c02e812002f8e9e465681cc62edda5b)
Bug: 140252993
Change-Id: Iead2a1d90731ae24fefec2a40af5ffdc457ac916
Signed-off-by: Jeff Vander Stoep <jeffv@google.com>
(cherry picked from commit 20810a2469745210a7b2b8e0f9f3b60a28305f43)
Signed-off-by: UtsavisGreat <utsavbalar1231@gmail.com>
  • Loading branch information
jeffvanderstoep authored and XeonDead committed Jan 12, 2020
1 parent 7364854 commit c667a7c
Show file tree
Hide file tree
Showing 9 changed files with 306 additions and 167 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 @@ -390,5 +390,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 @@ -1488,6 +1488,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 @@ -1603,6 +1629,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 @@ -1957,6 +2014,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 @@ -31,6 +31,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 @@ -136,12 +137,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 @@ -150,6 +152,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 @@ -160,5 +164,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 @@ -922,6 +922,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
Loading

0 comments on commit c667a7c

Please sign in to comment.