diff options
| author | Maxim Kuvyrkov <maxim@kugelworks.com> | 2013-12-24 09:44:50 +1300 |
|---|---|---|
| committer | Maxim Kuvyrkov <maxim@kugelworks.com> | 2014-01-05 15:00:40 +1300 |
| commit | c972bcc9ebdb5c2601b6b34001c7450e7a0b5ea3 (patch) | |
| tree | 86e77f0304ff216a80b86e453c76e6a3c60aae94 | |
| parent | 02eff8c4f82241c0843d47cb58c4355eb4f5d9e9 (diff) | |
| download | glibc-c972bcc9ebdb5c2601b6b34001c7450e7a0b5ea3.tar.gz glibc-c972bcc9ebdb5c2601b6b34001c7450e7a0b5ea3.tar.xz glibc-c972bcc9ebdb5c2601b6b34001c7450e7a0b5ea3.zip | |
Fix race in free() of fastbin chunk: BZ #15073
Perform sanity check only if we have_lock. Due to lockless nature of fastbins we need to be careful derefencing pointers to fastbin entries (chunksize(old) in this case) in multithreaded environments. The fix is to add have_lock to the if-condition checks. The rest of the patch only makes code more readable. * malloc/malloc.c (_int_free): Perform sanity check only if we have_lock. Conflicts: ChangeLog NEWS
| -rw-r--r-- | ChangeLog | 7 | ||||
| -rw-r--r-- | NEWS | 2 | ||||
| -rw-r--r-- | malloc/malloc.c | 20 |
3 files changed, 20 insertions, 9 deletions
diff --git a/ChangeLog b/ChangeLog index 3de21fd127..d823b1100c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2014-01-04 Maxim Kuvyrkov <maxim@kugelworks.com> + Ondřej Bílka <neleai@seznam.cz> + + [BZ #15073] + * malloc/malloc.c (_int_free): Perform sanity check only if we + have_lock. + 2012-11-20 Thomas Schwinge <thomas@codesourcery.com> * sysdeps/sh/dl-machine.h (ELF_MACHINE_RUNTIME_FIXUP_PARAMS): New diff --git a/NEWS b/NEWS index 86c6dfcbba..2c5fb07f7b 100644 --- a/NEWS +++ b/NEWS @@ -9,7 +9,7 @@ Version 2.16.1 * The following bugs are resolved with this release: - 6530, 14195, 14459, 14476, 14562, 14621, 14648, 14756, 14831 + 6530, 14195, 14459, 14476, 14562, 14621, 14648, 14756, 14831, 15073 Version 2.16 diff --git a/malloc/malloc.c b/malloc/malloc.c index 28039b4720..0ee5676c2e 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -3868,25 +3868,29 @@ _int_free(mstate av, mchunkptr p, int have_lock) unsigned int idx = fastbin_index(size); fb = &fastbin (av, idx); - mchunkptr fd; - mchunkptr old = *fb; + /* Atomically link P to its fastbin: P->FD = *FB; *FB = P; */ + mchunkptr old = *fb, old2; unsigned int old_idx = ~0u; do { - /* Another simple check: make sure the top of the bin is not the - record we are going to add (i.e., double free). */ + /* Check that the top of the bin is not the record we are going to add + (i.e., double free). */ if (__builtin_expect (old == p, 0)) { errstr = "double free or corruption (fasttop)"; goto errout; } - if (old != NULL) + /* Check that size of fastbin chunk at the top is the same as + size of the chunk that we are adding. We can dereference OLD + only if we have the lock, otherwise it might have already been + deallocated. See use of OLD_IDX below for the actual check. */ + if (have_lock && old != NULL) old_idx = fastbin_index(chunksize(old)); - p->fd = fd = old; + p->fd = old2 = old; } - while ((old = catomic_compare_and_exchange_val_rel (fb, p, fd)) != fd); + while ((old = catomic_compare_and_exchange_val_rel (fb, p, old2)) != old2); - if (fd != NULL && __builtin_expect (old_idx != idx, 0)) + if (have_lock && old != NULL && __builtin_expect (old_idx != idx, 0)) { errstr = "invalid fastbin entry (free)"; goto errout; |