flex_array: Change behaviour on zero size allocations
Subsystems (3)
PATCH v2 1/1 flex_array: Change behaviour on zero size allocations
flex_array_alloc allocates the basic struct flex_array regardless whether total_nr_elements or element_size is zero. Then flex_array_prealloc fails with -ENOSPC if total_nr_elements is zero or it hits a division by zero if element_size is zero.
This patch changes the behaviour on zero size allocations to the same as kmalloc, so zero size allocations are leagal now. We also change flex_array_prealloc to take the number of elements for which space should be allocated instead of the last (inclusive) element. Users and documentation are updated accordingly.
This fixes a regression on selinux policy loading.
--- Documentation/flexible-arrays.txt | 4 +- include/linux/flex_array.h | 2 +- lib/flex_array.c | 48 ++++++++++++++++++++++++++++-------- security/selinux/ss/policydb.c | 6 ++-- 4 files changed, 43 insertions(+), 17 deletions(-)
Documentation/flexible-arrays.txt +2 -2
@@ -66,10 +66,10 @@ trick is to ensure that any needed memory allocations are done before entering atomic context, using: int flex_array_prealloc(struct flex_array *array, unsigned int start, - unsigned int end, gfp_t flags); + unsigned int nr_elements, gfp_t flags); This function will ensure that memory for the elements indexed in the range -defined by start and end has been allocated. Thereafter, a +defined by start and nr_elements has been allocated. Thereafter, a flex_array_put() call on an element in that range is guaranteed not to block.
include/linux/flex_array.h +1 -1
@@ -61,7 +61,7 @@ struct flex_array {struct flex_array *flex_array_alloc (int element_size,unsigned int total,gfp_t flags);int flex_array_prealloc (struct flex_array *fa,unsigned int start, -unsigned int end,gfp_t flags); +unsigned int nr_elements,gfp_t flags);void flex_array_free (struct flex_array *fa);void flex_array_free_parts (struct flex_array *fa); int flex_array_put(struct flex_array *fa, unsigned int element_nr, void *src,
lib/flex_array.c +37 -11
120 lines not on this page · raw message
security/selinux/ss/policydb.c +3 -3
@@ -497,7 +497,7 @@ static int policydb_index(struct policydb *p) goto out; rc =flex_array_prealloc (p->type_val_to_struct_array,0 , - p->p_types.nprim -1 ,GFP_KERNEL | __GFP_ZERO); + p->p_types.nprim,GFP_KERNEL | __GFP_ZERO);if (rc) goto out;
@@ -514,7 +514,7 @@ static int policydb_index(struct policydb *p) goto out; rc =flex_array_prealloc (p->sym_val_to_name[i], -0 , p->symtab[i].nprim -1 , +0 , p->symtab[i].nprim,GFP_KERNEL | __GFP_ZERO);if (rc) goto out;
@@ -2286,7 +2286,7 @@ int policydb_read(struct policydb *p, void *fp) goto bad;/* preallocate so we don't have to worry about the put ever failing */ - rc =flex_array_prealloc (p->type_attr_map_array,0 , p->p_types.nprim -1 , + rc =flex_array_prealloc (p->type_attr_map_array,0 , p->p_types.nprim,GFP_KERNEL | __GFP_ZERO);if (rc) goto bad;
Signature · 1 line
1.7.0.4
Quoted 14 lines · Steffen Klassert
int flex_array_prealloc(struct
flex_array*fa, unsigned int start,lib/flex_array.c +5
-2- unsigned int end, gfp_t flags) + unsigned int nr_elements, gfp_t flags) { int start_part;int end_part;int part_nr; +unsigned int end;struct flex_array_part *part; -if (start >= fa->total_nr_elements || end >= fa->total_nr_elements) +if (!fa->total_nr_elements && !start) +return 0 ; +if (start >= fa->total_nr_elements)
lib/flex_array.c EXPORT_SYMBOL(flex_array_clear); · line 277
Quoted 10 lines
lib/flex_array.c +9
-0+return -ENOSPC ; +if (!nr_elements) +return 0 ; + + end = start + nr_elements -1 ; + +if (end >= fa->total_nr_elements)return -ENOSPC ; +if (!fa->element_size) +return 0 ;
The rest of this patch looks good. When you resend, you need to break this up in to at least two patches: one to change 'end' to 'nr_elements' (or something else) and the one to handle zero-sized elements and arrays.
I also think we need to remove the ability to do zero-sized elements. Unless there's some new code that I'm missing, I don't see any of the existing security policy code which would be able to do that.
-- Dave
Quoted 5 lines · Dave Hansen
The rest of this patch looks good. When you resend, you need to break this up in to at least two patches: one to change 'end' to '
nr_elements' (or something else) and the one to handle zero-sized elements and arrays.
Ok.
Quoted 5 lines
I also think we need to remove the ability to do zero-sized elements. Unless there's some new code that I'm missing, I don't see any of the existing security policy code which would be able to do that.
Well, from the selinux policy point of view we don't need the ability to handle zero-sized elements. But why you want to remove it? I don't see a reason why zero-sized elements should be handled different form zero number of elements. This could again produce a pitfall for potential new flex_array users.
Anyway, for the moment I'm just interested to load my selinux policy. So we could return -EINVAL is somebody tries to allocate with element size zero if you think it should be like that.
Steffen