netlabel: fix the horribly broken catmap functions

The NetLabel secattr catmap functions, and the SELinux import/export
glue routines, were broken in many horrible ways and the SELinux glue
code fiddled with the NetLabel catmap structures in ways that we
probably shouldn't allow.  At some point this "worked", but that was
likely due to a bit of dumb luck and sub-par testing (both inflicted
by yours truly).  This patch corrects these problems by basically
gutting the code in favor of something less obtuse and restoring the
NetLabel abstractions in the SELinux catmap glue code.

Everything is working now, and if it decides to break itself in the
future this code will be much easier to debug than the code it
replaces.

One noteworthy side effect of the changes is that it is no longer
necessary to allocate a NetLabel catmap before calling one of the
NetLabel APIs to set a bit in the catmap.  NetLabel will automatically
allocate the catmap nodes when needed, resulting in less allocations
when the lowest bit is greater than 255 and less code in the LSMs.

Cc: stable@vger.kernel.org
Reported-by: Christian Evans <frodox@zoho.com>
Signed-off-by: Paul Moore <pmoore@redhat.com>
Tested-by: Casey Schaufler <casey@schaufler-ca.com>

2 files changed, 50 insertions, 82 deletions

diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
index 820313a04d49..842deca9484d 100644
--- a/security/selinux/ss/ebitmap.c
+++ b/security/selinux/ss/ebitmap.c
@@ -89,48 +89,33 @@ int ebitmap_netlbl_export(struct ebitmap *ebmap,
 			  struct netlbl_lsm_secattr_catmap **catmap)
 {
 	struct ebitmap_node *e_iter = ebmap->node;
-	struct netlbl_lsm_secattr_catmap *c_iter;
-	u32 cmap_idx, cmap_sft;
-	int i;
-
-	/* NetLabel's NETLBL_CATMAP_MAPTYPE is defined as an array of u64,
-	 * however, it is not always compatible with an array of unsigned long
-	 * in ebitmap_node.
-	 * In addition, you should pay attention the following implementation
-	 * assumes unsigned long has a width equal with or less than 64-bit.
-	 */
+	unsigned long e_map;
+	u32 offset;
+	unsigned int iter;
+	int rc;
 
 	if (e_iter == NULL) {
 		*catmap = NULL;
 		return 0;
 	}
 
-	c_iter = netlbl_secattr_catmap_alloc(GFP_ATOMIC);
-	if (c_iter == NULL)
-		return -ENOMEM;
-	*catmap = c_iter;
-	c_iter->startbit = e_iter->startbit & ~(NETLBL_CATMAP_SIZE - 1);
+	if (*catmap != NULL)
+		netlbl_secattr_catmap_free(*catmap);
+	*catmap = NULL;
 
 	while (e_iter) {
-		for (i = 0; i < EBITMAP_UNIT_NUMS; i++) {
-			unsigned int delta, e_startbit, c_endbit;
-
-			e_startbit = e_iter->startbit + i * EBITMAP_UNIT_SIZE;
-			c_endbit = c_iter->startbit + NETLBL_CATMAP_SIZE;
-			if (e_startbit >= c_endbit) {
-				c_iter->next
-				  = netlbl_secattr_catmap_alloc(GFP_ATOMIC);
-				if (c_iter->next == NULL)
+		offset = e_iter->startbit;
+		for (iter = 0; iter < EBITMAP_UNIT_NUMS; iter++) {
+			e_map = e_iter->maps[iter];
+			if (e_map != 0) {
+				rc = netlbl_secattr_catmap_setlong(catmap,
+								   offset,
+								   e_map,
+								   GFP_ATOMIC);
+				if (rc != 0)
 					goto netlbl_export_failure;
-				c_iter = c_iter->next;
-				c_iter->startbit
-				  = e_startbit & ~(NETLBL_CATMAP_SIZE - 1);
 			}
-			delta = e_startbit - c_iter->startbit;
-			cmap_idx = delta / NETLBL_CATMAP_MAPSIZE;
-			cmap_sft = delta % NETLBL_CATMAP_MAPSIZE;
-			c_iter->bitmap[cmap_idx]
-				|= e_iter->maps[i] << cmap_sft;
+			offset += EBITMAP_UNIT_SIZE;
 		}
 		e_iter = e_iter->next;
 	}
@@ -155,56 +140,42 @@ netlbl_export_failure:
 int ebitmap_netlbl_import(struct ebitmap *ebmap,
 			  struct netlbl_lsm_secattr_catmap *catmap)
 {
+	int rc;
 	struct ebitmap_node *e_iter = NULL;
-	struct ebitmap_node *emap_prev = NULL;
-	struct netlbl_lsm_secattr_catmap *c_iter = catmap;
-	u32 c_idx, c_pos, e_idx, e_sft;
-
-	/* NetLabel's NETLBL_CATMAP_MAPTYPE is defined as an array of u64,
-	 * however, it is not always compatible with an array of unsigned long
-	 * in ebitmap_node.
-	 * In addition, you should pay attention the following implementation
-	 * assumes unsigned long has a width equal with or less than 64-bit.
-	 */
-
-	do {
-		for (c_idx = 0; c_idx < NETLBL_CATMAP_MAPCNT; c_idx++) {
-			unsigned int delta;
-			u64 map = c_iter->bitmap[c_idx];
-
-			if (!map)
-				continue;
+	struct ebitmap_node *e_prev = NULL;
+	u32 offset = 0, idx;
+	unsigned long bitmap;
+
+	for (;;) {
+		rc = netlbl_secattr_catmap_getlong(catmap, &offset, &bitmap);
+		if (rc < 0)
+			goto netlbl_import_failure;
+		if (offset == (u32)-1)
+			return 0;
 
-			c_pos = c_iter->startbit
-				+ c_idx * NETLBL_CATMAP_MAPSIZE;
-			if (!e_iter
-			    || c_pos >= e_iter->startbit + EBITMAP_SIZE) {
-				e_iter = kzalloc(sizeof(*e_iter), GFP_ATOMIC);
-				if (!e_iter)
-					goto netlbl_import_failure;
-				e_iter->startbit
-					= c_pos - (c_pos % EBITMAP_SIZE);
-				if (emap_prev == NULL)
-					ebmap->node = e_iter;
-				else
-					emap_prev->next = e_iter;
-				emap_prev = e_iter;
-			}
-			delta = c_pos - e_iter->startbit;
-			e_idx = delta / EBITMAP_UNIT_SIZE;
-			e_sft = delta % EBITMAP_UNIT_SIZE;
-			while (map) {
-				e_iter->maps[e_idx++] |= map & (-1UL);
-				map = EBITMAP_SHIFT_UNIT_SIZE(map);
-			}
+		if (e_iter == NULL ||
+		    offset >= e_iter->startbit + EBITMAP_SIZE) {
+			e_prev = e_iter;
+			e_iter = kzalloc(sizeof(*e_iter), GFP_ATOMIC);
+			if (e_iter == NULL)
+				goto netlbl_import_failure;
+			e_iter->startbit = offset & ~(EBITMAP_SIZE - 1);
+			if (e_prev == NULL)
+				ebmap->node = e_iter;
+			else
+				e_prev->next = e_iter;
+			ebmap->highbit = e_iter->startbit + EBITMAP_SIZE;
 		}
-		c_iter = c_iter->next;
-	} while (c_iter);
-	if (e_iter != NULL)
-		ebmap->highbit = e_iter->startbit + EBITMAP_SIZE;
-	else
-		ebitmap_destroy(ebmap);
 
+		/* offset will always be aligned to an unsigned long */
+		idx = EBITMAP_NODE_INDEX(e_iter, offset);
+		e_iter->maps[idx] = bitmap;
+
+		/* next */
+		offset += EBITMAP_UNIT_SIZE;
+	}
+
+	/* NOTE: we should never reach this return */
 	return 0;
 
 netlbl_import_failure:
diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
index 9ecf4f4b67a1..ea1bc5055792 100644
--- a/security/smack/smack_access.c
+++ b/security/smack/smack_access.c
@@ -435,10 +435,7 @@ int smk_netlbl_mls(int level, char *catset, struct netlbl_lsm_secattr *sap,
 
 	sap->flags |= NETLBL_SECATTR_MLS_CAT;
 	sap->attr.mls.lvl = level;
-	sap->attr.mls.cat = netlbl_secattr_catmap_alloc(GFP_ATOMIC);
-	if (!sap->attr.mls.cat)
-		return -ENOMEM;
-	sap->attr.mls.cat->startbit = 0;
+	sap->attr.mls.cat = NULL;
 
 	for (cat = 1, cp = catset, byte = 0; byte < len; cp++, byte++)
 		for (m = 0x80; m != 0; m >>= 1, cat++) {