[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#19563: grep -F: fix a heap buffer (read) overrun
From: |
Jim Meyering |
Subject: |
bug#19563: grep -F: fix a heap buffer (read) overrun |
Date: |
Sat, 10 Jan 2015 16:42:28 -0800 |
On Sat, Jan 10, 2015 at 4:02 PM, Paul Eggert <address@hidden> wrote:
> Jim Meyering wrote:
>>
>> +#if defined __clang__
>> +# if __has_feature(address_sanitizer)
>> +# define HAVE_ASAN 1
>> +# endif
>> +#elif defined __GNUC__ \
>> + && (((__GNUC__ == 4) && (__GNUC_MINOR__ >= 8)) || (__GNUC__ >= 5)) \
>> + && __SANITIZE_ADDRESS__
>> +# define HAVE_ASAN 1
>> +#endif
>
>
> How about the following instead?
>
> #ifndef __has_feature
> # define __has_feature(a) false
> #endif
>
> #if defined __SANITIZE_ADDRESS__ || __has_feature (address_sanitizer)
> # define HAVE_ASAN 1
> #else
> # define HAVE_ASAN 0
> #endif
>
> This is what Emacs uses (its symbol is ADDRESS_SANITIZER instead of
> HAVE_ASAN, for what that's worth).
>
>> + ASAN_POISON_MEMORY_REGION (buflim + sizeof(uword),
>> + bufalloc - (buflim - buffer) -
>> sizeof(uword));
>>
>
> The two 'sizeof's need spaces afterwards.
Fixed.
> I don't see the value of having macros here. How about the following
> instead?
>
> #ifndef HAVE_ASAN
> static void
> __asan_unpoison_memory_region (void const volatile *addr, size_t size)
> {
> }
>
> static void
> __asan_unpoison_memory_region (void const volatile *addr, size_t size)
> {
> }
> #endif
I agree. Adjusted via s/unpoison/poison/ in the first.
In addition, I have added _GL_UNUSED so as not to run afoul of grep's
use of -Werror=unused-function. Also, I have adapted not to
declare when using the static wrappers.
and to use #if, not #ifndef, since now HAVE_ASAN is always defined.
> And then have the callers invoke '__asan_poison_memory_region' instead of
> 'ASAN_POISON_MEMORY_REGION'. This way, there should be no need to pull in
> the ignore-value machinery, it's two less macros to worry about, and there's
> better type checking when address sanitization is not in use.
Thanks for all the good suggestions. Here's an updated version:
0001-tests-add-support-for-ASAN-memory-poisoning.patch
Description: Binary data