A while back we talked about having the capability of switching off checks
like the one for kmalloc(0) for stable kernel releases. This is a first stab
at such functionality. It adds #ifdef CONFIG_STABLE for now. Maybe we can
come up with some better way to handle it later. There should alsol be some
way to set CONFIG_STABLE from the Makefile.
CONFIG_STABLE switches off
- kmalloc(0) check in both slab allocators
- SLUB banner
- Makes SLUB tolerate object corruption like SLAB (not sure if we really want
to go down this route. See patch)
--
[email protected] wrote:
> A while back we talked about having the capability of switching off checks
> like the one for kmalloc(0) for stable kernel releases. This is a first stab
> at such functionality. It adds #ifdef CONFIG_STABLE for now. Maybe we can
> come up with some better way to handle it later. There should alsol be some
> way to set CONFIG_STABLE from the Makefile.
>
> CONFIG_STABLE switches off
>
> - kmalloc(0) check in both slab allocators
> - SLUB banner
> - Makes SLUB tolerate object corruption like SLAB (not sure if we really want
> to go down this route. See patch)
>
Perhaps I missed it, but what's the rationale for complaining about
0-sized allocations? They seem like a perfectly reasonable thing to me;
they turn up at the boundary conditions of many algorithms, and avoiding
them just cruds up the callsites to make them go through hoops to avoid
allocation.
Why not just do a 1 byte allocation instead, and be done with it? Any
non-constant-sized allocation will potentially have to deal with this
case, so it seems to me we could just put the fix in common code (and
use an inline wrapper to avoid it when dealing with constant non-zero
sized allocations).
J
On Fri, 1 Jun 2007, Jeremy Fitzhardinge wrote:
> Perhaps I missed it, but what's the rationale for complaining about
> 0-sized allocations? They seem like a perfectly reasonable thing to me;
> they turn up at the boundary conditions of many algorithms, and avoiding
> them just cruds up the callsites to make them go through hoops to avoid
> allocation.
Hmmm... We got there because SLUB initially return NULL for kmalloc(0).
Rationale: The user did not request any memory so we wont give him
any.
That (to my surprise) caused some strange behavior of code and so we then
decided to keep SLAB behavior and return the smallest available object
size and put a warning in there. At some later point we plan to switch
to returning NULL for kmalloc(0).
> Why not just do a 1 byte allocation instead, and be done with it? Any
> non-constant-sized allocation will potentially have to deal with this
> case, so it seems to me we could just put the fix in common code (and
> use an inline wrapper to avoid it when dealing with constant non-zero
> sized allocations).
The smallest allocation that SLUB can do is 8 bytes (SLAB 32). We are
giving the user the smallest object that we can allocate. We could just
remove the warning and would have the behavior that you want.
But now allocating code gets memory although none was requested. The
object can be modified without us being able to check.
By returning NULL any use of the returned pointer would BUG.
An allocation of zero bytes usually indicates that the code is not dealing
with a special case. Later code may operate on the allocated object. I
think its clearer and cleaner if code would deal with that special case
explicitly. We have seen a series of code pieces that do uncomfortably
looking operations on structures with no objects.
Christoph Lameter wrote:
> Hmmm... We got there because SLUB initially return NULL for kmalloc(0).
> Rationale: The user did not request any memory so we wont give him
> any.
>
> That (to my surprise) caused some strange behavior of code and so we then
> decided to keep SLAB behavior and return the smallest available object
> size and put a warning in there. At some later point we plan to switch
> to returning NULL for kmalloc(0).
>
Unfortunately, returning NULL is indistinguishable from ENOMEM, so the
caller would have to check to see how much it asked for before deciding
to really fail, which doesn't help things much.
Or does it (should it) return ERRPTR(-ENOMEM)? Bit of a major API
change if not.
>> Why not just do a 1 byte allocation instead, and be done with it? Any
>> non-constant-sized allocation will potentially have to deal with this
>> case, so it seems to me we could just put the fix in common code (and
>> use an inline wrapper to avoid it when dealing with constant non-zero
>> sized allocations).
>>
>
> The smallest allocation that SLUB can do is 8 bytes (SLAB 32). We are
> giving the user the smallest object that we can allocate. We could just
> remove the warning and would have the behavior that you want.
>
I think that's reasonable. 0-sized allocations seem to be relatively
rare anyway, so its not like we're going to be filling the heap with
these things.
I'm getting a couple of these warnings out of my code, but I've been
hesitating about fixing them because I can't see it resulting in any
improvement, either locally or globally - it would just be to shut the
warning up.
> But now allocating code gets memory although none was requested. The
> object can be modified without us being able to check.
>
Yes, that's a problem, but maybe heap debugging would help (ie, if
enabled make sure the "zero-sized allocation" pattern is undisturbed).
Or return a pointer to a specific non-NULL unmapped address, though that
would need special handling on the free side. Also, callers may get
upset if they get non-unique non-NULL addresses back from allocation.
> By returning NULL any use of the returned pointer would BUG.
>
Well, actually it would stomp usermode memory, but we generally assume
there's nothing mapped at 0. Or there are no bugs.
> An allocation of zero bytes usually indicates that the code is not dealing
> with a special case. Later code may operate on the allocated object. I
> think its clearer and cleaner if code would deal with that special case
> explicitly. We have seen a series of code pieces that do uncomfortably
> looking operations on structures with no objects.
>
I disagree. There are plenty of boundary conditions where 0 is not
really a special case, and making it a special case just complicates
things. I think at least some of the patches posted to silence this
warning have been generally negative for code quality. If we were
seeing lots of zero-sized allocations then that might indicate something
is amiss, but it seems to me that there's just a scattered handful.
I agree that it's always a useful debugging aid to make sure that
allocated regions are not over-run, but 0-sized allocations are not
special in this regard.
J
On Fri, 1 Jun 2007, Jeremy Fitzhardinge wrote:
> > An allocation of zero bytes usually indicates that the code is not dealing
> > with a special case. Later code may operate on the allocated object. I
> > think its clearer and cleaner if code would deal with that special case
> > explicitly. We have seen a series of code pieces that do uncomfortably
> > looking operations on structures with no objects.
> >
>
> I disagree. There are plenty of boundary conditions where 0 is not
> really a special case, and making it a special case just complicates
> things. I think at least some of the patches posted to silence this
> warning have been generally negative for code quality. If we were
> seeing lots of zero-sized allocations then that might indicate something
> is amiss, but it seems to me that there's just a scattered handful.
>
> I agree that it's always a useful debugging aid to make sure that
> allocated regions are not over-run, but 0-sized allocations are not
> special in this regard.
Still insisting on it even after the discovery of the cpuset kmalloc(0) issue?
Christoph Lameter wrote:
>> I disagree. There are plenty of boundary conditions where 0 is not
>> really a special case, and making it a special case just complicates
>> things. I think at least some of the patches posted to silence this
>> warning have been generally negative for code quality. If we were
>> seeing lots of zero-sized allocations then that might indicate something
>> is amiss, but it seems to me that there's just a scattered handful.
>>
>> I agree that it's always a useful debugging aid to make sure that
>> allocated regions are not over-run, but 0-sized allocations are not
>> special in this regard.
>>
>
> Still insisting on it even after the discovery of the cpuset kmalloc(0) issue?
>
Sure. That was a normal buffer-overrun bug. There's nothing special
about 0-sized allocations.
J
On Fri, 2007-06-01 at 11:58 -0700, Jeremy Fitzhardinge wrote:
> Christoph Lameter wrote:
> > Hmmm... We got there because SLUB initially return NULL for kmalloc(0).
> > Rationale: The user did not request any memory so we wont give him
> > any.
> >
> > That (to my surprise) caused some strange behavior of code and so we then
> > decided to keep SLAB behavior and return the smallest available object
> > size and put a warning in there. At some later point we plan to switch
> > to returning NULL for kmalloc(0).
> >
>
> Unfortunately, returning NULL is indistinguishable from ENOMEM, so the
> caller would have to check to see how much it asked for before deciding
> to really fail, which doesn't help things much.
>
> Or does it (should it) return ERRPTR(-ENOMEM)? Bit of a major API
> change if not.
I'm on Christoph's side here. I don't think it makes sense for any code
to ask to allocate zero bytes of memory and expect valid memory to be
returned.
Would a compromise be to return a pointer to some known invalid region?
This way the kmalloc(0) call would appear successful to the caller, but
any access to the memory would result in an exception.
Just my 2 cents,
Shaggy
--
David Kleikamp
IBM Linux Technology Center
Dave Kleikamp wrote:
> I'm on Christoph's side here. I don't think it makes sense for any code
> to ask to allocate zero bytes of memory and expect valid memory to be
> returned.
>
Yes, everyone agrees on that. If you do kmalloc(0), its never OK to
dereference the result. The question is whether kmalloc(0) should complain.
> Would a compromise be to return a pointer to some known invalid region?
> This way the kmalloc(0) call would appear successful to the caller, but
> any access to the memory would result in an exception.
>
Yes, that's what Christoph has posted. I'm slightly concerned about
kmalloc() returning the same non-NULL address multiple times, but it
seems sound otherwise.
J
On Sat, 2007-06-02 at 09:28 -0700, Jeremy Fitzhardinge wrote:
> Dave Kleikamp wrote:
> > I'm on Christoph's side here. I don't think it makes sense for any code
> > to ask to allocate zero bytes of memory and expect valid memory to be
> > returned.
> >
>
> Yes, everyone agrees on that. If you do kmalloc(0), its never OK to
> dereference the result. The question is whether kmalloc(0) should complain.
Yeah, I see that you aren't necessarily asking for valid memory, just
something that appears valid. I'm still of the mind that if code is
asking for a zero-length allocation, it's raising a flag that it's not
taking some corner case into account. But I think I'm just
regurgitating what Christoph is arguing.
> > Would a compromise be to return a pointer to some known invalid region?
> > This way the kmalloc(0) call would appear successful to the caller, but
> > any access to the memory would result in an exception.
> >
>
> Yes, that's what Christoph has posted.
Oh. I went back and re-read the thread and it looks like you proposed
this already. I don't see where Christoph did, or agreed, but maybe I
missed something.
> I'm slightly concerned about
> kmalloc() returning the same non-NULL address multiple times, but it
> seems sound otherwise.
If the caller is asking for 0 bytes, it shouldn't be doing anything with
the returned address except checking for a NULL return. But then, it's
hard to predict everything that calling code might be doing, such as
allocating buffers and creating a hash based on their addresses. Of
course, if there's code that would have a problem with it, I think it's
a further argument that it would be better off avoiding the calling
kmalloc(0) in the first place.
Shaggy
--
David Kleikamp
IBM Linux Technology Center
Dave Kleikamp wrote:
> I'm on Christoph's side here. I don't think it makes sense for any code
> to ask to allocate zero bytes of memory and expect valid memory to be
> returned.
>
> Would a compromise be to return a pointer to some known invalid region?
> This way the kmalloc(0) call would appear successful to the caller, but
> any access to the memory would result in an exception.
I would think returning 1 as the address would work here, it's not NULL
but any access to that page should still oops..
--
Robert Hancock Saskatoon, SK, Canada
To email, remove "nospam" from [email protected]
Home Page: http://www.roberthancock.com/