From 2caba086abeb8f10b4d723b3d2cc205f2da7bbc3 Mon Sep 17 00:00:00 2001 From: Roman Vinogradov Date: Wed, 18 Mar 2026 17:43:19 +0100 Subject: [PATCH] [ASan] Fix overflow and last byte handling in __asan_region_is_poisoned (#183900) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit __asan_region_is_poisoned() uses an exclusive end address (end = beg + size) to validate the region [beg, end) and to compute the aligned inner shadow region. This causes correctness issue near memory range upper boundary and could trigger address space overflow on 32-bit targets. 1. Incorrect handling of the last byte of a memory range The implementation checks AddrIsInMem(end) instead of the last application byte (end - 1). For regions ending at the last byte of Low/Mid/HighMem (e.g. __asan_region_is_poisoned(kHighMemEnd, 1)), this returns end (kHighMemEnd + 1) instead of the original pointer. This behavior is inconsistent with the function’s semantics and with __asan_address_is_poisoned(). 2) address space overflow and invalid shadow range If a region ends at the top of the virtual address space (kHighMemEnd), e.g. on 32-bit targets, end = beg + size could wrap to 0. This violated the invariant beg < end and could trigger the CHECK failure. Additionally, overflow in RoundUpTo alignment computations for aligned_b could produce an invalid shadow region spanning LowShadow to HighShadow across ShadowGap, leading mem_is_zero() to access unmapped memory and crash. Fix by switching to an inclusive last byte: last = beg + size - 1 All checks are now performed on beg and last. The aligned inner shadow region is also computed from [beg, last]. Additional guard for aligned_b prevents the mapping to shadow if aligned_b is wrapped (in this case the aligned inner region is also empty and doesn't require the shadow scan via mem_is_zero()). This fixes incorrect return values at memory range ends and prevents overflow related crashes on 32-bit targets. Test is extended to cover these boundary cases. --------- Co-authored-by: Vitaly Buka --- .../include/sanitizer/asan_interface.h | 2 +- compiler-rt/lib/asan/asan_poisoning.cpp | 20 ++++++------ .../lib/asan/tests/asan_noinst_test.cpp | 32 ++++++++++++------- .../test/asan/TestCases/wild_pointer.cpp | 2 +- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/compiler-rt/include/sanitizer/asan_interface.h b/compiler-rt/include/sanitizer/asan_interface.h index 544f2e4b3268..6060d6032228 100644 --- a/compiler-rt/include/sanitizer/asan_interface.h +++ b/compiler-rt/include/sanitizer/asan_interface.h @@ -102,7 +102,7 @@ int SANITIZER_CDECL __asan_address_is_poisoned(void const volatile *addr); /// address of the first such byte. Otherwise returns 0. /// /// \param beg Start of memory region. -/// \param size Start of memory region. +/// \param size Size of memory region. /// \returns Address of first poisoned byte. void *SANITIZER_CDECL __asan_region_is_poisoned(void *beg, size_t size); diff --git a/compiler-rt/lib/asan/asan_poisoning.cpp b/compiler-rt/lib/asan/asan_poisoning.cpp index b08737775991..89f6ea85d605 100644 --- a/compiler-rt/lib/asan/asan_poisoning.cpp +++ b/compiler-rt/lib/asan/asan_poisoning.cpp @@ -241,30 +241,32 @@ int __asan_address_is_poisoned(void const volatile *addr) { uptr __asan_region_is_poisoned(uptr beg, uptr size) { if (!size) return 0; - uptr end = beg + size; + uptr last = beg + size - 1; if (!AddrIsInMem(beg)) return beg; - if (!AddrIsInMem(end)) - return end; - CHECK_LT(beg, end); + if (!AddrIsInMem(last)) + return last; + CHECK_LE(beg, last); // First check the first and the last application bytes, - // then check the ASAN_SHADOW_GRANULARITY-aligned region by calling + // then check the ASAN_SHADOW_GRANULARITY-aligned inner region by calling // mem_is_zero on the corresponding shadow. - if (!__asan::AddressIsPoisoned(beg) && !__asan::AddressIsPoisoned(end - 1)) { + if (!__asan::AddressIsPoisoned(beg) && !__asan::AddressIsPoisoned(last)) { uptr aligned_b = RoundUpTo(beg, ASAN_SHADOW_GRANULARITY); - uptr aligned_e = RoundDownTo(end, ASAN_SHADOW_GRANULARITY); + uptr aligned_e = RoundDownTo(last, ASAN_SHADOW_GRANULARITY); if (aligned_e <= aligned_b) return 0; + if (UNLIKELY(aligned_b < beg)) // address space overflow. + return 0; uptr shadow_beg = MemToShadow(aligned_b); uptr shadow_end = MemToShadow(aligned_e); - CHECK_LE(shadow_beg, shadow_end); + CHECK_LT(shadow_beg, shadow_end); if (__sanitizer::mem_is_zero((const char*)shadow_beg, shadow_end - shadow_beg)) return 0; } // The fast check failed, so we have a poisoned byte somewhere. // Find it slowly. - for (; beg < end; beg++) + for (; beg <= last; beg++) if (__asan::AddressIsPoisoned(beg)) return beg; UNREACHABLE("mem_is_zero returned false, but poisoned byte was not found"); diff --git a/compiler-rt/lib/asan/tests/asan_noinst_test.cpp b/compiler-rt/lib/asan/tests/asan_noinst_test.cpp index 3f7abce71472..cda3764b24f1 100644 --- a/compiler-rt/lib/asan/tests/asan_noinst_test.cpp +++ b/compiler-rt/lib/asan/tests/asan_noinst_test.cpp @@ -231,30 +231,40 @@ TEST(AddressSanitizer, ShadowRegionIsPoisonedTest) { } // Test regions fully contained in the last 8 bytes (shadow granularity) -// before given end address of a memory range (LowMem / MidMem / HighMem). -// __asan_region_is_poisoned() should not crash for those regions. -static void TestRegionIsPoisonedNearEnd(uptr end) { +// of a memory range (LowMem / MidMem / HighMem). +static void TestIsPoisonedNearMemEnd(uptr end) { static const uptr granularity = 1ULL << 3; // shadow granularity for (uptr offset = 0; offset < granularity; ++offset) { - uptr ptr = end - offset; - for (uptr size = 1; ptr < ptr + size && ptr + size <= end + 1; ++size) { - uptr first_poisoned = __asan_region_is_poisoned(ptr, size); + const uptr first = end - offset; + for (uptr last = first; first <= last && last <= end; ++last) { + const uptr size = last - first + 1; + // __asan_region_is_poisoned() should not crash for the region. + uptr first_poisoned = __asan_region_is_poisoned(first, size); EXPECT_TRUE(first_poisoned == 0 || - (first_poisoned >= ptr && first_poisoned <= ptr + size)); + (first_poisoned >= first && first_poisoned <= last)) + << " first=" << (void*)first << " size=" << size + << " first_poisoned=" << (void*)first_poisoned; + + // __asan_region_is_poisoned() and __asan_address_is_poisoned() should + // behave consistently, i.e. both should return the same result. + if (size == 1) { + int is_poisoned = __asan_address_is_poisoned((void*)first); + EXPECT_EQ((void*)first_poisoned, is_poisoned ? (void*)first : 0); + } } } } -TEST(AddressSanitizer, IsPoisonedDoesNotCrashOnMemoryBoundaries) { +TEST(AddressSanitizer, IsPoisonedOnMemoryBoundariesTest) { using __asan::kHighMemEnd; using __asan::kMidMemBeg; using __asan::kMidMemEnd; - TestRegionIsPoisonedNearEnd(kLowMemEnd); + TestIsPoisonedNearMemEnd(kLowMemEnd); if (__asan::kMidMemBeg) // if mid memory is available - TestRegionIsPoisonedNearEnd(__asan::kMidMemEnd); + TestIsPoisonedNearMemEnd(__asan::kMidMemEnd); if (kHighMemBeg) // if high memory is available - TestRegionIsPoisonedNearEnd(__asan::kHighMemEnd); + TestIsPoisonedNearMemEnd(__asan::kHighMemEnd); } // Test __asan_load1 & friends. diff --git a/compiler-rt/test/asan/TestCases/wild_pointer.cpp b/compiler-rt/test/asan/TestCases/wild_pointer.cpp index 8969a285e565..248a85aa8e88 100644 --- a/compiler-rt/test/asan/TestCases/wild_pointer.cpp +++ b/compiler-rt/test/asan/TestCases/wild_pointer.cpp @@ -19,7 +19,7 @@ int main() { // On Windows, %p omits %0x and prints hex characters in upper case, // so we use PRIxPTR instead of %p. fprintf(stderr, "Expected bad addr: %#" PRIxPTR "\n", - reinterpret_cast(p + offset)); + reinterpret_cast(p + offset - 1)); // Flush it so the output came out before the asan report. fflush(stderr);