[lkml]   [2011]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: flex_array related problems on selinux policy loading
On Thu, 2011-01-20 at 13:26 +0100, Steffen Klassert wrote:
> 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, we return ZERO_SIZE_PTR in this case. All flex_array handlers
> test for ZERO_SIZE_PTR and return an appropriate value to the caller.

Some of this seems pretty sane, at least to make it more of a drop-in
for kmalloc().

> @@ -187,6 +195,9 @@ int flex_array_put(struct flex_array *fa, unsigned int element_nr, void *src,
> struct flex_array_part *part;
> void *dst;
> + if (unlikely(ZERO_OR_NULL_PTR(fa)))
> + return 0;

I think it's OK to add these for the array alloc and free cases. But,
it's really dangerous to do it for put. It has the potential to
silently throw away data and then be really confusing to debug when you
can't get it back later.

This can only make sense if you have a zero-byte element. However, this
would also just return if you happened to try and insert data in to a
zero-length array. That's a bug we need to catch. Note that
kmem_cache_create() doesn't let you create caches for zero-byte objects.

> @@ -215,6 +226,9 @@ int flex_array_clear(struct flex_array *fa, unsigned int element_nr)
> struct flex_array_part *part;
> void *dst;
> + if (unlikely(ZERO_OR_NULL_PTR(fa)))
> + return 0;

I tend to think about the flex_array itself as being more like a
kmem_cache than anything else. So, all of the operations on the array
itself, like shrinking and growing are probably OK.

Can you also pull the unlikely()s?

-- Dave

 \ /
  Last update: 2011-01-20 16:33    [W:0.081 / U:19.604 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site