flex_array: Change behaviour on zero size allocations

I think this still has some of the issues of the earlier patch. The zero-size check needs to be moved after the ->total_nr_elements check. Otherwise, this won't produce any errors:

	fa = flex_array_alloc(0, 100, GFP_KERNEL);
	flex_array_put(fa, 1001, ptr, GFP_KERNEL);

Quoted 4 lines

lib/flex_array.c +0 -0

@@ -284,6 +297,8 @@ void *flex_array_get(struct flex_array *fa, unsigned int element_nr)
        int part_nr = fa_element_to_part_nr(fa, element_nr);
        struct flex_array_part *part;
 

lib/flex_array.c void *flex_array_get(struct flex_array *fa, unsigned int element_nr) · line 304

Quoted 5 lines

lib/flex_array.c +2 -0

+       if (!fa->total_nr_elements || !fa->element_size)
+               return NULL;
        if (element_nr >= fa->total_nr_elements)
                return NULL;
        if (elements_fit_in_base(fa))

Do you really need to check fa->total_nr_elements both for zero and against element_nr? Seems a but superfluous to me.

Both objections are correct, I'll send an updated patch.

rc = flex_array_prealloc(p->type_val_to_struct_array, 0,

                         p->p_types.nprim - 1, GFP_KERNEL | __GFP_ZERO);
if (rc)
        goto out;

If p->p_types.nprim is zero, we allocare with total_nr_elements equal to zerro and then we try to prealloc with p->p_types.nprim - 1. flex_array_prealloc interprets this as an unsigned int and fails, because this is bigger than total_nr_elements, which is correct I think.

Thoughts?

Quoted 5 lines · Steffen Klassert

rc = flex_array_prealloc(p->type_val_to_struct_array, 0,

                         p->p_types.nprim - 1, GFP_KERNEL | __GFP_ZERO);
if (rc)
        goto out;

Quoted 7 lines

If p->p_types.nprim is zero, we allocare with total_nr_elements equal to zerro and then we try to prealloc with p->p_types.nprim - 1. flex_array_prealloc interprets this as an unsigned int and fails, because this is bigger than total_nr_elements, which is correct I think.

Thoughts?

The most we ever hold in a flex_array is ~2 million entries. So we have plenty of room to use a normal int if you want.

On the other hand, there's only one user of flex_array_prealloc(), and making the "end" argument inclusive doesn't seem to be what that user wants. We might want to either make flex_array_prealloc() take start and length, or instead make "end" be exclusive of the "end" index.

I thought that flex_array_prealloc would say, effectively: "all put()'s would work up until 'end'". But, looking at it now, that's probably not how people will use it.

I'm fine with any solution. It's obviously broken for SELinux to be passing -1 even if the library supported it. I guess I don't really have strong feelings on how to fix it.

1) make end exclusive
2) change 'end' to 'len'
3) just make selinux not prealloc() when the #elements == 0

All seem perfectly reasonable to me, but I'd probably do them in that order.