[compiler-rt] 189428c - [Profile] Handle invalid profile data
Arthur Eubanks via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 10 16:12:11 PDT 2021
Author: Arthur Eubanks
Date: 2021-06-10T16:10:13-07:00
New Revision: 189428c8fc2465c25efbf4f0bb73e26fecf150ce
URL: https://github.com/llvm/llvm-project/commit/189428c8fc2465c25efbf4f0bb73e26fecf150ce
DIFF: https://github.com/llvm/llvm-project/commit/189428c8fc2465c25efbf4f0bb73e26fecf150ce.diff
LOG: [Profile] Handle invalid profile data
This mostly follows LLVM's InstrProfReader.cpp error handling.
Previously, attempting to merge corrupted profile data would result in
crashes. See https://crbug.com/1216811#c4.
Reviewed By: rnk
Differential Revision: https://reviews.llvm.org/D104050
Added:
compiler-rt/test/profile/Linux/corrupted-profile.c
Modified:
compiler-rt/lib/profile/InstrProfiling.h
compiler-rt/lib/profile/InstrProfilingFile.c
compiler-rt/lib/profile/InstrProfilingMerge.c
compiler-rt/test/profile/Linux/instrprof-merge-vp.c
compiler-rt/test/profile/instrprof-merge.c
compiler-rt/test/profile/instrprof-without-libc.c
Removed:
################################################################################
diff --git a/compiler-rt/lib/profile/InstrProfiling.h b/compiler-rt/lib/profile/InstrProfiling.h
index 7d1c77a3fab3..39fe4db73da6 100644
--- a/compiler-rt/lib/profile/InstrProfiling.h
+++ b/compiler-rt/lib/profile/InstrProfiling.h
@@ -101,13 +101,13 @@ void __llvm_profile_reset_counters(void);
/*!
* \brief Merge profile data from buffer.
*
- * Read profile data form buffer \p Profile and merge with
- * in-process profile counters. The client is expected to
- * have checked or already knows the profile data in the
- * buffer matches the in-process counter structure before
- * calling it.
+ * Read profile data form buffer \p Profile and merge with in-process profile
+ * counters. The client is expected to have checked or already knows the profile
+ * data in the buffer matches the in-process counter structure before calling
+ * it. Returns 0 (success) if the profile data is valid. Upon reading
+ * invalid/corrupted profile data, returns 1 (failure).
*/
-void __llvm_profile_merge_from_buffer(const char *Profile, uint64_t Size);
+int __llvm_profile_merge_from_buffer(const char *Profile, uint64_t Size);
/*! \brief Check if profile in buffer matches the current binary.
*
diff --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index 5da709c2a724..420e8246f433 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -261,9 +261,13 @@ static int doProfileMerging(FILE *ProfileFile, int *MergeDone) {
return -1;
/* Now start merging */
- __llvm_profile_merge_from_buffer(ProfileBuffer, ProfileFileSize);
+ if (__llvm_profile_merge_from_buffer(ProfileBuffer, ProfileFileSize)) {
+ PROF_ERR("%s\n", "Invalid profile data to merge");
+ (void)munmap(ProfileBuffer, ProfileFileSize);
+ return -1;
+ }
- // Truncate the file in case merging of value profile did not happend to
+ // Truncate the file in case merging of value profile did not happen to
// prevent from leaving garbage data at the end of the profile file.
(void)COMPILER_RT_FTRUNCATE(ProfileFile,
__llvm_profile_get_size_for_buffer());
diff --git a/compiler-rt/lib/profile/InstrProfilingMerge.c b/compiler-rt/lib/profile/InstrProfilingMerge.c
index 0fd9b2bcd41f..4dce2d119dbe 100644
--- a/compiler-rt/lib/profile/InstrProfilingMerge.c
+++ b/compiler-rt/lib/profile/InstrProfilingMerge.c
@@ -82,13 +82,13 @@ int __llvm_profile_check_compatibility(const char *ProfileData,
}
COMPILER_RT_VISIBILITY
-void __llvm_profile_merge_from_buffer(const char *ProfileData,
- uint64_t ProfileSize) {
+int __llvm_profile_merge_from_buffer(const char *ProfileData,
+ uint64_t ProfileSize) {
__llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;
__llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;
uint64_t *SrcCountersStart;
const char *SrcNameStart;
- ValueProfData *SrcValueProfDataStart, *SrcValueProfData;
+ const char *SrcValueProfDataStart, *SrcValueProfData;
SrcDataStart =
(__llvm_profile_data *)(ProfileData + sizeof(__llvm_profile_header));
@@ -96,37 +96,47 @@ void __llvm_profile_merge_from_buffer(const char *ProfileData,
SrcCountersStart = (uint64_t *)SrcDataEnd;
SrcNameStart = (const char *)(SrcCountersStart + Header->CountersSize);
SrcValueProfDataStart =
- (ValueProfData *)(SrcNameStart + Header->NamesSize +
- __llvm_profile_get_num_padding_bytes(
- Header->NamesSize));
+ SrcNameStart + Header->NamesSize +
+ __llvm_profile_get_num_padding_bytes(Header->NamesSize);
+ if (SrcNameStart < (const char *)SrcCountersStart)
+ return 1;
for (SrcData = SrcDataStart,
DstData = (__llvm_profile_data *)__llvm_profile_begin_data(),
SrcValueProfData = SrcValueProfDataStart;
SrcData < SrcDataEnd; ++SrcData, ++DstData) {
- uint64_t *SrcCounters;
uint64_t *DstCounters = (uint64_t *)DstData->CounterPtr;
- unsigned I, NC, NVK = 0;
+ unsigned NVK = 0;
- NC = SrcData->NumCounters;
- SrcCounters = SrcCountersStart +
- ((size_t)SrcData->CounterPtr - Header->CountersDelta) /
- sizeof(uint64_t);
- for (I = 0; I < NC; I++)
+ unsigned NC = SrcData->NumCounters;
+ if (NC == 0 || (const char *)SrcCountersStart >= SrcNameStart)
+ return 1;
+ uint64_t *SrcCounters = SrcCountersStart + ((size_t)SrcData->CounterPtr -
+ Header->CountersDelta) /
+ sizeof(uint64_t);
+ if (SrcCounters < SrcCountersStart ||
+ (const char *)SrcCounters >= SrcNameStart ||
+ (const char *)(SrcCounters + NC) > SrcNameStart)
+ return 1;
+ for (unsigned I = 0; I < NC; I++)
DstCounters[I] += SrcCounters[I];
- /* Now merge value profile data. */
+ /* Now merge value profile data. */
if (!VPMergeHook)
continue;
- for (I = 0; I <= IPVK_Last; I++)
+ for (unsigned I = 0; I <= IPVK_Last; I++)
NVK += (SrcData->NumValueSites[I] != 0);
if (!NVK)
continue;
- VPMergeHook(SrcValueProfData, DstData);
- SrcValueProfData = (ValueProfData *)((char *)SrcValueProfData +
- SrcValueProfData->TotalSize);
+ if (SrcValueProfData >= ProfileData + ProfileSize)
+ return 1;
+ VPMergeHook((ValueProfData *)SrcValueProfData, DstData);
+ SrcValueProfData =
+ SrcValueProfData + ((ValueProfData *)SrcValueProfData)->TotalSize;
}
+
+ return 0;
}
diff --git a/compiler-rt/test/profile/Linux/corrupted-profile.c b/compiler-rt/test/profile/Linux/corrupted-profile.c
new file mode 100644
index 000000000000..4b8682bf8a7f
--- /dev/null
+++ b/compiler-rt/test/profile/Linux/corrupted-profile.c
@@ -0,0 +1,61 @@
+// RUN: rm -f %t.profraw
+// RUN: touch %t.profraw
+// RUN: %clang_profgen -o %t %s
+// RUN: %t %t.profraw
+// RUN: %t %t.profraw modifyfile
+// RUN: cp %t.profraw %t.profraw.old
+// RUN: %t %t.profraw 2>&1 | FileCheck %s
+// RUN:
diff %t.profraw %t.profraw.old
+// CHECK: Invalid profile data to merge
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+
+void __llvm_profile_set_file_object(FILE* File, int EnableMerge);
+
+void bail(const char* str) {
+ fprintf(stderr, "%s %s\n", str, strerror(errno));
+ exit(1);
+}
+
+int main(int argc, char** argv) {
+ if (argc == 3) {
+ int fd = open(argv[1], O_RDWR);
+ if (fd == -1)
+ bail("open");
+
+ struct stat st;
+ if (stat(argv[1], &st))
+ bail("stat");
+ uint64_t FileSize = st.st_size;
+
+ char* Buf = (char *) mmap(NULL, FileSize, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+ if (Buf == MAP_FAILED)
+ bail("mmap");
+
+ // We're trying to make the first CounterPtr invalid.
+ // 10 64-bit words as header.
+ // CounterPtr is the third 64-bit word field.
+ memset(&Buf[10 * 8 + 2 * 8], 0xAB, 8);
+
+ if (munmap(Buf, FileSize))
+ bail("munmap");
+
+ if (close(fd))
+ bail("close");
+ } else {
+ FILE* f = fopen(argv[1], "r+b");
+ if (!f)
+ bail("fopen");
+ __llvm_profile_set_file_object(f, 1);
+ }
+}
diff --git a/compiler-rt/test/profile/Linux/instrprof-merge-vp.c b/compiler-rt/test/profile/Linux/instrprof-merge-vp.c
index 8daed3352b22..e4ad7759d780 100644
--- a/compiler-rt/test/profile/Linux/instrprof-merge-vp.c
+++ b/compiler-rt/test/profile/Linux/instrprof-merge-vp.c
@@ -14,7 +14,7 @@
int __llvm_profile_runtime = 0;
int __llvm_profile_write_file();
void __llvm_profile_reset_counters(void);
-void __llvm_profile_merge_from_buffer(const char *, uint64_t);
+int __llvm_profile_merge_from_buffer(const char *, uint64_t);
void __llvm_profile_set_filename(const char *);
struct __llvm_profile_data;
struct ValueProfData;
diff --git a/compiler-rt/test/profile/instrprof-merge.c b/compiler-rt/test/profile/instrprof-merge.c
index de46510e3471..4c9e4b277b79 100644
--- a/compiler-rt/test/profile/instrprof-merge.c
+++ b/compiler-rt/test/profile/instrprof-merge.c
@@ -14,7 +14,7 @@ int __llvm_profile_runtime = 0;
uint64_t __llvm_profile_get_size_for_buffer(void);
int __llvm_profile_write_buffer(char *);
void __llvm_profile_reset_counters(void);
-void __llvm_profile_merge_from_buffer(const char *, uint64_t);
+int __llvm_profile_merge_from_buffer(const char *, uint64_t);
int dumpBuffer(const char *FileN, const char *Buffer, uint64_t Size) {
FILE *File = fopen(FileN, "w");
diff --git a/compiler-rt/test/profile/instrprof-without-libc.c b/compiler-rt/test/profile/instrprof-without-libc.c
index cd9fb5e1dd8e..d6df73614dde 100644
--- a/compiler-rt/test/profile/instrprof-without-libc.c
+++ b/compiler-rt/test/profile/instrprof-without-libc.c
@@ -19,7 +19,7 @@
int __llvm_profile_runtime = 0;
uint64_t __llvm_profile_get_size_for_buffer(void);
int __llvm_profile_write_buffer(char *);
-void __llvm_profile_merge_from_buffer(const char *, uint64_t Size);
+int __llvm_profile_merge_from_buffer(const char *, uint64_t Size);
int write_buffer(uint64_t, const char *);
int main(int argc, const char *argv[]) {
More information about the llvm-commits
mailing list