Skip to content

[asan] Fix unknown-crash reported for multi-byte errors #144480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wxwern
Copy link

@wxwern wxwern commented Jun 17, 2025

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:

https://godbolt.org/z/abrjrvnzj

# minimal reprod (should occur on gcc-7 - gcc-15, x86_64)
#
# 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).

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@wxwern wxwern changed the title [libasan] Fix unknown-crash reported for multi-byte errors [asan] Fix unknown-crash reported for multi-byte errors Jun 17, 2025
@wxwern wxwern force-pushed the fix-unknown-crash-desc-for-multi-byte-err branch from 0a9f98e to 4a46b7a Compare June 17, 2025 09:06
@wxwern wxwern marked this pull request as ready for review June 18, 2025 02:23
@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2025

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Wern (wxwern)

Changes

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:

https://godbolt.org/z/abrjrvnzj

# minimal reprod (should occur on gcc-7 - gcc-15)
#
# 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).


Full diff: https://github.com/llvm/llvm-project/pull/144480.diff

1 Files Affected:

  • (modified) compiler-rt/lib/asan/asan_errors.cpp (+5-2)
diff --git a/compiler-rt/lib/asan/asan_errors.cpp b/compiler-rt/lib/asan/asan_errors.cpp
index 2a207cd06ccac..9e109c0895589 100644
--- a/compiler-rt/lib/asan/asan_errors.cpp
+++ b/compiler-rt/lib/asan/asan_errors.cpp
@@ -437,8 +437,11 @@ ErrorGeneric::ErrorGeneric(u32 tid, uptr pc_, uptr bp_, uptr sp_, uptr addr,
     bug_descr = "unknown-crash";
     if (AddrIsInMem(addr)) {
       u8 *shadow_addr = (u8 *)MemToShadow(addr);
-      // If we are accessing 16 bytes, look at the second shadow byte.
-      if (*shadow_addr == 0 && access_size > ASAN_SHADOW_GRANULARITY)
+      u8 *shadow_addr_upper_bound =
+          shadow_addr + (1 + ((access_size - 1) / ASAN_SHADOW_GRANULARITY));
+      // If the read could span multiple shadow bytes,
+      // do a sequential scan and look for the first bad shadow byte.
+      while (*shadow_addr == 0 && shadow_addr < shadow_addr_upper_bound)
         shadow_addr++;
       // If we are in the partial right redzone, look at the next shadow byte.
       if (*shadow_addr > 0 && *shadow_addr < 128) shadow_addr++;

@wxwern wxwern force-pushed the fix-unknown-crash-desc-for-multi-byte-err branch from 4a46b7a to 69f24b6 Compare June 18, 2025 02:26
@vitalybuka vitalybuka self-requested a review June 18, 2025 02:37
@vitalybuka
Copy link
Collaborator

We need a test for that

@vitalybuka
Copy link
Collaborator

The above example doesn't reproduce the issue on clang as it reports errors via different pathways:

You can probably trigger that path through ACCESS_MEMORY_RANGE and INTERCEPTORs?

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).
@wxwern wxwern force-pushed the fix-unknown-crash-desc-for-multi-byte-err branch from 69f24b6 to f39a0fe Compare June 18, 2025 03:03
@wxwern
Copy link
Author

wxwern commented Jun 18, 2025

We need a test for that

The above example doesn't reproduce the issue on clang as it reports errors via different pathways:

You can probably trigger that path through ACCESS_MEMORY_RANGE and INTERCEPTORs?

Thanks, will look into it. I've not written tests yet as I haven't found a way to reproduce this via clang, will do once I can find a reproducible example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants