[PATCH] D66979: [InstrProf] Tighten a check for malformed data records in raw profiles

[Vedant Kumar via Phabricator via llvm-commits]

vsk created this revision.
vsk added reviewers: davidxl, Dor1s.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

The check needs to validate a counter offset before performing pointer
arithmetic with the (potentially corrupt) offset.

Found by UBSan's pointer overflow check.

rdar://54843625


https://reviews.llvm.org/D66979

Files:
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/lib/ProfileData/InstrProfReader.cpp
  llvm/test/tools/llvm-profdata/Inputs/malformed-ptr-to-counter-array.profraw
  llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test


Index: llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test
===================================================================
--- /dev/null
+++ llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test
@@ -0,0 +1,5 @@
+REQUIRES: zlib
+
+RUN: not llvm-profdata merge -o /dev/null %p/Inputs/malformed-ptr-to-counter-array.profraw 2>&1 | FileCheck %s
+
+CHECK: Malformed instrumentation profile data
Index: llvm/lib/ProfileData/InstrProfReader.cpp
===================================================================
--- llvm/lib/ProfileData/InstrProfReader.cpp
+++ llvm/lib/ProfileData/InstrProfReader.cpp
@@ -413,14 +413,17 @@
   if (NumCounters == 0)
     return error(instrprof_error::malformed);
 
-  auto RawCounts = makeArrayRef(getCounter(CounterPtr), NumCounters);
   auto *NamesStartAsCounter = reinterpret_cast<const uint64_t *>(NamesStart);
 
-  // Check bounds.
-  if (RawCounts.data() < CountersStart ||
-      RawCounts.data() + RawCounts.size() > NamesStartAsCounter)
+  // Check bounds. Note that the counter pointer embedded in the data record
+  // may itself be corrupt.
+  ptrdiff_t CounterOffset = getCounterOffset(CounterPtr);
+  if (CounterOffset < 0 ||
+      CounterOffset >= (NamesStartAsCounter - CountersStart))
     return error(instrprof_error::malformed);
 
+  auto RawCounts = makeArrayRef(getCounter(CounterOffset), NumCounters);
+
   if (ShouldSwapBytes) {
     Record.Counts.clear();
     Record.Counts.reserve(RawCounts.size());
Index: llvm/include/llvm/ProfileData/InstrProfReader.h
===================================================================
--- llvm/include/llvm/ProfileData/InstrProfReader.h
+++ llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -268,8 +268,11 @@
       return (const char *)ValueDataStart;
   }
 
-  const uint64_t *getCounter(IntPtrT CounterPtr) const {
-    ptrdiff_t Offset = (swap(CounterPtr) - CountersDelta) / sizeof(uint64_t);
+  ptrdiff_t getCounterOffset(IntPtrT CounterPtr) const {
+    return (swap(CounterPtr) - CountersDelta) / sizeof(uint64_t);
+  }
+
+  const uint64_t *getCounter(ptrdiff_t Offset) const {
     return CountersStart + Offset;
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66979.217990.patch
Type: text/x-patch
Size: 2152 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190829/9185dfee/attachment.bin>