Re: [PATCH 2/3] t/helper/test-delta: segfault on OOB access

From: Jann Horn <jannh@google.com>
Date: Aug 29, 2018, 21:40 UTC
Message-ID: <CAG48ez2BL1fgP_FkUi=6EYAuA+ja3ujZFFqKkPQN50tRCEUJiQ@mail.gmail.com>
In-Reply-To: <20180829213433.GC29880@sigill.intra.peff.net>

On Wed, Aug 29, 2018 at 11:34 PM Jeff King <peff@peff.net> wrote:

Show 18 quoted lines

>
> On Wed, Aug 29, 2018 at 10:58:56PM +0200, Jann Horn wrote:
>
> > This ensures that any attempts to access memory directly after the input
> > buffer or delta buffer in a delta test will cause a segmentation fault.
> >
> > Inspired by vsftpd.
>
> Neat trick, but it seems funny to protect this one buffer in
> non-production code. Obviously you were interested in demonstrating the
> issue for your tests, but do we want to carry this all the time?
>
> If we want to detect this kind of thing in tests, we should probably be
> relying on tools like ASan, which would cover all mmaps.
>
> It would be nice if there was a low-cost way to detect this in
> production use, but it looks like this replaces mmap with
> read_in_full(), which I think is a non-starter for most uses.

I think even with ASAN, you'd still need read_in_full() or an mmap() wrapper that fiddles with the ASAN shadow, because mmap() always maps whole pages:

$ cat mmap-read-asan-blah.c
#include <sys/mman.h>
#include <stdlib.h>
int main(void) {
  volatile char *p = mmap(NULL, 1, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
  p[200] = 1;
}
$ gcc -o mmap-read-asan-blah mmap-read-asan-blah.c -fsanitize=address
$ ./mmap-read-asan-blah
$

But that aside, you do have a point about having some custom hack for a single patch.