Skip to content

Commit

Permalink
[GWP-ASan] Only pack frames that are stored.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
hctim committed Mar 24, 2020
1 parent 3f1defa commit a4e8d89
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 6 deletions.
5 changes: 5 additions & 0 deletions compiler-rt/lib/gwp_asan/common.cpp
Expand Up @@ -59,6 +59,11 @@ void AllocationMetadata::CallSiteInfo::RecordBacktrace(
uintptr_t UncompressedBuffer[kMaxTraceLengthToCollect];
size_t BacktraceLength =
Backtrace(UncompressedBuffer, kMaxTraceLengthToCollect);
// Backtrace() returns the number of available frames, which may be greater
// than the number of frames in the buffer. In this case, we need to only pack
// the number of frames that are in the buffer.
if (BacktraceLength > kMaxTraceLengthToCollect)
BacktraceLength = kMaxTraceLengthToCollect;
TraceSize =
compression::pack(UncompressedBuffer, BacktraceLength, CompressedTrace,
AllocationMetadata::kStackFrameStorageBytes);
Expand Down
37 changes: 31 additions & 6 deletions compiler-rt/lib/gwp_asan/tests/backtrace.cpp
Expand Up @@ -8,20 +8,21 @@

#include <string>

#include "gwp_asan/crash_handler.h"
#include "gwp_asan/tests/harness.h"

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

std::string DeathRegex = "Double Free.*";
DeathRegex.append("backtrace\\.cpp:25.*");
DeathRegex.append("backtrace\\.cpp:26.*");

DeathRegex.append("was deallocated.*");
DeathRegex.append("backtrace\\.cpp:15.*");
DeathRegex.append("backtrace\\.cpp:16.*");

DeathRegex.append("was allocated.*");
DeathRegex.append("backtrace\\.cpp:14.*");
DeathRegex.append("backtrace\\.cpp:15.*");
ASSERT_DEATH(GPA.deallocate(Ptr), DeathRegex);
}

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

std::string DeathRegex = "Use After Free.*";
DeathRegex.append("backtrace\\.cpp:40.*");
DeathRegex.append("backtrace\\.cpp:41.*");

DeathRegex.append("was deallocated.*");
DeathRegex.append("backtrace\\.cpp:30.*");
DeathRegex.append("backtrace\\.cpp:31.*");

DeathRegex.append("was allocated.*");
DeathRegex.append("backtrace\\.cpp:29.*");
DeathRegex.append("backtrace\\.cpp:30.*");
ASSERT_DEATH({ *Ptr = 7; }, DeathRegex);
}

TEST(Backtrace, Short) {
gwp_asan::AllocationMetadata Meta;
Meta.AllocationTrace.RecordBacktrace(
[](uintptr_t *TraceBuffer, size_t /* Size */) -> size_t {
TraceBuffer[0] = 123u;
TraceBuffer[1] = 321u;
return 2u;
});
uintptr_t TraceOutput[2] = {};
EXPECT_EQ(2u, __gwp_asan_get_allocation_trace(&Meta, TraceOutput, 2));
EXPECT_EQ(TraceOutput[0], 123u);
EXPECT_EQ(TraceOutput[1], 321u);
}

TEST(Backtrace, ExceedsStorableLength) {
gwp_asan::AllocationMetadata Meta;
Meta.AllocationTrace.RecordBacktrace(
[](uintptr_t * /* TraceBuffer */, size_t /* Size */) -> size_t {
return SIZE_MAX; // Wow, that's big!
});
uintptr_t TraceOutput;
EXPECT_EQ(1u, __gwp_asan_get_allocation_trace(&Meta, &TraceOutput, 1));
}

0 comments on commit a4e8d89

Please sign in to comment.