From: Collin Funk Date: Wed, 12 Nov 2025 20:06:47 GMT Subject: Re: [PATCH 3/9] Makefile: turn on NO_MMAP when building with ASan Message-ID: <87qzu32ey0.fsf@gmail.com> In-Reply-To: <20251112103158.GA983233@coredump.intra.peff.net> Jeff King writes: > On Wed, Nov 12, 2025 at 12:17:24AM -0800, Collin Funk wrote: > >> I see that an interceptor was added in 2023 [1]. Maybe your compiler is >> older than that? > > No, I'm using gcc 15.2.0 (from Debian unstable). > > But I'm not sure if the linked code does anything useful for us. > > One, it's not clear to me if it is even kicking in or not. It only does > anything if the region is "sanitizer managed", according to the details > at https://reviews.llvm.org/D154659. I'm not sure what that means > exactly, because I'm fuzzy on how the shadow map works. > > But even when it does do something, it seems to round up to the nearest > page size. But we really want to know if we go even one byte over the > requested length, because if we touch the 1235th byte of a 1234-byte > buffer (which is going to be a NUL because of mmap rounding up the > pages), then there's probably another test case somewhere where we > access the 4097th byte of a 4096-byte buffer (which is going to > segfault). The glibc docs say that the length is rounded up to the nearest page size [1]: Thus, addresses for mapping must be page-aligned, and length values will be rounded up. This wording in POSIX makes me think that all systems will round up to the nearest page size [2]: Thus, while the parameter len need not meet a size or alignment constraint, the system shall include, in any mapping operation, any partial page specified by the address range starting at pa and continuing for len bytes. >> char *ptr = mmap (NULL, getpagesize (), PROT_READ | PROT_WRITE, >> MAP_ANONYMOUS, -1, 0); >> if (ptr == NULL) >> abort (); > > I think you want to check for MAP_FAILED here, not NULL. And I think we > always get that, because MAP_ANONYMOUS needs to be OR-ed into MAP_SHARED > or MAP_PRIVATE. So here: > >> $ gcc -fsanitize=address main.c && ./a.out 2>&1 | grep ^SUMMARY: >> SUMMARY: AddressSanitizer: SEGV (/home/collin/a.out+0x400554) (BuildId: 1b7a82189bfffb3f73d420e138b9859add25901a) in main >> $ clang -fsanitize=address main.c && ./a.out 2>&1 | grep ^SUMMARY: >> SUMMARY: AddressSanitizer: SEGV (/home/collin/a.out+0x4e9ee6) (BuildId: aca1d168eacebaa239082d8a45ab74c8470f4b31) in main > > I don't think this is ASan finding a problem. It is just telling us that > we segfaulted for other reasons. And the fault here is because the > broken mmap() invocation returned MAP_FAILED, and we tried to access > that garbage pointer. > >> ptr[getpagesize () + 1] = 'a'; > > This is also making a map that is a multiple of the page size, and then > touching a byte that's on the next page. That's the easy-ish case that > we can often already find, even without ASan (though it depends on what > comes after the mapped memory; it might be a valid page). Oops. I clearly don't use mmap much. :) > A more interesting test for Git is to actually map a file, like: > > $ cat main.c > #include > #include > #include > #include > #include > static void die(const char *msg) > { > perror(msg); > exit(1); > } > int main (int argc, const char **argv) > { > struct stat st; > int fd; > char *ptr; > > fd = open(argv[1], O_RDONLY); > if (fd < 0) > die("open"); > if (fstat(fd, &st) < 0) > die("fstat"); > ptr = mmap (NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0); > if (ptr == MAP_FAILED) > die("mmap"); > printf("last byte: %d\n", ptr[st.st_size-1]); > printf("one byte after: %d\n", ptr[st.st_size]); > return 0; > } > $ yes | head -c 4096 >big > $ yes | head -c 372 >small > > And ASan does often detect the problem for the "big" page-sized file, > but not consistently! If I do: > > gcc -fsanitize=address main.c > while ./a.out big; do echo ok; done > > I may get output like: > > last byte: 10 > one byte after: 127 > ok > last byte: 10 > one byte after: 0 > ok > last byte: 10 > one byte after: 0 > ok > last byte: 10 > ================================================================= > ==988617==ERROR: AddressSanitizer: unknown-crash on address 0x7efd40b9f000 at pc 0x564fe77b64eb bp 0x7ffff49e8160 sp 0x7ffff49e8158 > READ of size 1 at 0x7efd40b9f000 thread T0 > #0 0x564fe77b64ea in main (/home/peff/a.out+0x14ea) (BuildId: 8db121bb5c048cb336f8be729e8cefebd6f059a3) > #1 0x7efd41233ca7 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 > #2 0x7efd41233d64 in __libc_start_main_impl ../csu/libc-start.c:360 > #3 0x564fe77b6150 in _start (/home/peff/a.out+0x1150) (BuildId: 8db121bb5c048cb336f8be729e8cefebd6f059a3) > > Address 0x7efd40b9f000 is a wild pointer inside of access range of size 0x000000000001. > > So it worked three times without ASan noticing the problem (producing > two different outputs), and then ASan finally crashed. But it didn't > give us the usual information we get for a malloc overflow. It's just an > "unknown crash" from a "wild pointer". So I'm not sure if it's even > finding these through its own poisoning, and not just catching an > unlucky segfault. > > If we switch to the small file, then ASan never reports anything! The OS > gives us a page-sized chunk, so we consistently read a "0" in from the > byte after our requested size. > > If we swap out the mmap for: > > ptr = malloc(st.st_size); > read(fd, ptr, st.st_size); > > (which is roughly what our NO_MMAP wrapper is doing behind the scenes), > then ASan does catch it consistently, even for the "small" file: > > $ ./a.out small > ================================================================= > ==1008630==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7c7d11fe01b4 at pc 0x55cf89b1b4c8 bp 0x7fff471b84e0 sp 0x7fff471b84d8 > READ of size 1 at 0x7c7d11fe01b4 thread T0 > #0 0x55cf89b1b4c7 in main (/home/peff/a.out+0x14c7) (BuildId: 2cebfcd0a00064eaaed750af010fcecdae2f5666) > #1 0x7f4d12e33ca7 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 > #2 0x7f4d12e33d64 in __libc_start_main_impl ../csu/libc-start.c:360 > #3 0x55cf89b1b150 in _start (/home/peff/a.out+0x1150) (BuildId: 2cebfcd0a00064eaaed750af010fcecdae2f5666) > > 0x7c7d11fe01b4 is located 0 bytes after 372-byte region [0x7c7d11fe0040,0x7c7d11fe01b4) > allocated by thread T0 here: > #0 0x7f4d1311a0ab in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:67 > #1 0x55cf89b1b3a0 in main (/home/peff/a.out+0x13a0) (BuildId: 2cebfcd0a00064eaaed750af010fcecdae2f5666) > #2 0x7f4d12e33ca7 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 Cool, thanks for the actual working example. Your patch makes perfect sense now. Collin [1] https://www.gnu.org/software/libc/manual/html_node/Memory_002dmapped-I_002fO.html [2] https://pubs.opengroup.org/onlinepubs/9799919799/functions/mmap.html