Skip to content

Commit a4e8d89

Browse files
committed
[GWP-ASan] Only pack frames that are stored.
Summary: Backtrace() returns the number of frames that are *available*, rather than the number of frames stored. When we pack, we supply the number of frames we have stored. The number of available frames can exceed the number of stored frames, leading to stack OOB read. Fix up this problem. Reviewers: eugenis Reviewed By: eugenis Subscribers: #sanitizers, morehouse, cferris, pcc Tags: #sanitizers Differential Revision: https://reviews.llvm.org/D76722
1 parent 3f1defa commit a4e8d89

File tree

2 files changed

+36
-6
lines changed

2 files changed

+36
-6
lines changed

compiler-rt/lib/gwp_asan/common.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ void AllocationMetadata::CallSiteInfo::RecordBacktrace(
5959
uintptr_t UncompressedBuffer[kMaxTraceLengthToCollect];
6060
size_t BacktraceLength =
6161
Backtrace(UncompressedBuffer, kMaxTraceLengthToCollect);
62+
// Backtrace() returns the number of available frames, which may be greater
63+
// than the number of frames in the buffer. In this case, we need to only pack
64+
// the number of frames that are in the buffer.
65+
if (BacktraceLength > kMaxTraceLengthToCollect)
66+
BacktraceLength = kMaxTraceLengthToCollect;
6267
TraceSize =
6368
compression::pack(UncompressedBuffer, BacktraceLength, CompressedTrace,
6469
AllocationMetadata::kStackFrameStorageBytes);

compiler-rt/lib/gwp_asan/tests/backtrace.cpp

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,21 @@
88

99
#include <string>
1010

11+
#include "gwp_asan/crash_handler.h"
1112
#include "gwp_asan/tests/harness.h"
1213

1314
TEST_F(BacktraceGuardedPoolAllocator, DoubleFree) {
1415
void *Ptr = GPA.allocate(1);
1516
GPA.deallocate(Ptr);
1617

1718
std::string DeathRegex = "Double Free.*";
18-
DeathRegex.append("backtrace\\.cpp:25.*");
19+
DeathRegex.append("backtrace\\.cpp:26.*");
1920

2021
DeathRegex.append("was deallocated.*");
21-
DeathRegex.append("backtrace\\.cpp:15.*");
22+
DeathRegex.append("backtrace\\.cpp:16.*");
2223

2324
DeathRegex.append("was allocated.*");
24-
DeathRegex.append("backtrace\\.cpp:14.*");
25+
DeathRegex.append("backtrace\\.cpp:15.*");
2526
ASSERT_DEATH(GPA.deallocate(Ptr), DeathRegex);
2627
}
2728

@@ -30,12 +31,36 @@ TEST_F(BacktraceGuardedPoolAllocator, UseAfterFree) {
3031
GPA.deallocate(Ptr);
3132

3233
std::string DeathRegex = "Use After Free.*";
33-
DeathRegex.append("backtrace\\.cpp:40.*");
34+
DeathRegex.append("backtrace\\.cpp:41.*");
3435

3536
DeathRegex.append("was deallocated.*");
36-
DeathRegex.append("backtrace\\.cpp:30.*");
37+
DeathRegex.append("backtrace\\.cpp:31.*");
3738

3839
DeathRegex.append("was allocated.*");
39-
DeathRegex.append("backtrace\\.cpp:29.*");
40+
DeathRegex.append("backtrace\\.cpp:30.*");
4041
ASSERT_DEATH({ *Ptr = 7; }, DeathRegex);
4142
}
43+
44+
TEST(Backtrace, Short) {
45+
gwp_asan::AllocationMetadata Meta;
46+
Meta.AllocationTrace.RecordBacktrace(
47+
[](uintptr_t *TraceBuffer, size_t /* Size */) -> size_t {
48+
TraceBuffer[0] = 123u;
49+
TraceBuffer[1] = 321u;
50+
return 2u;
51+
});
52+
uintptr_t TraceOutput[2] = {};
53+
EXPECT_EQ(2u, __gwp_asan_get_allocation_trace(&Meta, TraceOutput, 2));
54+
EXPECT_EQ(TraceOutput[0], 123u);
55+
EXPECT_EQ(TraceOutput[1], 321u);
56+
}
57+
58+
TEST(Backtrace, ExceedsStorableLength) {
59+
gwp_asan::AllocationMetadata Meta;
60+
Meta.AllocationTrace.RecordBacktrace(
61+
[](uintptr_t * /* TraceBuffer */, size_t /* Size */) -> size_t {
62+
return SIZE_MAX; // Wow, that's big!
63+
});
64+
uintptr_t TraceOutput;
65+
EXPECT_EQ(1u, __gwp_asan_get_allocation_trace(&Meta, &TraceOutput, 1));
66+
}

0 commit comments

Comments
 (0)