Date: Thu, 20 Dec 2012 13:42:54 +0100 From: Jilles Tjoelker <jilles@stack.nl> To: Dimitry Andric <dim@FreeBSD.org> Cc: Eitan Adler <lists@eitanadler.com>, Gabor Kovesdan <gabor@freebsd.org>, FreeBSD Hackers <freebsd-hackers@freebsd.org> Subject: Re: use after free in grep? Message-ID: <20121220124254.GA99616@stack.nl> In-Reply-To: <50D3023B.8090407@FreeBSD.org> References: <CAF6rxg=Ni2Kcgdw2XrSVtU1f9eHaFt1-oBTNv8pm8An52x13nQ@mail.gmail.com> <50D3023B.8090407@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Dec 20, 2012 at 01:19:07PM +0100, Dimitry Andric wrote:
> On 2012-12-20 08:13, Eitan Adler wrote:
> > in xrealloc_impl
> > 338 new_ptr = realloc(ptr, new_size);
> > 339 if (new_ptr != NULL)
> > 340 {
> > 341 hash_table_del(xmalloc_table, ptr);
> > ^^^ isn't this a use-after-free of ptr?
> Yes, realloc does not guarantee the realloc'd space will be at the same
> address, so it may free ptr at its discretion.
Even if you somehow know realloc() is not going to move the block, it is
still wrong to use any pointer not derived from its return value to
access the block. Comparing the old and the new pointers (normally or
with memcmp()) does not help; it has an indeterminate result.
See http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_260.htm
> Also, there is a memory leak if realloc() returns NULL. This is a
> very usual mistake when using realloc(). :-)
No, this would be correct if a successful realloc() call did not make
the old pointer indeterminate. The hash table remains unchanged if
realloc() fails.
> Probably, the code should do the hash_table_del() before the realloc(),
> but I am not sure if hash_table_del() will already free ptr.
Yes, and add it back if realloc() fails.
A smarter internal interface to the hash table would avoid freeing and
reallocating hash table entries here (which might fail).
--
Jilles Tjoelker
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121220124254.GA99616>