From 4b8feff251da3d7058b5779e21b33a85c686b974 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 1 Aug 2014 11:17:17 -0400 Subject: 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 Signed-off-by: Paul Moore Tested-by: Casey Schaufler --- security/selinux/ss/ebitmap.c | 127 ++++++++++++++++-------------------------- 1 file changed, 49 insertions(+), 78 deletions(-) (limited to 'security/selinux') 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: -- cgit v1.1 From 4fbe63d1c773cceef3fe1f6ed0c9c268f4f24760 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 1 Aug 2014 11:17:37 -0400 Subject: netlabel: shorter names for the NetLabel catmap funcs/structs Historically the NetLabel LSM secattr catmap functions and data structures have had very long names which makes a mess of the NetLabel code and anyone who uses NetLabel. This patch renames the catmap functions and structures from "*_secattr_catmap_*" to just "*_catmap_*" which improves things greatly. There are no substantial code or logic changes in this patch. Signed-off-by: Paul Moore Tested-by: Casey Schaufler --- security/selinux/ss/ebitmap.c | 18 +++++++++--------- security/selinux/ss/ebitmap.h | 8 ++++---- 2 files changed, 13 insertions(+), 13 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c index 842deca..afe6a26 100644 --- a/security/selinux/ss/ebitmap.c +++ b/security/selinux/ss/ebitmap.c @@ -86,7 +86,7 @@ int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src) * */ int ebitmap_netlbl_export(struct ebitmap *ebmap, - struct netlbl_lsm_secattr_catmap **catmap) + struct netlbl_lsm_catmap **catmap) { struct ebitmap_node *e_iter = ebmap->node; unsigned long e_map; @@ -100,7 +100,7 @@ int ebitmap_netlbl_export(struct ebitmap *ebmap, } if (*catmap != NULL) - netlbl_secattr_catmap_free(*catmap); + netlbl_catmap_free(*catmap); *catmap = NULL; while (e_iter) { @@ -108,10 +108,10 @@ int ebitmap_netlbl_export(struct ebitmap *ebmap, 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); + rc = netlbl_catmap_setlong(catmap, + offset, + e_map, + GFP_ATOMIC); if (rc != 0) goto netlbl_export_failure; } @@ -123,7 +123,7 @@ int ebitmap_netlbl_export(struct ebitmap *ebmap, return 0; netlbl_export_failure: - netlbl_secattr_catmap_free(*catmap); + netlbl_catmap_free(*catmap); return -ENOMEM; } @@ -138,7 +138,7 @@ netlbl_export_failure: * */ int ebitmap_netlbl_import(struct ebitmap *ebmap, - struct netlbl_lsm_secattr_catmap *catmap) + struct netlbl_lsm_catmap *catmap) { int rc; struct ebitmap_node *e_iter = NULL; @@ -147,7 +147,7 @@ int ebitmap_netlbl_import(struct ebitmap *ebmap, unsigned long bitmap; for (;;) { - rc = netlbl_secattr_catmap_getlong(catmap, &offset, &bitmap); + rc = netlbl_catmap_getlong(catmap, &offset, &bitmap); if (rc < 0) goto netlbl_import_failure; if (offset == (u32)-1) diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h index 712c8a7..9637b8c 100644 --- a/security/selinux/ss/ebitmap.h +++ b/security/selinux/ss/ebitmap.h @@ -132,17 +132,17 @@ int ebitmap_write(struct ebitmap *e, void *fp); #ifdef CONFIG_NETLABEL int ebitmap_netlbl_export(struct ebitmap *ebmap, - struct netlbl_lsm_secattr_catmap **catmap); + struct netlbl_lsm_catmap **catmap); int ebitmap_netlbl_import(struct ebitmap *ebmap, - struct netlbl_lsm_secattr_catmap *catmap); + struct netlbl_lsm_catmap *catmap); #else static inline int ebitmap_netlbl_export(struct ebitmap *ebmap, - struct netlbl_lsm_secattr_catmap **catmap) + struct netlbl_lsm_catmap **catmap) { return -ENOMEM; } static inline int ebitmap_netlbl_import(struct ebitmap *ebmap, - struct netlbl_lsm_secattr_catmap *catmap) + struct netlbl_lsm_catmap *catmap) { return -ENOMEM; } -- cgit v1.1