Skip to content

Commit 4a46b7a

Browse files
committed
[asan] Fix 'unknown-crash' reported for multi-byte errors
Given that a reported error by asan spans multiple bytes, asan may flag the error as an 'unknown-crash' instead of the appropriate error name. This error can be reproduced via a partial buffer overflow (on gcc), which reports 'unknown-crash' instead of 'stack-buffer-overflow' for the below: # minimal reprod (should occur on gcc-7 - gcc-15) # https://godbolt.org/z/abrjrvnzj # # gcc -fsanitize=address reprod.c struct X { char bytes[16]; }; __attribute__((noinline)) struct X out_of_bounds() { volatile char bytes[16]; struct X* x_ptr = (struct X*)(bytes + 2); return *x_ptr; } int main() { struct X x = out_of_bounds(); return x.bytes[0]; } This is due to a flawed heuristic in asan_errors.cpp, which won't always locate the appropriate shadow byte that would indicate a corresponding error. This can happen for any reported errors which span either: exactly 8 bytes, or 16 and more bytes. The above example doesn't reproduce the issue on clang as it reports errors via different pathways: - gcc-compiled binaries report the starting address and size of the failing read attempt to asan. - clang-compiled binaries highlight the first byte access that overflows the buffer to asan. Note: out-of-scope, but this is also possibly misleading, as it still reports the full size of the read attempt, paired with an address that's not the start of the read. This behavior appears to be identical for all past versions tested. I'm not aware of a way to replicate this specific issue with clang, though it might have impacted error reporting in other areas. This patch resolves this issue via a linear scan of applicable shadow bytes (instead of the original heuristic, which, at best, only increments the shadow byte address by 1 for these scenarios).
1 parent cb355de commit 4a46b7a

File tree

1 file changed

+5
-2
lines changed

1 file changed

+5
-2
lines changed

compiler-rt/lib/asan/asan_errors.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,11 @@ ErrorGeneric::ErrorGeneric(u32 tid, uptr pc_, uptr bp_, uptr sp_, uptr addr,
437437
bug_descr = "unknown-crash";
438438
if (AddrIsInMem(addr)) {
439439
u8 *shadow_addr = (u8 *)MemToShadow(addr);
440-
// If we are accessing 16 bytes, look at the second shadow byte.
441-
if (*shadow_addr == 0 && access_size > ASAN_SHADOW_GRANULARITY)
440+
u8 *shadow_addr_upper_bound =
441+
shadow_addr + (1 + ((access_size - 1) / ASAN_SHADOW_GRANULARITY));
442+
// If the read could span multiple shadow bytes,
443+
// do a sequential scan and look for the first bad shadow byte.
444+
while (*shadow_addr == 0 && shadow_addr < shadow_addr_upper_bound)
442445
shadow_addr++;
443446
// If we are in the partial right redzone, look at the next shadow byte.
444447
if (*shadow_addr > 0 && *shadow_addr < 128) shadow_addr++;

0 commit comments

Comments
 (0)