If the call to realloc requires transmem_list's memory to be moved, then
the prior pointer is free'd. Most of the time this doesn't cause a
noticeable problem because the memory is still mapped and the
dereference in transmem_list->next shortly after the realloc call works
fine (this is still wrong obviously).
However, if transmem_list was allocated by mmap (because it was large),
when the original pointer is free'd as a result of the realloc call, the
old memory gets ummapped. With the memory now unmapped, the
transmem_list->next dereference of the old pointer fails with a segfault.
I don't have a good testcase -- the one provided to me hasn't tripped in
about 12 hours of running or in shorter runs with differing values of
M_MMAP_THRESHOLD and may contain confidential information. Hopefully
the analysis above is clear enough to show this code is clearly broken.
While that fix looks like it can't be wrong, looking at the surrounding
code it looks like there's a better fix. The old value is always available
in the new copy, i.e. NEWMEM->next. But about 30 lines below, we have:
On 06/21/2012 02:47 PM, Roland McGrath wrote:
> While that fix looks like it can't be wrong, looking at the surrounding
> code it looks like there's a better fix. The old value is always available
> in the new copy, i.e. NEWMEM->next.
> But about 30 lines below, we have:
> newmem->next = transmem_list;
> transmem_list = newmem;
> It popping the element off and then putting it back on is entirely redundant.
> The addition is necessary in the malloc (not realloc) case. But it could
> just be done inside that case.
Something like this perhaps?
On 06/21/2012 04:42 PM, Roland McGrath wrote:
> That looks right to me.
> Your first message mentioned a BZ#, but it's not in your log entry.
Fixed in the final commit. NEWS file updated and copyright year updated