selinux: require a class's permission values to cover its permission count

security_get_permissions() sizes an array by the class's permissions.nprim
and fills it at value - 1, from the inherited common's permission table and
then the class's own. A value no permission defines leaves a NULL that
sel_make_perm_files() passes to d_alloc_name(), an oops inside
sel_write_load() that strands selinux_state.policy_mutex and leaves every
later load in uninterruptible sleep; two permissions sharing a value
overwrite the first kstrdup(). Bounding each value by nprim catches
neither, and neither would a count: the symbol table is keyed on the
permission name, so duplicates pass.

Track the values each permission table claims and require them to cover
exactly what its count declares, rejecting a count no value can reach.
Conforming policies are unaffected.

Cc: stable@vger.kernel.org
Fixes: 55fcf09b3fe4 ("selinux: add support for querying object classes and permissions from the running policy")
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 69777e885ae750d87d6c3465070bc4832e4d6011..d358200817bd9a990cbdbbdf3e040566b5cc4953 100644 (file)
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -1154,7 +1154,18 @@ int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
        return 0;
 }
 
-static int perm_read(struct policydb *p, struct symtab *s, struct policy_file *fp)
+/*
+ * Bitmap of the permission values a symtab has claimed.  Values are 1-based
+ * and bounded by SEL_VEC_MAX, the width of an access vector, so the whole set
+ * fits in a u32 and the callers reject an nprim past that width.
+ */
+static u32 perm_claimed_mask(u32 nprim)
+{
+       return nprim ? U32_MAX >> (SEL_VEC_MAX - nprim) : 0;
+}
+
+static int perm_read(struct policydb *p, struct symtab *s,
+                    struct policy_file *fp, u32 *claimed)
 {
        char *key = NULL;
        struct perm_datum *perdatum;
@@ -1178,6 +1189,10 @@ static int perm_read(struct policydb *p, struct symtab *s, struct policy_file *f
        /* indexes an nprim-sized array in security_get_permissions() */
        if (perdatum->value > s->nprim)
                goto bad;
+       /* two permissions cannot share one slot of that array */
+       if (*claimed & (1U << (perdatum->value - 1)))
+               goto bad;
+       *claimed |= 1U << (perdatum->value - 1);
 
        rc = str_read(&key, GFP_KERNEL, fp, len);
        if (rc)
@@ -1198,7 +1213,7 @@ static int common_read(struct policydb *p, struct symtab *s, struct policy_file
        char *key = NULL;
        struct common_datum *comdatum;
        __le32 buf[4];
-       u32 i, len, nel;
+       u32 i, len, nel, claimed = 0;
        int rc;
 
        comdatum = kzalloc_obj(*comdatum);
@@ -1225,17 +1240,28 @@ static int common_read(struct policydb *p, struct symtab *s, struct policy_file
        if (rc)
                goto bad;
        comdatum->permissions.nprim = le32_to_cpu(buf[2]);
+       /* no permission value can reach a slot past SEL_VEC_MAX */
+       rc = -EINVAL;
+       if (comdatum->permissions.nprim > SEL_VEC_MAX)
+               goto bad;
 
        rc = str_read(&key, GFP_KERNEL, fp, len);
        if (rc)
                goto bad;
 
        for (i = 0; i < nel; i++) {
-               rc = perm_read(p, &comdatum->permissions, fp);
+               rc = perm_read(p, &comdatum->permissions, fp, &claimed);
                if (rc)
                        goto bad;
        }
 
+       rc = -EINVAL;
+       if (claimed != perm_claimed_mask(comdatum->permissions.nprim)) {
+               pr_err("SELinux:  common %s does not define every permission it declares\n",
+                      key);
+               goto bad;
+       }
+
        hash_eval(&comdatum->permissions.table, "common_permissions", key);
 
        rc = symtab_insert(s, key, comdatum);
@@ -1369,7 +1395,7 @@ static int class_read(struct policydb *p, struct symtab *s, struct policy_file *
        char *key = NULL;
        struct class_datum *cladatum;
        __le32 buf[6];
-       u32 i, len, len2, ncons, nel, val;
+       u32 i, len, len2, ncons, nel, val, claimed = 0, inherited = 0;
        int rc;
 
        cladatum = kzalloc_obj(*cladatum);
@@ -1402,6 +1428,10 @@ static int class_read(struct policydb *p, struct symtab *s, struct policy_file *
        if (rc)
                goto bad;
        cladatum->permissions.nprim = le32_to_cpu(buf[3]);
+       /* no permission value can reach a slot past SEL_VEC_MAX */
+       rc = -EINVAL;
+       if (cladatum->permissions.nprim > SEL_VEC_MAX)
+               goto bad;
 
        ncons = le32_to_cpu(buf[5]);
 
@@ -1436,11 +1466,22 @@ static int class_read(struct policydb *p, struct symtab *s, struct policy_file *
                }
        }
        for (i = 0; i < nel; i++) {
-               rc = perm_read(p, &cladatum->permissions, fp);
+               rc = perm_read(p, &cladatum->permissions, fp, &claimed);
                if (rc)
                        goto bad;
        }
 
+       /* the class's own permissions must claim the slots the common leaves */
+       if (cladatum->comdatum)
+               inherited = cladatum->comdatum->permissions.nprim;
+       rc = -EINVAL;
+       if (claimed != (perm_claimed_mask(cladatum->permissions.nprim) &
+                       ~perm_claimed_mask(inherited))) {
+               pr_err("SELinux:  class %s does not define every permission it declares\n",
+                      key);
+               goto bad;
+       }
+
        hash_eval(&cladatum->permissions.table, "class_permissions", key);
 
        rc = read_cons_helper(p, &cladatum->constraints, ncons, 0, fp);