| Summary: | Pointer used after free'd | ||
|---|---|---|---|
| Product: | glibc | Reporter: | law |
| Component: | libc | Assignee: | law |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | aj, bugdal, drepper.fsp, fweimer |
| Priority: | P2 | Flags: | fweimer:
security+
|
| Version: | 2.15 | ||
| Target Milestone: | --- | ||
| Host: | Target: | ||
| Build: | Last reconfirmed: | ||
| Attachments: | Potential fix | ||
On Thu, 21 Jun 2012, law at redhat dot com wrote:
> 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.
Does use of M_PERTURB (to cause free to overwrite freed memory - I don't
actually know if it works for realloc as well) make it easier to reproduce
the problem?
M_PERTURB does work for realloc (which just calls free internally). I can even verify that it puts my perturb bytes into the free'd memory. All fine and good. Unfortunately after installing my perturb byte, something else inside malloc decides to put the value 0 into the first 4 bytes of the free'd memory, overwriting my nice invalid pointer perturb byte ;( That happens to correspond to transmem_list->next. When we then try to use the value, it just looks like a null pointer, which the code assumes is the end of the list. So M_PERTURB isn't particularly helpful in generating a testcase for this bug. Comment on attachment 6463 [details]
Potential fix
Jeff, the patch looks fine to me. I suggest to send it to libc-alpha to review to see whether Carlos likes to include it or not for 2.16. Thanks!
A worse manifestation of this bug is in multithreaded programs: the just-freed memory could be obtained by another thread and thus the code would not crash, but would instead read memory now belonging to another thread and use it as a pointer. This could have serious consequences, possibly much worse than a crash... Fixed now, thanks! commit 006dd86111c44572dbd3b26e9c63dd0f834d7762 Author: Jeff Law <law@redhat.com> Date: Thu Jun 21 17:15:38 2012 -0600 [BZ #14277] * intl/dcigettext.c (_nl_find_msg): Avoid use after potential free. Simplify list management for _LIBC case. |
Created attachment 6463 [details] Potential fix dcigettext.c has a very clear use-after-free problem which can result in a user process getting a segfault or other error. newmem = (transmem_block_t *) realloc (transmem_list, freemem_size); # ifdef _LIBC if (newmem != NULL) transmem_list = transmem_list->next; else { struct transmem_list *old = transmem_list; transmem_list = transmem_list->next; free (old); } # endif 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. Attached is a potential fix for this problem.