|
 |
a7d3f6 |
commit 362b47fe09ca9a928d444c7e2f7992f7f61bfc3e
|
|
|
a7d3f6 |
Author: Maxim Kuvyrkov <maxim@kugelworks.com>
|
|
|
a7d3f6 |
Date: Tue Dec 24 09:44:50 2013 +1300
|
|
|
a7d3f6 |
|
|
|
a7d3f6 |
Fix race in free() of fastbin chunk: BZ #15073
|
|
|
a7d3f6 |
|
|
|
a7d3f6 |
Perform sanity check only if we have_lock. Due to lockless nature of fastbins
|
|
|
a7d3f6 |
we need to be careful derefencing pointers to fastbin entries (chunksize(old)
|
|
|
a7d3f6 |
in this case) in multithreaded environments.
|
|
|
a7d3f6 |
|
|
|
a7d3f6 |
The fix is to add have_lock to the if-condition checks. The rest of the patch
|
|
|
a7d3f6 |
only makes code more readable.
|
|
|
a7d3f6 |
|
|
|
a7d3f6 |
* malloc/malloc.c (_int_free): Perform sanity check only if we
|
|
|
a7d3f6 |
have_lock.
|
|
|
a7d3f6 |
|
|
|
a7d3f6 |
Index: b/malloc/malloc.c
|
|
|
a7d3f6 |
===================================================================
|
|
|
a7d3f6 |
--- a/malloc/malloc.c
|
|
|
a7d3f6 |
+++ b/malloc/malloc.c
|
|
|
a7d3f6 |
@@ -3909,25 +3909,29 @@ _int_free(mstate av, mchunkptr p, int ha
|
|
|
a7d3f6 |
unsigned int idx = fastbin_index(size);
|
|
|
a7d3f6 |
fb = &fastbin (av, idx);
|
|
|
a7d3f6 |
|
|
|
a7d3f6 |
- mchunkptr fd;
|
|
|
a7d3f6 |
- mchunkptr old = *fb;
|
|
|
a7d3f6 |
+ /* Atomically link P to its fastbin: P->FD = *FB; *FB = P; */
|
|
|
a7d3f6 |
+ mchunkptr old = *fb, old2;
|
|
|
a7d3f6 |
unsigned int old_idx = ~0u;
|
|
|
a7d3f6 |
do
|
|
|
a7d3f6 |
{
|
|
|
a7d3f6 |
- /* Another simple check: make sure the top of the bin is not the
|
|
|
a7d3f6 |
- record we are going to add (i.e., double free). */
|
|
|
a7d3f6 |
+ /* Check that the top of the bin is not the record we are going to add
|
|
|
a7d3f6 |
+ (i.e., double free). */
|
|
|
a7d3f6 |
if (__builtin_expect (old == p, 0))
|
|
|
a7d3f6 |
{
|
|
|
a7d3f6 |
errstr = "double free or corruption (fasttop)";
|
|
|
a7d3f6 |
goto errout;
|
|
|
a7d3f6 |
}
|
|
|
a7d3f6 |
- if (old != NULL)
|
|
|
a7d3f6 |
+ /* Check that size of fastbin chunk at the top is the same as
|
|
|
a7d3f6 |
+ size of the chunk that we are adding. We can dereference OLD
|
|
|
a7d3f6 |
+ only if we have the lock, otherwise it might have already been
|
|
|
a7d3f6 |
+ deallocated. See use of OLD_IDX below for the actual check. */
|
|
|
a7d3f6 |
+ if (have_lock && old != NULL)
|
|
|
a7d3f6 |
old_idx = fastbin_index(chunksize(old));
|
|
|
a7d3f6 |
- p->fd = fd = old;
|
|
|
a7d3f6 |
+ p->fd = old2 = old;
|
|
|
a7d3f6 |
}
|
|
|
a7d3f6 |
- while ((old = catomic_compare_and_exchange_val_rel (fb, p, fd)) != fd);
|
|
|
a7d3f6 |
+ while ((old = catomic_compare_and_exchange_val_rel (fb, p, old2)) != old2);
|
|
|
a7d3f6 |
|
|
|
a7d3f6 |
- if (fd != NULL && __builtin_expect (old_idx != idx, 0))
|
|
|
a7d3f6 |
+ if (have_lock && old != NULL && __builtin_expect (old_idx != idx, 0))
|
|
|
a7d3f6 |
{
|
|
|
a7d3f6 |
errstr = "invalid fastbin entry (free)";
|
|
|
a7d3f6 |
goto errout;
|