Re: alloca vs malloc

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Mon, 19 May 2014, Ondrej Bilka wrote:

> > One straightforward set of changes in this area would be to replace
> > the alloca+extend_alloca usage in getaddrinfo code with straight
> > malloc+free.  I doubt if the performance improvement due to using
> > alloca is in any way noticeable in those functions given that the
> > major costs there would be file or network I/O.
> > 
> I will replace it everywhere. It does not improve performance for simple
> reason that it is called after function returned insufficient space then
> even if first buffer was 4096 bytes and its speed was cycle per byte you
> already spent 4096 cycles.

There are various cases where replacing alloca with malloc is not safe 
(but use of alloca isn't safe either unless the size allocated is 
bounded):

* If the function is AS-safe, or is meant to be AS-safe or users would 
reasonably expect it to be safe in contexts where it is not safe to call 
malloc.  It has been well-established that there is no consensus for 
making malloc AS-safe.

* That also applies in some cases even if the AS-safety is only 
conditional.  E.g., there have been arguments for snprintf being AS-safe 
and there may be more consensus on that than for malloc; right now it's 
only safe in certain circumstances when it does allocation with alloca, 
but we shouldn't regress its safety in those cases.

* If the function's interface does not provide a way for it to return an 
error indication for memory allocation failures at all, or people 
otherwise reasonably expect it never to fail for that reason, then bounded 
alloca / VLAs can be used, but malloc cannot.

So, if replacing alloca or VLAs with malloc then you need a justification 
of why it's already expected for the function not to be AS-safe and 
possibly to fail when out of memory.  This justification needs giving 
separately for each instance of alloca being replaced (each of which 
should have a separately submitted patch, as well as a separate Bugzilla 
bug for any existing unbounded stack allocation).

By way of example, consider bug 16962, an unbounded VLA bug I've just 
submitted to illustrate this point.  The nan function has no way of 
returning error status, so replacing the VLA with malloc would not be 
correct, and it can also reasonably be expected to be AS-safe (in fact, 
it's documented as such in the glibc manual at present).  A fix needs 
refactoring to avoid copying, rather than use of malloc.

(I don't know about practical exploitability or CVE-worthiness of that 
bug; I just presume that any unbounded stack allocation should be 
considered bad, although those where the space allocated is proportional 
to the space already used by function arguments passed to the glibc 
function are maybe less critical.)

-- 
Joseph S. Myers
joseph@codesourcery.com