git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/9] Makefile: turn on NO_MMAP when building with ASan

From
Collin Funk <collin.funk1@gmail.com>
Date
Nov 12, 2025, 20:06 UTC
Message-ID
<87qzu32ey0.fsf@gmail.com>
In-Reply-To
<20251112103158.GA983233@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 21 quoted lines
> 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.
Show 25 quoted lines
>>       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. :)
Show 95 quoted lines
> A more interesting test for Git is to actually map a file, like:
>
>   $ cat main.c
>   #include <unistd.h>
>   #include <fcntl.h>
>   #include <sys/mman.h>
>   #include <sys/stat.h>
>   #include <stdio.h>
>   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

Previous: Jeff KingNext: Patrick Steinhardt
Message 10 of 64 in “asan bonanza”
  1. 0/9 asan bonanzaJeff King, Nov 12, 2025
  2. 1/9 compat/mmap: mark unused argument in git_munmap()Jeff King, Nov 12, 2025
  3. 2/9 pack-bitmap: handle name-hash lookups in incremental bitmapsJeff King, Nov 12, 2025
  4. Patrick SteinhardtNov 12, 2025
  5. Taylor BlauNov 13, 2025
  6. Jeff KingNov 18, 2025
  7. 3/9 Makefile: turn on NO_MMAP when building with ASanJeff King, Nov 12, 2025
  8. Collin FunkNov 12, 2025
  9. Jeff KingNov 12, 2025
  10. Collin FunkNov 12, 2025
  11. Patrick SteinhardtNov 12, 2025
  12. Taylor BlauNov 13, 2025
  13. Patrick SteinhardtNov 13, 2025
  14. Jeff KingNov 18, 2025
  15. Junio C HamanoNov 13, 2025
  16. Patrick SteinhardtNov 14, 2025
  17. Jeff KingNov 15, 2025
  18. 4/9 cache-tree: avoid strtol() on non-string bufferJeff King, Nov 12, 2025
  19. Patrick SteinhardtNov 12, 2025
  20. Taylor BlauNov 13, 2025
  21. Jeff KingNov 18, 2025
  22. Jeff KingNov 18, 2025
  23. 5/9 fsck: assert newline presence in fsck_ident()Jeff King, Nov 12, 2025
  24. 6/9 fsck: avoid strcspn() in fsck_ident()Jeff King, Nov 12, 2025
  25. 7/9 fsck: remove redundant date timestamp checkJeff King, Nov 12, 2025
  26. 8/9 fsck: avoid parse_timestamp() on buffer that isn't NUL-terminatedJeff King, Nov 12, 2025
  27. Patrick SteinhardtNov 12, 2025
  28. Junio C HamanoNov 12, 2025
  29. Jeff KingNov 15, 2025
  30. 9/9 t: enable ASan's strict_string_checks optionJeff King, Nov 12, 2025
  31. Taylor BlauNov 13, 2025
  32. 0/9 asan bonanzaJeff King, Nov 18, 2025
  33. 1/9 compat/mmap: mark unused argument in git_munmap()Jeff King, Nov 18, 2025
  34. 2/9 pack-bitmap: handle name-hash lookups in incremental bitmapsJeff King, Nov 18, 2025
  35. 3/9 Makefile: turn on NO_MMAP when building with ASanJeff King, Nov 18, 2025
  36. 4/9 cache-tree: avoid strtol() on non-string bufferJeff King, Nov 18, 2025
  37. Phillip WoodNov 18, 2025
  38. Junio C HamanoNov 23, 2025
  39. Phillip WoodNov 23, 2025
  40. Junio C HamanoNov 23, 2025
  41. Jeff KingNov 24, 2025
  42. Junio C HamanoNov 24, 2025
  43. Jeff KingNov 26, 2025
  44. Junio C HamanoNov 26, 2025
  45. 0/4 more robust functions for parsing int from bufJeff King, Nov 30, 2025
  46. 1/4 parse: prefer bool to int for boolean returnsJeff King, Nov 30, 2025
  47. Patrick SteinhardtDec 4, 2025
  48. 2/4 parse: add functions for parsing from non-string buffersJeff King, Nov 30, 2025
  49. my complaints with clarJeff King, Nov 30, 2025
  50. Phillip WoodDec 1, 2025
  51. Patrick SteinhardtDec 4, 2025
  52. Jeff KingDec 5, 2025
  53. Patrick SteinhardtDec 4, 2025
  54. Phillip WoodDec 5, 2025
  55. Junio C HamanoJan 20, 2026
  56. Jeff KingJan 21, 2026
  57. 3/4 cache-tree: use parse_int_from_buf()Jeff King, Nov 30, 2025
  58. 4/4 fsck: use parse_unsigned_from_buf() for parsing timestampJeff King, Nov 30, 2025
  59. 5/9 fsck: assert newline presence in fsck_ident()Jeff King, Nov 18, 2025
  60. 6/9 fsck: avoid strcspn() in fsck_ident()Jeff King, Nov 18, 2025
  61. 7/9 fsck: remove redundant date timestamp checkJeff King, Nov 18, 2025
  62. 8/9 fsck: avoid parse_timestamp() on buffer that isn't NUL-terminatedJeff King, Nov 18, 2025
  63. 9/9 t: enable ASan's strict_string_checks optionJeff King, Nov 18, 2025
  64. Junio C HamanoNov 23, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.