Re: [Patch] [BZ 13761] Fix another unbound alloca

[Jeff Law <law at redhat dot com>]

On 07/02/2012 04:02 PM, Roland McGrath wrote:
> > It seems to me that freeing dataset is just wrong if it's allocated via
> > mempool_alloc and things in mempool_alloc are GC'd.
>
> Correct.
>
> > ISTM the safe thing to do would be something like:
> >
> >        if (he == NULL)
> >           dataset = (struct dataset *) mempool_alloc (db, total + n, 1);
> >        else if (! __libc_use_alloca (alloca_used + total + n))
> >           dataset = (struct dataset *) malloc (...);
>
> Agreed.  Note that in the existing code ALLOCA_USED is a Boolean, and it is
> used to mean "cannot be placed in the cache".  So the bookkeeping would
> change somewhat to support the malloc case (which is alloca_used=true in
> the current sense of that variable, but also has to be freed).
>
> > While it might be safe/appropriate to use mempools in the case where the
> > needed space is large and transient, it's hard to prove.
>
> The purpose of the mempool stuff is to use the shared memory space that's
> available to clients as a cache without interacting with nscd.  So it's
> never appropriate for something that is not going to be looked up in that
> cache later.
Sorry this has taken so long to get back to.

To refresh your memory, DATASET can be allocated via alloca within 
cache_addgr and the size is potentially unbound.  The original patch had 
the problem that it could allocate DATASET via mempool_alloc, then try 
to free it later.  As we've discussed, that's clearly wrong.

This new version of the patch implements the changes noted above.  Those 
changes actually make the patch simpler to follow.

Attachment: patch
Description: Text document