From: Jacob Satterfield <jsatterfield.linux@gmail.com>
To: selinux@vger.kernel.org
Cc: Jacob Satterfield <jsatterfield.linux@gmail.com>,
stephen.smalley.work@gmail.com, paul@paul-moore.com,
omosnace@redhat.com
Subject: [PATCH v4 3/3] selinux: use arrays for avtab hashtable nodes
Date: Thu, 2 Nov 2023 15:45:24 +0000 [thread overview]
Message-ID: <20231102154524.12006-4-jsatterfield.linux@gmail.com> (raw)
In-Reply-To: <20231102154524.12006-1-jsatterfield.linux@gmail.com>
The current avtab hashtable employs a separate chaining collision
resolution strategy where each bucket/chain holds an ordered linked list
of pointers to kmem_cache allocated avtab_node elements.
On Fedora 38 (x86_64) using the default policy, avtab_node_cachep
uses 573 slabs each containing 170 objects totaling 2,337,840 bytes.
A call to kmem_cache_zalloc() is required for every single rule, which
in the default policy is currently 96,730 and continually rising.
When both sets of avtab_node (regular and cond.) are turned into arrays
with the hash table chain heads pointing into it, this results in only
two additional kvcalloc() calls and the complete removal of the
kmem_cache itself and its memory and runtime overheads.
Running "perf stat -r 100 -d load_policy" has shown a runtime reduction
of around 10% on a Fedora 38 x86_64 VM with this single patch. Future
patches focused on improving the hash table's collision resolution
strategy and array layout (struct-of-arrays vs. array-of-structs) may
elicit even more caching and therefore runtime performance improvements.
To prevent the conditional table from under-allocating the avtab_node
array, which creates a heap-overflow bug, the two-pass algorithm in the
patch "selinux: fix conditional avtab slot hint" is required.
Signed-off-by: Jacob Satterfield <jsatterfield.linux@gmail.com>
---
security/selinux/ss/avtab.c | 36 ++++++++++++++++++++----------------
security/selinux/ss/avtab.h | 2 ++
2 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index f437bd588b04..50c8db8eb511 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -24,7 +24,6 @@
#include "avtab.h"
#include "policydb.h"
-static struct kmem_cache *avtab_node_cachep __ro_after_init;
static struct kmem_cache *avtab_xperms_cachep __ro_after_init;
#define avtab_chain_for_each(pos, tab, slot) \
@@ -79,17 +78,15 @@ avtab_insert_node(struct avtab *h, struct avtab_node **dst,
{
struct avtab_node *newnode;
struct avtab_extended_perms *xperms;
- newnode = kmem_cache_zalloc(avtab_node_cachep, GFP_KERNEL);
- if (newnode == NULL)
+ if (h->nel == h->nnodes)
return NULL;
+ newnode = &h->nodes[h->nel];
newnode->key = *key;
if (key->specified & AVTAB_XPERMS) {
xperms = kmem_cache_zalloc(avtab_xperms_cachep, GFP_KERNEL);
- if (xperms == NULL) {
- kmem_cache_free(avtab_node_cachep, newnode);
+ if (xperms == NULL)
return NULL;
- }
*xperms = *(datum->u.xperms);
newnode->datum.u.xperms = xperms;
} else {
@@ -225,7 +222,7 @@ void avtab_destroy(struct avtab *h)
u32 i;
struct avtab_node *cur, *temp;
- if (!h)
+ if (!h || !h->htable)
return;
for (i = 0; i < h->nslot; i++) {
@@ -236,11 +233,13 @@ void avtab_destroy(struct avtab *h)
if (temp->key.specified & AVTAB_XPERMS)
kmem_cache_free(avtab_xperms_cachep,
temp->datum.u.xperms);
- kmem_cache_free(avtab_node_cachep, temp);
}
}
kvfree(h->htable);
+ kvfree(h->nodes);
h->htable = NULL;
+ h->nodes = NULL;
+ h->nnodes = 0;
h->nel = 0;
h->nslot = 0;
h->mask = 0;
@@ -249,20 +248,28 @@ void avtab_destroy(struct avtab *h)
void avtab_init(struct avtab *h)
{
h->htable = NULL;
+ h->nodes = NULL;
+ h->nnodes = 0;
h->nel = 0;
h->nslot = 0;
h->mask = 0;
}
-static int avtab_alloc_common(struct avtab *h, u32 nslot)
+static int avtab_alloc_common(struct avtab *h, u32 nslot, u32 nrules)
{
if (!nslot)
return 0;
- h->htable = kvcalloc(nslot, sizeof(void *), GFP_KERNEL);
+ h->htable = kvcalloc(nslot, sizeof(*h->htable), GFP_KERNEL);
if (!h->htable)
return -ENOMEM;
-
+ h->nodes = kvcalloc(nrules, sizeof(*h->nodes), GFP_KERNEL);
+ if (!h->nodes) {
+ kvfree(h->htable);
+ h->htable = NULL;
+ return -ENOMEM;
+ }
+ h->nnodes = nrules;
h->nslot = nslot;
h->mask = nslot - 1;
return 0;
@@ -278,7 +285,7 @@ int avtab_alloc(struct avtab *h, u32 nrules)
if (nslot > MAX_AVTAB_HASH_BUCKETS)
nslot = MAX_AVTAB_HASH_BUCKETS;
- rc = avtab_alloc_common(h, nslot);
+ rc = avtab_alloc_common(h, nslot, nrules);
if (rc)
return rc;
}
@@ -289,7 +296,7 @@ int avtab_alloc(struct avtab *h, u32 nrules)
int avtab_alloc_dup(struct avtab *new, const struct avtab *orig)
{
- return avtab_alloc_common(new, orig->nslot);
+ return avtab_alloc_common(new, orig->nslot, orig->nel);
}
#ifdef CONFIG_SECURITY_SELINUX_DEBUG
@@ -614,9 +621,6 @@ int avtab_write(struct policydb *p, struct avtab *a, void *fp)
void __init avtab_cache_init(void)
{
- avtab_node_cachep = kmem_cache_create("avtab_node",
- sizeof(struct avtab_node),
- 0, SLAB_PANIC, NULL);
avtab_xperms_cachep = kmem_cache_create("avtab_extended_perms",
sizeof(struct avtab_extended_perms),
0, SLAB_PANIC, NULL);
diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h
index 86fb6f793eec..5e465be6f057 100644
--- a/security/selinux/ss/avtab.h
+++ b/security/selinux/ss/avtab.h
@@ -82,6 +82,8 @@ struct avtab_node {
struct avtab {
struct avtab_node **htable;
+ struct avtab_node *nodes;
+ u32 nnodes; /* number of nodes */
u32 nel; /* number of elements */
u32 nslot; /* number of hash slots */
u32 mask; /* mask to compute hash func */
--
2.34.1
next prev parent reply other threads:[~2023-11-02 15:46 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-02 15:45 [PATCH v4 0/3] selinux: avtab arrays and refactors Jacob Satterfield
2023-11-02 15:45 ` [PATCH v4 1/3] selinux: refactor avtab_node comparisons Jacob Satterfield
2023-11-02 15:45 ` [PATCH v4 2/3] selinux: fix conditional avtab slot hint Jacob Satterfield
2023-11-02 15:45 ` Jacob Satterfield [this message]
2023-11-02 17:34 ` [PATCH v4 3/3] selinux: use arrays for avtab hashtable nodes Stephen Smalley
2023-11-02 21:06 ` Jacob Satterfield
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231102154524.12006-4-jsatterfield.linux@gmail.com \
--to=jsatterfield.linux@gmail.com \
--cc=omosnace@redhat.com \
--cc=paul@paul-moore.com \
--cc=selinux@vger.kernel.org \
--cc=stephen.smalley.work@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.