This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PING][PATCH][BZ #15073] Fix race in free.
- From: Maxim Kuvyrkov <maxim at kugelworks dot com>
- To: David Miller <davem at davemloft dot net>, Carlos O'Donell <carlos at redhat dot com>, Andreas Schwab <schwab at linux-m68k dot org>
- Cc: GLIBC Devel <libc-alpha at sourceware dot org>, OndÅej BÃlka <neleai at seznam dot cz>
- Date: Sat, 28 Dec 2013 12:05:45 +1300
- Subject: Re: [PING][PATCH][BZ #15073] Fix race in free.
- Authentication-results: sourceware.org; auth=none
- References: <20131210194349 dot GA19644 at domone dot podge> <20131218102757 dot GC6559 at domone dot podge> <C593C68D-FF0C-4C46-93E2-4A3ABA33E84B at kugelworks dot com> <20131221205921 dot GA3549 at domone dot podge> <30A9C720-86CB-48CB-B931-00A83542DC84 at kugelworks dot com> <20131223163345 dot GA5291 at domone>
Dave,
Carlos,
Andreas,
Consider backporting fix [*] for BZ #15073 (https://sourceware.org/bugzilla/show_bug.cgi?id=15073) to your respective release branches.
The bug is present since 2.12, conditioned on ATOMIC_FASTBINS, which became unconditional default in 2.15. Therefore backporting to 2.12 and 2.14 is optional, but 2.15 onwards is very desirable.
The patch applies cleanly to 2.15 onwards.
The essence of the fix is to avoid "chunksize(old)" memory reference since OLD can be deallocated if !HAVE_LOCK.
[*] 362b47fe09ca9a928d444c7e2f7992f7f61bfc3e
Let me know if you have any questions or if the fix does not appear to be obvious/safe.
Thank you,
--
Maxim Kuvyrkov
www.kugelworks.com
On 24/12/2013, at 5:33 am, OndÅej BÃlka <neleai@seznam.cz> wrote:
> On Mon, Dec 23, 2013 at 10:01:47AM +1300, Maxim Kuvyrkov wrote:
>> On 22/12/2013, at 10:07 am, OndÅej BÃlka <neleai@seznam.cz> wrote:
>>
>>> On Sun, Dec 22, 2013 at 09:08:42AM +1300, Maxim Kuvyrkov wrote:
>>>> On 18/12/2013, at 11:27 pm, OndÅej BÃlka <neleai@seznam.cz> wrote:
>>>>>>
>> ...
>>>>>> Solution is simply drop that check.
>>>>
>>>> Or fix the check. If we have_lock for the memory state there is no way a competing thread can snatch and deallocate the fastbin chunk that we are adding to.
>>>>
>>> As most malloc checks decrease readibility I favor deletion. Adding a
>>> check for lock solves problem but as _int_free always called from
>>> __libc_free with have_lock = 0 it would render check mostly useless.
>>
>> I do not quite agree with this. Sanity checks like the one in question tend to expose arcane bugs, and, given that it is very lightweight (no function calls, just several macros doing bit math), I prefer to keep it. It is true that primary use from __libc_free will not benefit from the sanity check, but there are quite a few uses with have_lock==1.
>>
>> I do not think that we should dwell for too much longer on whether remove or keep the check, as our time is best spent fixing other bugs in glibc. If you really feel strongly about removing the check, how about rock-paper-scissors on IRC :-)?
>>
> If you think that this is useful you can keep that and proceed with your
> patch.