Attachment 923018 Details for Bug 1119662 – 02-netlbl-catmap_ulong_fixes.patch

[patch] 02-netlbl-catmap_ulong_fixes.patch

02-netlbl-catmap_ulong_fixes.patch (text/plain), 16.71 KB, created by Paul Moore on 2014-07-31 18:26:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Paul Moore

Created: 2014-07-31 18:26:21 UTC

Size: 16.71 KB

patch

obsolete

>netlabel: fix the horribly broken catmap functions
>
>From: Paul Moore <pmoore@redhat.com>
>
>The NetLabel secattr catmap functions, and the SELinux import/export
>glue routines, were broken in many horrible ways.  This patch corrects
>the problems and creates a level of independence between the SELinux
>ebitmap structure and the NetLabel secattr catmap.
>
>One nice 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.
>
>Cc: stable@vger.kernel.org
>Reported-by: Christian Evans <frodox@zoho.com>
>Signed-off-by: Paul Moore <pmoore@redhat.com>
>---
> include/net/netlabel.h        |   26 +++++
> net/ipv4/cipso_ipv4.c         |   12 --
> net/netlabel/netlabel_kapi.c  |  216 ++++++++++++++++++++++++++++++++---------
> security/selinux/ss/ebitmap.c |  127 +++++++++---------------
> security/smack/smack_access.c |    5 -
> 5 files changed, 240 insertions(+), 146 deletions(-)
>
>diff --git a/include/net/netlabel.h b/include/net/netlabel.h
>index 1c40d65..bda7a12 100644
>--- a/include/net/netlabel.h
>+++ b/include/net/netlabel.h
>@@ -285,11 +285,11 @@ static inline void netlbl_secattr_catmap_free(
> {
> 	struct netlbl_lsm_secattr_catmap *iter;
> 
>-	do {
>+	while (catmap) {
> 		iter = catmap;
> 		catmap = catmap->next;
> 		kfree(iter);
>-	} while (catmap);
>+	}
> }
> 
> /**
>@@ -394,6 +394,9 @@ int netlbl_secattr_catmap_walk(struct netlbl_lsm_secattr_catmap *catmap,
> 			       u32 offset);
> int netlbl_secattr_catmap_walk_rng(struct netlbl_lsm_secattr_catmap *catmap,
> 				   u32 offset);
>+int netlbl_secattr_catmap_getlong(struct netlbl_lsm_secattr_catmap *catmap,
>+				  u32 *offset,
>+				  unsigned long *bitmap);
> int netlbl_secattr_catmap_setbit(struct netlbl_lsm_secattr_catmap **catmap,
> 				 u32 bit,
> 				 gfp_t flags);
>@@ -401,6 +404,10 @@ int netlbl_secattr_catmap_setrng(struct netlbl_lsm_secattr_catmap **catmap,
> 				 u32 start,
> 				 u32 end,
> 				 gfp_t flags);
>+int netlbl_secattr_catmap_setlong(struct netlbl_lsm_secattr_catmap **catmap,
>+				  u32 offset,
>+				  unsigned long bitmap,
>+				  gfp_t flags);
> 
> /*
>  * LSM protocol operations (NetLabel LSM/kernel API)
>@@ -504,6 +511,13 @@ static inline int netlbl_secattr_catmap_walk_rng(
> {
> 	return -ENOENT;
> }
>+static inline int netlbl_secattr_catmap_getlong(
>+				      struct netlbl_lsm_secattr_catmap *catmap,
>+				      u32 *offset,
>+				      unsigned long *bitmap)
>+{
>+	return 0;
>+}
> static inline int netlbl_secattr_catmap_setbit(
> 				      struct netlbl_lsm_secattr_catmap **catmap,
> 				      u32 bit,
>@@ -519,6 +533,14 @@ static inline int netlbl_secattr_catmap_setrng(
> {
> 	return 0;
> }
>+static int netlbl_secattr_catmap_setlong(
>+				      struct netlbl_lsm_secattr_catmap **catmap,
>+				      u32 offset,
>+				      unsigned long bitmap,
>+				      gfp_t flags)
>+{
>+	return 0;
>+}
> static inline int netlbl_enabled(void)
> {
> 	return 0;
>diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
>index dd433c9..8a0c7bd 100644
>--- a/net/ipv4/cipso_ipv4.c
>+++ b/net/ipv4/cipso_ipv4.c
>@@ -1335,10 +1335,6 @@ static int cipso_v4_parsetag_rbm(const struct cipso_v4_doi *doi_def,
> 	secattr->flags |= NETLBL_SECATTR_MLS_LVL;
> 
> 	if (tag_len > 4) {
>-		secattr->attr.mls.cat = netlbl_secattr_catmap_alloc(GFP_ATOMIC);
>-		if (secattr->attr.mls.cat == NULL)
>-			return -ENOMEM;
>-
> 		ret_val = cipso_v4_map_cat_rbm_ntoh(doi_def,
> 						    &tag[4],
> 						    tag_len - 4,
>@@ -1430,10 +1426,6 @@ static int cipso_v4_parsetag_enum(const struct cipso_v4_doi *doi_def,
> 	secattr->flags |= NETLBL_SECATTR_MLS_LVL;
> 
> 	if (tag_len > 4) {
>-		secattr->attr.mls.cat = netlbl_secattr_catmap_alloc(GFP_ATOMIC);
>-		if (secattr->attr.mls.cat == NULL)
>-			return -ENOMEM;
>-
> 		ret_val = cipso_v4_map_cat_enum_ntoh(doi_def,
> 						     &tag[4],
> 						     tag_len - 4,
>@@ -1524,10 +1516,6 @@ static int cipso_v4_parsetag_rng(const struct cipso_v4_doi *doi_def,
> 	secattr->flags |= NETLBL_SECATTR_MLS_LVL;
> 
> 	if (tag_len > 4) {
>-		secattr->attr.mls.cat = netlbl_secattr_catmap_alloc(GFP_ATOMIC);
>-		if (secattr->attr.mls.cat == NULL)
>-			return -ENOMEM;
>-
> 		ret_val = cipso_v4_map_cat_rng_ntoh(doi_def,
> 						    &tag[4],
> 						    tag_len - 4,
>diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
>index 84e810b..d9e1046 100644
>--- a/net/netlabel/netlabel_kapi.c
>+++ b/net/netlabel/netlabel_kapi.c
>@@ -405,6 +405,63 @@ out_entry:
>  * Security Attribute Functions
>  */
> 
>+#define _CM_F_NONE	0x00000000
>+#define _CM_F_ALLOC	0x00000001
>+
>+/**
>+ * _netlbl_secattr_catmap_getnode - Get a individual node from a catmap
>+ * @catmap: pointer to the category bitmap
>+ * @offset: the requested offset
>+ * @cm_flags: catmap flags, see _CM_F_*
>+ * @gfp_flags: memory allocation flags
>+ *
>+ * Description:
>+ * Iterate through the catmap looking for the node associated with @offset; if
>+ * the _CM_F_ALLOC flag is set in @cm_flags and there is no associated node,
>+ * one will be created and inserted into the catmap.  Returns a pointer to the
>+ * node on success, NULL on failure.
>+ *
>+ */
>+static struct netlbl_lsm_secattr_catmap *_netlbl_secattr_catmap_getnode(
>+				struct netlbl_lsm_secattr_catmap **catmap,
>+				u32 offset,
>+				unsigned int cm_flags,
>+				gfp_t gfp_flags)
>+{
>+	struct netlbl_lsm_secattr_catmap *iter = *catmap;
>+	struct netlbl_lsm_secattr_catmap *prev = NULL;
>+
>+	if (iter == NULL || offset < iter->startbit)
>+		goto secattr_catmap_getnode_alloc;
>+	while (iter && offset >= (iter->startbit + NETLBL_CATMAP_SIZE)) {
>+		prev = iter;
>+		iter = iter->next;
>+	}
>+	if (iter == NULL || offset < iter->startbit)
>+		goto secattr_catmap_getnode_alloc;
>+
>+	return iter;
>+
>+secattr_catmap_getnode_alloc:
>+	if (!(cm_flags & _CM_F_ALLOC))
>+		return NULL;
>+
>+	iter = netlbl_secattr_catmap_alloc(gfp_flags);
>+	if (iter == NULL)
>+		return NULL;
>+	iter->startbit = offset & ~(NETLBL_CATMAP_SIZE - 1);
>+
>+	if (prev == NULL) {
>+		iter->next = *catmap;
>+		*catmap = iter;
>+	} else {
>+		iter->next = prev->next;
>+		prev->next = iter;
>+	}
>+
>+	return iter;
>+}
>+
> /**
>  * netlbl_secattr_catmap_walk - Walk a LSM secattr catmap looking for a bit
>  * @catmap: the category bitmap
>@@ -521,6 +578,54 @@ int netlbl_secattr_catmap_walk_rng(struct netlbl_lsm_secattr_catmap *catmap,
> }
> 
> /**
>+ * netlbl_secattr_catmap_getlong - Export an unsigned long bitmap
>+ * @catmap: pointer to the category bitmap
>+ * @offset: pointer to the requested offset
>+ * @bitmap: the exported bitmap
>+ *
>+ * Description:
>+ * Export a bitmap with an offset greater than or equal to @offset and return
>+ * it in @bitmap.  The @offset must be aligned to an unsigned long and will be
>+ * updated on return if different from what was requested; if the catmap is
>+ * empty at the requested offset and beyond, the @offset is set to (u32)-1.
>+ * Returns zero on sucess, negative values on failure.
>+ *
>+ */
>+int netlbl_secattr_catmap_getlong(struct netlbl_lsm_secattr_catmap *catmap,
>+				  u32 *offset,
>+				  unsigned long *bitmap)
>+{
>+	struct netlbl_lsm_secattr_catmap *iter;
>+	u32 off = *offset;
>+	u32 idx;
>+
>+	/* only allow aligned offsets */
>+	if ((off & (BITS_PER_LONG - 1)) != 0)
>+		return -EINVAL;
>+
>+	if (off < catmap->startbit) {
>+		off = catmap->startbit;
>+		*offset = off;
>+	}
>+	iter = _netlbl_secattr_catmap_getnode(&catmap, off, _CM_F_NONE, 0);
>+	if (iter == NULL) {
>+		*offset = (u32)-1;
>+		return 0;
>+	}
>+
>+	if (off < iter->startbit) {
>+		off = iter->startbit;
>+		*offset = off;
>+	} else
>+		off -= iter->startbit;
>+
>+	idx = off / NETLBL_CATMAP_MAPSIZE;
>+	*bitmap = iter->bitmap[idx] >> (off % NETLBL_CATMAP_SIZE);
>+
>+	return 0;
>+}
>+
>+/**
>  * netlbl_secattr_catmap_setbit - Set a bit in a LSM secattr catmap
>  * @catmap: pointer to the category bitmap
>  * @bit: the bit to set
>@@ -535,32 +640,16 @@ int netlbl_secattr_catmap_setbit(struct netlbl_lsm_secattr_catmap **catmap,
> 				 u32 bit,
> 				 gfp_t flags)
> {
>-	struct netlbl_lsm_secattr_catmap *iter = *catmap;
>-	u32 node_bit;
>-	u32 node_idx;
>+	struct netlbl_lsm_secattr_catmap *iter;
>+	u32 idx;
> 
>-	while (iter->next != NULL &&
>-	       bit >= (iter->startbit + NETLBL_CATMAP_SIZE))
>-		iter = iter->next;
>-	if (bit < iter->startbit) {
>-		iter = netlbl_secattr_catmap_alloc(flags);
>-		if (iter == NULL)
>-			return -ENOMEM;
>-		iter->next = *catmap;
>-		iter->startbit = bit & ~(NETLBL_CATMAP_SIZE - 1);
>-		*catmap = iter;
>-	} else if (bit >= (iter->startbit + NETLBL_CATMAP_SIZE)) {
>-		iter->next = netlbl_secattr_catmap_alloc(flags);
>-		if (iter->next == NULL)
>-			return -ENOMEM;
>-		iter = iter->next;
>-		iter->startbit = bit & ~(NETLBL_CATMAP_SIZE - 1);
>-	}
>+	iter = _netlbl_secattr_catmap_getnode(catmap, bit, _CM_F_ALLOC, flags);
>+	if (iter == NULL)
>+		return -ENOMEM;
> 
>-	/* gcc always rounds to zero when doing integer division */
>-	node_idx = (bit - iter->startbit) / NETLBL_CATMAP_MAPSIZE;
>-	node_bit = bit - iter->startbit - (NETLBL_CATMAP_MAPSIZE * node_idx);
>-	iter->bitmap[node_idx] |= NETLBL_CATMAP_BIT << node_bit;
>+	bit -= iter->startbit;
>+	idx = bit / NETLBL_CATMAP_MAPSIZE;
>+	iter->bitmap[idx] |= NETLBL_CATMAP_BIT << (bit % NETLBL_CATMAP_MAPSIZE);
> 
> 	return 0;
> }
>@@ -582,34 +671,61 @@ int netlbl_secattr_catmap_setrng(struct netlbl_lsm_secattr_catmap **catmap,
> 				 u32 end,
> 				 gfp_t flags)
> {
>-	int ret_val = 0;
>-	struct netlbl_lsm_secattr_catmap *iter = *catmap;
>-	u32 iter_max_spot;
>-	u32 spot;
>-	u32 orig_spot = iter->startbit;
>-
>-	/* XXX - This could probably be made a bit faster by combining writes
>-	 * to the catmap instead of setting a single bit each time, but for
>-	 * right now skipping to the start of the range in the catmap should
>-	 * be a nice improvement over calling the individual setbit function
>-	 * repeatedly from a loop. */
>-
>-	while (iter->next != NULL &&
>-	       start >= (iter->startbit + NETLBL_CATMAP_SIZE))
>-		iter = iter->next;
>-	iter_max_spot = iter->startbit + NETLBL_CATMAP_SIZE;
>-
>-	for (spot = start; spot <= end && ret_val == 0; spot++) {
>-		if (spot >= iter_max_spot && iter->next != NULL) {
>-			iter = iter->next;
>-			iter_max_spot = iter->startbit + NETLBL_CATMAP_SIZE;
>-		}
>-		ret_val = netlbl_secattr_catmap_setbit(&iter, spot, flags);
>-		if (iter->startbit < orig_spot)
>-			*catmap = iter;
>+	int rc = 0;
>+	u32 spot = start;
>+
>+	while (rc == 0 && spot <= end) {
>+		if (((spot & (BITS_PER_LONG - 1)) != 0) &&
>+		    ((end - spot) > BITS_PER_LONG)) {
>+			rc = netlbl_secattr_catmap_setlong(catmap,
>+							   spot,
>+							   (unsigned long)-1,
>+							   flags);
>+			spot += BITS_PER_LONG;
>+		} else
>+			rc = netlbl_secattr_catmap_setbit(catmap,
>+							  spot++,
>+							  flags);
> 	}
> 
>-	return ret_val;
>+	return rc;
>+}
>+
>+/**
>+ * netlbl_secattr_catmap_setlong - Import an unsigned long bitmap
>+ * @catmap: pointer to the category bitmap
>+ * @offset: offset to the start of the imported bitmap
>+ * @bitmap: the bitmap to import
>+ * @flags: memory allocation flags
>+ *
>+ * Description:
>+ * Import the bitmap specified in @bitmap into @catmap, using the offset
>+ * in @offset.  The offset must be aligned to an unsigned long.  Returns zero
>+ * on success, negative values on failure.
>+ *
>+ */
>+int netlbl_secattr_catmap_setlong(struct netlbl_lsm_secattr_catmap **catmap,
>+				  u32 offset,
>+				  unsigned long bitmap,
>+				  gfp_t flags)
>+{
>+	struct netlbl_lsm_secattr_catmap *iter;
>+	u32 idx;
>+
>+	/* only allow aligned offsets */
>+	if ((offset & (BITS_PER_LONG - 1)) != 0)
>+		return -EINVAL;
>+
>+	iter = _netlbl_secattr_catmap_getnode(catmap,
>+					      offset, _CM_F_ALLOC, flags);
>+	if (iter == NULL)
>+		return -ENOMEM;
>+
>+	offset -= iter->startbit;
>+	idx = offset / NETLBL_CATMAP_MAPSIZE;
>+	iter->bitmap[idx] |= bitmap << (offset % NETLBL_CATMAP_MAPSIZE);
>+
>+	return 0;
> }
> 
> /*
>diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
>index 820313a..842deca 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 9ecf4f4..ea1bc50 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++) {

Actions: View | Diff

Attachments on bug 1119662: 918084 | 923017 | 923018 | 923019 | 923020