selinux: check level category sets once at load time

As reported by Jiri Vozar, commit 7edea6e8c8e8 ("selinux: beef up
isvalid checks") introduces a new loop in mls_level_isvalid() that
causes ~89-94% throughput regression in System V IPC message queue
operations (msgsnd/msgrcv).

Move the expensive part of the ebitmap checking to policy load time
instead as the reporter suggested.

Link: https://lore.kernel.org/selinux/CAMgFczCi2Z011dNf84Amc0Q-qnTt0+VUjWY+Y7zPyXdaH35Jvw@mail.gmail.com/
Fixes: 7edea6e8c8e8 ("selinux: beef up isvalid checks")
Reported-by: Jiri Vozar <jvozar@redhat.com>
Suggested-by: Jiri Vozar <jvozar@redhat.com>
Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>

diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index 3cd36e2015fa22bcad0a70236724c053078090e0..40c62600650e8e950d09c9638bf2dc1da2709a8b 100644 (file)
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -160,9 +160,6 @@ bool mls_level_isvalid(const struct policydb *p, const struct mls_level *l)
 {
        const char *name;
        const struct level_datum *levdatum;
-       struct ebitmap_node *node;
-       u32 bit;
-       int rc;
 
        if (!l->sens || l->sens > p->p_levels.nprim)
                return false;
@@ -176,21 +173,14 @@ bool mls_level_isvalid(const struct policydb *p, const struct mls_level *l)
                return false;
 
        /*
-        * Validate that all bits set in l->cat are also be set in
-        * levdatum->level->cat and no bit in l->cat is larger than
-        * p->p_cats.nprim.
+        * l is valid iff every bit in l->cat is set in levdatum->level.cat
+        * and no bit in l->cat is larger than p->p_cats.nprim.
+        * policydb_index() has already verified that every bit set in
+        * levdatum->level.cat names a defined category, so containment is
+        * sufficient here.
         */
-       rc = ebitmap_contains(&levdatum->level.cat, &l->cat,
-                             p->p_cats.nprim);
-       if (!rc)
-               return false;
-
-       ebitmap_for_each_positive_bit(&levdatum->level.cat, node, bit) {
-               if (!sym_name(p, SYM_CATS, bit))
-                       return false;
-       }
-
-       return true;
+       return ebitmap_contains(&levdatum->level.cat, &l->cat,
+                               p->p_cats.nprim);
 }
 
 bool mls_range_isvalid(const struct policydb *p, const struct mls_range *r)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index d88713201be92a6e2100a113c10ebc15504ef702..8a32666c0ba22fa938f1ca2f40880dd0ded58eb7 100644 (file)
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -665,6 +665,23 @@ static int cat_index(void *key, void *datum, void *datap)
        return 0;
 }
 
+static int sens_cat_index_check(void *key, void *datum, void *datap)
+{
+       struct policydb *p = datap;
+       struct level_datum *levdatum = datum;
+       struct ebitmap_node *node;
+       u32 bit;
+
+       ebitmap_for_each_positive_bit(&levdatum->level.cat, node, bit) {
+               if (bit >= p->p_cats.nprim || !sym_name(p, SYM_CATS, bit)) {
+                       pr_err("SELinux: sensitivity %s allows undefined category %u\n",
+                               (const char *)key, bit + 1);
+                       return -EINVAL;
+               }
+       }
+       return 0;
+}
+
 /* clang-format off */
 static int (*const index_f[SYM_NUM])(void *key, void *datum, void *datap) = {
        common_index,
@@ -788,6 +805,12 @@ static int policydb_index(struct policydb *p)
                }
        }
 
+       if (p->mls_enabled) {
+               rc = hashtab_map(&p->p_levels.table, sens_cat_index_check, p);
+               if (rc)
+                       goto out;
+       }
+
        rc = 0;
 out:
        return rc;