[PATCH] selinux: Fix bugs in policy loading code

From: Stephen Smalley <sds@epoch.ncsc.mil>

This patch fixes a couple of bugs in the SELinux policy loading code.  The
first bug was reported by Magosanyi Arpad; kernel panic upon feeding the
kernel a policy with an empty avtab due to cleanup code trying to free the
avtab twice.  The other bugs were reported by Frank Mayer; failure to
properly validate certain values read from the policy.

diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index a1eab290d5a6d7d3e7e55860bdb5bd4f7f60dc78..778e51192bcaf7c3c511eb9b20716bdf2e124743 100644 (file)
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -98,7 +98,7 @@ void avtab_destroy(struct avtab *h)
        int i;
        struct avtab_node *cur, *temp;
 
-       if (!h)
+       if (!h || !h->htable)
                return;
 
        for (i = 0; i < AVTAB_SIZE; i++) {
@@ -111,6 +111,7 @@ void avtab_destroy(struct avtab *h)
                h->htable[i] = NULL;
        }
        vfree(h->htable);
+       h->htable = NULL;
 }
 
 

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 153a5b26247a31a329cb004c850319974cab40d7..2ebcef14af8952e728f66f299ab9da6bd9e22818 100644 (file)
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -124,6 +124,8 @@ static int common_index(void *key, void *datum, void *datap)
 
        comdatum = datum;
        p = datap;
+       if (!comdatum->value || comdatum->value > p->p_commons.nprim)
+               return -EINVAL;
        p->p_common_val_to_name[comdatum->value - 1] = key;
        return 0;
 }
@@ -135,6 +137,8 @@ static int class_index(void *key, void *datum, void *datap)
 
        cladatum = datum;
        p = datap;
+       if (!cladatum->value || cladatum->value > p->p_classes.nprim)
+               return -EINVAL;
        p->p_class_val_to_name[cladatum->value - 1] = key;
        p->class_val_to_struct[cladatum->value - 1] = cladatum;
        return 0;
@@ -147,6 +151,8 @@ static int role_index(void *key, void *datum, void *datap)
 
        role = datum;
        p = datap;
+       if (!role->value || role->value > p->p_roles.nprim)
+               return -EINVAL;
        p->p_role_val_to_name[role->value - 1] = key;
        p->role_val_to_struct[role->value - 1] = role;
        return 0;
@@ -160,8 +166,11 @@ static int type_index(void *key, void *datum, void *datap)
        typdatum = datum;
        p = datap;
 
-       if (typdatum->primary)
+       if (typdatum->primary) {
+               if (!typdatum->value || typdatum->value > p->p_types.nprim)
+                       return -EINVAL;
                p->p_type_val_to_name[typdatum->value - 1] = key;
+       }
 
        return 0;
 }
@@ -173,6 +182,8 @@ static int user_index(void *key, void *datum, void *datap)
 
        usrdatum = datum;
        p = datap;
+       if (!usrdatum->value || usrdatum->value > p->p_users.nprim)
+               return -EINVAL;
        p->p_user_val_to_name[usrdatum->value - 1] = key;
        p->user_val_to_struct[usrdatum->value - 1] = usrdatum;
        return 0;
@@ -502,13 +513,19 @@ int policydb_context_isvalid(struct policydb *p, struct context *c)
        struct role_datum *role;
        struct user_datum *usrdatum;
 
-       /*
-        * Role must be authorized for the type.
-        */
        if (!c->role || c->role > p->p_roles.nprim)
                return 0;
 
+       if (!c->user || c->user > p->p_users.nprim)
+               return 0;
+
+       if (!c->type || c->type > p->p_types.nprim)
+               return 0;
+
        if (c->role != OBJECT_R_VAL) {
+               /*
+                * Role must be authorized for the type.
+                */
                role = p->role_val_to_struct[c->role - 1];
                if (!ebitmap_get_bit(&role->types,
                                     c->type - 1))
@@ -518,8 +535,6 @@ int policydb_context_isvalid(struct policydb *p, struct context *c)
                /*
                 * User must be authorized for the role.
                 */
-               if (!c->user || c->user > p->p_users.nprim)
-                       return 0;
                usrdatum = p->user_val_to_struct[c->user - 1];
                if (!usrdatum)
                        return 0;