Fix bug in policydb_write when attempting to correct policy for older versions.

The current implementation fails as it doesn't remove scope declarations further in
the modular policy.

This fixes the problem by removing the offending entries in the hash table and
the scope table.

Steps to reproduce:

Try to build the following module then make a module from an older release:
module test 1.0.0;

require {
  type default_t;
}
attribute_role new_atrole;
checkmodule -M -m -c 12 -o test.mod test.te
semodule_package -o test.pp -m test.mod
semodule_package:  Error while reading policy module from test.mod

Failure occurs when the current module gets written out with the scope declaration intact.
This is due to policydb.c:3913 doing a hashtab search on a scope key that is not
in the symbol table.

Signed-off-by: Matthew Ife <[email protected]>

Author: Matthew Ife

libsepol/src/hashtab.c

@@ -223,18 +223,19 @@ int hashtab_map(hashtab_t h,
 			      hashtab_datum_t d, void *args), void *args)
 {
 	unsigned int i, ret;
-	hashtab_ptr_t cur;
+	hashtab_ptr_t cur, next;
 
 	if (!h)
 		return SEPOL_OK;
 
 	for (i = 0; i < h->size; i++) {
 		cur = h->htable[i];
-		while (cur != NULL) {
+		while (curi != NULL) {
+			next = cur->next;
 			ret = apply(cur->key, cur->datum, args);
 			if (ret)
 				return ret;
-			cur = cur->next;
+			cur = next;
 		}
 	}
 	return SEPOL_OK;

libsepol/src/write.c

@@ -2143,30 +2143,67 @@ static int scope_write(hashtab_key_t key, hashtab_datum_t datum, void *ptr)
 	return rc;
 }
 
-static int type_attr_uncount(hashtab_key_t key __attribute__ ((unused)),
-			     hashtab_datum_t datum, void *args)
+static void role_write_destroy(hashtab_key_t key __attribute__ ((unused)),
+            hashtab_datum_t datum,
+            void *p __attribute__ ((unused)))
 {
-	type_datum_t *typdatum = datum;
-	uint32_t *p_nel = args;
+    role_datum_destroy((role_datum_t *) datum);
+    free(datum);
+}
 
-	if (typdatum->flavor == TYPE_ATTRIB) {
-		/* uncount attribute from total number of types */
-		(*p_nel)--;
-	}
-	return 0;
+static void type_write_destroy(hashtab_key_t key __attribute__ ((unused)),
+            hashtab_datum_t datum,
+            void *p __attribute__ ((unused)))
+{
+    type_datum_destroy((type_datum_t *) datum);
+    free(datum);
 }
 
-static int role_attr_uncount(hashtab_key_t key __attribute__ ((unused)),
-			     hashtab_datum_t datum, void *args)
+static void scope_write_destroy(hashtab_key_t key __attribute__ ((unused)),
+        hashtab_datum_t datum,
+        void *p __attribute__ ((unused)))
 {
-	role_datum_t *role = datum;
-	uint32_t *p_nel = args;
+    scope_datum_t *cur = (scope_datum_t *) datum;
+    if (cur != NULL) {
+        free(cur->decl_ids);
+    }
+    free(cur);
+}
 
-	if (role->flavor == ROLE_ATTRIB) {
-		/* uncount attribute from total number of roles */
-		(*p_nel)--;
-	}
-	return 0;
+static void type_attr_filter(hashtab_key_t key,
+                 hashtab_datum_t datum, void *args)
+{
+    type_datum_t *typdatum = datum;
+    scope_datum_t *scope = NULL;
+    policydb_t *p = (policydb_t *)args;
+    hashtab_t typetbl = p->symtab[SYM_TYPES].table;
+    hashtab_t scopetbl = p->scope[SYM_TYPES].table;
+
+    if (typdatum->flavor == TYPE_ATTRIB) {
+        /* Remove the entry from the hash table and scope table */
+        hashtab_remove(typetbl, key, type_write_destroy, typdatum);
+        scope = (scope_datum_t *)hashtab_search(scopetbl, key);
+        if (scope) 
+            hashtab_remove(scopetbl, key, scope_write_destroy, scope);
+    }
+}
+
+static void role_attr_filter(hashtab_key_t key,
+                 hashtab_datum_t datum, void *args)
+{
+    role_datum_t *role = datum;
+    scope_datum_t *scope = NULL;
+    policydb_t *p = (policydb_t *)args;
+    hashtab_t roletbl = p->symtab[SYM_ROLES].table;
+    hashtab_t scopetbl = p->scope[SYM_ROLES].table;
+
+    if (role->flavor == ROLE_ATTRIB) {
+        /* Remove the entry from the hash table and scope table */
+        hashtab_remove(roletbl, key, role_write_destroy, role);
+        scope = (scope_datum_t *)hashtab_search(scopetbl, key);
+        if (scope) 
+            hashtab_remove(scopetbl, key, scope_write_destroy, scope);
+    }
 }
 
 /*
@@ -2300,29 +2337,36 @@ int policydb_write(policydb_t * p, struct policy_file *fp)
 		buf[0] = cpu_to_le32(p->symtab[i].nprim);
 		buf[1] = p->symtab[i].table->nel;
 
-		/*
-		 * A special case when writing type/attribute symbol table.
-		 * The kernel policy version less than 24 does not support
-		 * to load entries of attribute, so we have to re-calculate
-		 * the actual number of types except for attributes.
-		 */
-		if (i == SYM_TYPES &&
-		    p->policyvers < POLICYDB_VERSION_BOUNDARY &&
-		    p->policy_type == POLICY_KERN) {
-			hashtab_map(p->symtab[i].table, type_attr_uncount, &buf[1]);
-		}
-
-		/*
-		 * Another special case when writing role/attribute symbol
-		 * table, role attributes are redundant for policy.X, or
-		 * when the pp's version is not big enough. So deduct
-		 * their numbers from p_roles.table->nel.
-		 */
-		if ((i == SYM_ROLES) &&
-		    ((p->policy_type == POLICY_KERN) ||
-		     (p->policy_type != POLICY_KERN &&
-		      p->policyvers < MOD_POLICYDB_VERSION_ROLEATTRIB)))
-			(void)hashtab_map(p->symtab[i].table, role_attr_uncount, &buf[1]);
+        /*
+         * A special case when writing type/attribute symbol table.
+         * The kernel policy version less than 24 does not support
+         * to load entries of attribute, so we filter the entries
+         * from the table.
+         */
+        if (i == SYM_TYPES &&
+            p->policyvers < POLICYDB_VERSION_BOUNDARY &&
+            p->policy_type == POLICY_KERN) {
+            (void)hashtab_map(p->symtab[i].table, type_attr_filter, p);
+            if (buf[1] != p->symtab[i].table->nel)
+                WARN(fp->handle, "Discarding type attribute rules");
+            buf[1] = p->symtab[i].table->nel;
+        }
+
+        /*
+         * Another special case when writing role/attribute symbol
+         * table, role attributes are redundant for policy.X, or
+         * when the pp's version is not big enough. So filter the entries
+         * from the table.
+         */
+        if ((i == SYM_ROLES) &&
+            ((p->policy_type == POLICY_KERN) ||
+             (p->policy_type != POLICY_KERN &&
+              p->policyvers < MOD_POLICYDB_VERSION_ROLEATTRIB))) {
+            (void)hashtab_map(p->symtab[i].table, role_attr_filter, p);
+			if (buf[1] != p->symtab[i].table->nel)
+				WARN(fp->handle, "Discarding role attribute rules");
+            buf[1] = p->symtab[i].table->nel;
+        }
 
 		buf[1] = cpu_to_le32(buf[1]);
 		items = put_entry(buf, sizeof(uint32_t), 2, fp);