[PATCH] D110365: [llvm][profile] Do not read padding when printing build IDs

Leonard Chan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 23 14:17:44 PDT 2021


leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr, gulfem.
Herald added a subscriber: hiraditya.
leonardchan requested review of this revision.
Herald added a project: LLVM.

PPC builders appear to be segfaulting when printing binary IDs after 6bc9c8dfe32cc4662f2ed9041af527f69dfff13b <https://reviews.llvm.org/rG6bc9c8dfe32cc4662f2ed9041af527f69dfff13b>. It's likely because when we reach the padding, a uint64_t is still attempted to be read, but since the padding will always be less than 8 bytes, the 8-byte read could read some bytes from the following data section. This the binary ID pointer is advanced by this amount, then we end up reading from some random address.

This patch adds an extra check to see that (1) we are able to read 8-bytes for the binary ID length, and (2) if not, then check for padding. The remaining bytes should all be zero, otherwise the data is malformed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110365

Files:
  llvm/lib/ProfileData/InstrProfReader.cpp


Index: llvm/lib/ProfileData/InstrProfReader.cpp
===================================================================
--- llvm/lib/ProfileData/InstrProfReader.cpp
+++ llvm/lib/ProfileData/InstrProfReader.cpp
@@ -527,8 +527,24 @@
 
   OS << "Binary IDs: \n";
   const uint8_t *BI = BinaryIdsStart;
-  while (BI < BinaryIdsStart + BinaryIdsSize) {
-    uint64_t BinaryIdLen = swap(*reinterpret_cast<const uint64_t *>(BI));
+  const uint8_t *BIEnd = BinaryIdsStart + BinaryIdsSize;
+  while (BI < BIEnd) {
+    uint64_t BinaryIdLen = 0;
+    auto Remaining = BIEnd - BI;
+
+    // If we do not have enough to read a uint64_t, then this remaining data
+    // must be zero-padding. If it's not padded, then the data must be corrupted
+    // somehow. In case of misalignment, we can use memcpy() for safe reads.
+    if (Remaining < sizeof(BinaryIdLen)) {
+      memcpy(&BinaryIDLen, BI, Remaining);
+      if (!BinaryIdLen)
+        return success();
+      return make_error<InstrProfError>(instrprof_error::malformed);
+    }
+
+    memcpy(&BinaryIdLen, BI, sizeof(BinaryIDLen));
+    BinaryIdLen = swap(BinaryIdLen);
+
     // Increment by binary id length data type size.
     BI += sizeof(BinaryIdLen);
     if (BI > (const uint8_t *)DataBuffer->getBufferEnd())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110365.374665.patch
Type: text/x-patch
Size: 1262 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210923/62b9a3aa/attachment-0001.bin>


More information about the llvm-commits mailing list