GNU bug report logs - #19563
grep -F: fix a heap buffer (read) overrun

Previous Next

Full log

View this message in rfc822 format

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jim Meyering <jim <at> meyering.net>, 19563 <at> debbugs.gnu.org
Cc: Yuliy Pisetsky <ypisetsky <at> fb.com>, Nima Aghdaii <naghdaii <at> fb.com>
Subject: bug#19563: grep -F: fix a heap buffer (read) overrun
Date: Sat, 10 Jan 2015 16:02:46 -0800

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.

> +#ifdef HAVE_ASAN
> +# define ASAN_POISON_MEMORY_REGION(addr, size) \
> +  __asan_poison_memory_region ((addr), (size))
> +# define ASAN_UNPOISON_MEMORY_REGION(addr, size) \
> +  __asan_unpoison_memory_region ((addr), (size))
> +#else
> +# define ASAN_POISON_MEMORY_REGION(addr, size) \
> +  (ignore_value (addr), ignore_value (size))
> +# define ASAN_UNPOISON_MEMORY_REGION(addr, size) \
> +  (ignore_value (addr), ignore_value (size))
> +#endif

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

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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.