linux-kernel - Re: flex_array related problems on selinux policy loading

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1295537330.9039.583.camel@nimitz>
Date:	Thu, 20 Jan 2011 07:28:50 -0800
From:	Dave Hansen <dave@...ux.vnet.ibm.com>
To:	Steffen Klassert <steffen.klassert@...unet.com>
Cc:	Eric Paris <eparis@...hat.com>, linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org
Subject: Re: 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/