From: Dave Hansen <dave@linux.vnet.ibm.com>
To: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Eric Paris <eparis@redhat.com>,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org
Subject: Re: flex_array related problems on selinux policy loading
Date: Thu, 20 Jan 2011 07:28:50 -0800 [thread overview]
Message-ID: <1295537330.9039.583.camel@nimitz> (raw)
In-Reply-To: <20110120122659.GD4639@secunet.com>
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
next prev parent reply other threads:[~2011-01-20 15:30 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-20 12:26 flex_array related problems on selinux policy loading Steffen Klassert
2011-01-20 15:28 ` Dave Hansen [this message]
2011-01-21 7:20 ` Steffen Klassert
2011-01-21 15:57 ` Dave Hansen
2011-01-26 10:23 ` Steffen Klassert
2011-01-26 16:10 ` Dave Hansen
2011-01-27 12:15 ` Steffen Klassert
2011-01-31 8:08 ` Steffen Klassert
2011-01-26 13:04 ` Steffen Klassert
2011-01-26 16:15 ` Dave Hansen
2011-01-27 12:46 ` Steffen Klassert
2011-01-27 16:57 ` Dave Hansen
2011-01-31 8:00 ` Steffen Klassert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1295537330.9039.583.camel@nimitz \
--to=dave@linux.vnet.ibm.com \
--cc=eparis@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=steffen.klassert@secunet.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).