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: <46606C71.9010008@goop.org>
Date:	Fri, 01 Jun 2007 11:58:57 -0700
From:	Jeremy Fitzhardinge <jeremy@...p.org>
To:	Christoph Lameter <clameter@....com>
CC:	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	akpm@...ux-foundation.org
Subject: Re: [RFC 0/4] CONFIG_STABLE to switch off development checks

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
-
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ