[compiler-rt] a4e8d89 - [GWP-ASan] Only pack frames that are stored.

[Mitch Phillips via llvm-commits]

Author: Mitch Phillips
Date: 2020-03-24T14:37:09-07:00
New Revision: a4e8d89704d2584d5c56cb27745beab25c7b9b36

URL: https://github.com/llvm/llvm-project/commit/a4e8d89704d2584d5c56cb27745beab25c7b9b36
DIFF: https://github.com/llvm/llvm-project/commit/a4e8d89704d2584d5c56cb27745beab25c7b9b36.diff

LOG: [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

Added: 
    

Modified: 
    compiler-rt/lib/gwp_asan/common.cpp
    compiler-rt/lib/gwp_asan/tests/backtrace.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/gwp_asan/common.cpp b/compiler-rt/lib/gwp_asan/common.cpp
index 44935817f8e9..3438c4b91893 100644
--- a/compiler-rt/lib/gwp_asan/common.cpp
+++ b/compiler-rt/lib/gwp_asan/common.cpp
@@ -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);

diff  --git a/compiler-rt/lib/gwp_asan/tests/backtrace.cpp b/compiler-rt/lib/gwp_asan/tests/backtrace.cpp
index bc81f35cb379..6c9a9309ed8b 100644
--- a/compiler-rt/lib/gwp_asan/tests/backtrace.cpp
+++ b/compiler-rt/lib/gwp_asan/tests/backtrace.cpp
@@ -8,6 +8,7 @@
 
 #include <string>
 
+#include "gwp_asan/crash_handler.h"
 #include "gwp_asan/tests/harness.h"
 
 TEST_F(BacktraceGuardedPoolAllocator, DoubleFree) {
@@ -15,13 +16,13 @@ TEST_F(BacktraceGuardedPoolAllocator, DoubleFree) {
   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);
 }
 
@@ -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));
+}