[msan] Detect dereferencing zero-alloc as use-of-uninitialized-value #155944
+6
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a zero-byte allocation is requested, MSan actually allocates 1-byte for compatibility. This change poisons that byte, to detect dereferences. Also updates the test from llvm#155934
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Thurston Dang (thurstond) ChangesWhen a zero-byte allocation is requested, MSan actually allocates 1-byte for compatibility. This change poisons that byte, to detect dereferences. Also updates the test from #155934 Full diff: https://github.com/llvm/llvm-project/pull/155944.diff 2 Files Affected:
diff --git a/compiler-rt/lib/msan/msan_allocator.cpp b/compiler-rt/lib/msan/msan_allocator.cpp
index 2b543db49d36e..64df863839c06 100644
--- a/compiler-rt/lib/msan/msan_allocator.cpp
+++ b/compiler-rt/lib/msan/msan_allocator.cpp
@@ -230,6 +230,12 @@ static void *MsanAllocate(BufferedStackTrace *stack, uptr size, uptr alignment,
__msan_set_origin(allocated, size, o.raw_id());
}
}
+
+ uptr actually_allocated_size = allocator.GetActuallyAllocatedSize(allocated);
+ // For compatibility, the allocator converted 0-sized allocations into 1 byte
+ if (size == 0 && actually_allocated_size > 0 && flags()->poison_in_malloc)
+ __msan_poison(allocated, 1);
+
UnpoisonParam(2);
RunMallocHooks(allocated, size);
return allocated;
diff --git a/compiler-rt/test/msan/zero_alloc.cpp b/compiler-rt/test/msan/zero_alloc.cpp
index e60051872eba2..6e38ce4c0a8f8 100644
--- a/compiler-rt/test/msan/zero_alloc.cpp
+++ b/compiler-rt/test/msan/zero_alloc.cpp
@@ -1,9 +1,5 @@
// RUN: %clang_msan -Wno-alloc-size -fsanitize-recover=memory %s -o %t && not %run %t 2>&1 | FileCheck %s
-// MSan doesn't catch this because internally it translates 0-byte allocations
-// into 1-byte
-// XFAIL: *
-
#include <malloc.h>
#include <stdio.h>
|
fmayer
approved these changes
Aug 28, 2025
thurstond
added a commit
that referenced
this pull request
Aug 29, 2025
…build breakage from #155943) (#156103) ASan now detects dereferences of zero-sized allocations (#155943; the corresponding MSan change is #155944). This appears to have detected a bug in CrossOverTest.cpp, causing a buildbot breakage. This patch fixes the test. Buildbot report: https://lab.llvm.org/buildbot/#/builders/4/builds/8732 ``` 7: ==949882==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf169cfbe0010 at pc 0xb5f45efc6d1c bp 0xffffd933e460 sp 0xffffd933e458 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 8: READ of size 1 at 0xf169cfbe0010 thread T0 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 9: #0 0xb5f45efc6d18 in LLVMFuzzerTestOneInput /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/test/fuzzer/CrossOverTest.cpp:48:7 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ check:20'1 ? possible intended match 10: #1 0xb5f45eec7288 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:619:13 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 11: #2 0xb5f45eec85d4 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:812:3 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 12: #3 0xb5f45eec8c60 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:872:3 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 13: #4 0xb5f45eeb5c64 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:923:6 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 14: #5 0xb5f45eee09d0 in main /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` For context, FuzzerLoop.cpp:812 tries empty input: ``` 810 // Test the callback with empty input and never try it again. 811 uint8_t dummy = 0; 812 ExecuteCallback(&dummy, 0); ```
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Aug 29, 2025
…ix-forward build breakage from #155943) (#156103) ASan now detects dereferences of zero-sized allocations (llvm/llvm-project#155943; the corresponding MSan change is llvm/llvm-project#155944). This appears to have detected a bug in CrossOverTest.cpp, causing a buildbot breakage. This patch fixes the test. Buildbot report: https://lab.llvm.org/buildbot/#/builders/4/builds/8732 ``` 7: ==949882==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf169cfbe0010 at pc 0xb5f45efc6d1c bp 0xffffd933e460 sp 0xffffd933e458 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 8: READ of size 1 at 0xf169cfbe0010 thread T0 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 9: #0 0xb5f45efc6d18 in LLVMFuzzerTestOneInput /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/test/fuzzer/CrossOverTest.cpp:48:7 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ check:20'1 ? possible intended match 10: #1 0xb5f45eec7288 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:619:13 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 11: #2 0xb5f45eec85d4 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:812:3 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 12: #3 0xb5f45eec8c60 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:872:3 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 13: #4 0xb5f45eeb5c64 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:923:6 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 14: #5 0xb5f45eee09d0 in main /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` For context, FuzzerLoop.cpp:812 tries empty input: ``` 810 // Test the callback with empty input and never try it again. 811 uint8_t dummy = 0; 812 ExecuteCallback(&dummy, 0); ```
mdenson
pushed a commit
to mdenson/llvm-project
that referenced
this pull request
Aug 29, 2025
…build breakage from llvm#155943) (llvm#156103) ASan now detects dereferences of zero-sized allocations (llvm#155943; the corresponding MSan change is llvm#155944). This appears to have detected a bug in CrossOverTest.cpp, causing a buildbot breakage. This patch fixes the test. Buildbot report: https://lab.llvm.org/buildbot/#/builders/4/builds/8732 ``` 7: ==949882==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf169cfbe0010 at pc 0xb5f45efc6d1c bp 0xffffd933e460 sp 0xffffd933e458 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 8: READ of size 1 at 0xf169cfbe0010 thread T0 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 9: #0 0xb5f45efc6d18 in LLVMFuzzerTestOneInput /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/test/fuzzer/CrossOverTest.cpp:48:7 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ check:20'1 ? possible intended match 10: llvm#1 0xb5f45eec7288 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:619:13 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 11: llvm#2 0xb5f45eec85d4 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:812:3 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 12: llvm#3 0xb5f45eec8c60 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:872:3 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 13: llvm#4 0xb5f45eeb5c64 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:923:6 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 14: llvm#5 0xb5f45eee09d0 in main /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10 check:20'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` For context, FuzzerLoop.cpp:812 tries empty input: ``` 810 // Test the callback with empty input and never try it again. 811 uint8_t dummy = 0; 812 ExecuteCallback(&dummy, 0); ```
vitalybuka
reviewed
Aug 30, 2025
| @@ -230,6 +230,12 @@ static void *MsanAllocate(BufferedStackTrace *stack, uptr size, uptr alignment, | |||
| __msan_set_origin(allocated, size, o.raw_id()); | |||
| } | |||
| } | |||
|
|
|||
| uptr actually_allocated_size = allocator.GetActuallyAllocatedSize(allocated); | |||
There was a problem hiding this comment.
Shouldn't this be like:
if (flags()->poison_in_malloc) {
__msan_poison(allocated + size, allocator.GetActuallyAllocatedSize(allocated) - size);
}
There was a problem hiding this comment.
It would be expensive to do the extra poisoning. The main advantage is it would provide a small amount of buffer overflow protection, but that is outside the scope of MSan. If users want to reliably detect buffer overflows, they should use ASan/HWASan/MTE.
MSan gives no guarantees about:
char *p = malloc(12); // Allocator actually allocates 16 bytes
printf ("%d\n", p[100]); // Whether MSan catches this or not depends on which memory it reuses
There's no particular reason it should care about p[12] or p[15] either, which are allocated only because of the nuances of the sanitizer allocator's size classes.
There was a problem hiding this comment.
Why did you create this patch than, it falls into the same category?
There was a problem hiding this comment.
It would be expensive to do the extra poisoning.
I don't think so. It's about alignment tail which likely smaller than primary allocation, which we poison anyway.
There was a problem hiding this comment.
It would be expensive to do the extra poisoning.
I don't think so. It's about alignment tail which likely smaller than requested allocation, which we poison anyway.
There was a problem hiding this comment.
Why did you create this patch than, it falls into the same category?
Zero-sized allocations are a special case. POSIX allows that malloc(0) could even return NULL, which definitely should not be dereferenced. The new behavior (returning a non-NULL pointer, but not allowing it to be dereferenced) is a compromise between compatibility and detection.
There was a problem hiding this comment.
It would be expensive to do the extra poisoning.
I don't think so. It's about alignment tail which likely smaller than requested allocation, which we poison anyway.
It is expensive from a cost-benefit perspective. It has some cost, but the benefit is limited (detecting issues that are out of scope for MSan).
There was a problem hiding this comment.
Why did you create this patch than, it falls into the same category?
Zero-sized allocations are a special case. POSIX allows that malloc(0) could even return NULL, which definitely should not be dereferenced. The new behavior (returning a non-NULL pointer, but not allowing it to be dereferenced) is a compromise between compatibility and detection.
It is expensive from a cost-benefit perspective. It has some cost, but the benefit is limited (detecting issues that are out of scope for MSan).
a. my intuition is it's almost zero cost, you would not be able to measure
b. this patch is already "out of scope" (it's essentially OOB detection)
There was a problem hiding this comment.
b. this patch is already "out of scope" (it's essentially OOB detection)
To be logically consistent with the position that MSan does not handle OOB, I've made a revert pull request for this patch: #156148
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a zero-byte allocation is requested, MSan actually allocates 1-byte for compatibility. This change poisons that byte, to detect dereferences.
Also updates the test from #155934