diff --git a/compiler-rt/lib/profile/InstrProfiling.h b/compiler-rt/lib/profile/InstrProfiling.h --- a/compiler-rt/lib/profile/InstrProfiling.h +++ b/compiler-rt/lib/profile/InstrProfiling.h @@ -101,13 +101,13 @@ /*! * \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 --- a/compiler-rt/lib/profile/InstrProfilingFile.c +++ b/compiler-rt/lib/profile/InstrProfilingFile.c @@ -261,9 +261,13 @@ 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 --- a/compiler-rt/lib/profile/InstrProfilingMerge.c +++ b/compiler-rt/lib/profile/InstrProfilingMerge.c @@ -82,13 +82,13 @@ } 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 @@ 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 --- /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 +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +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 --- 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 --- a/compiler-rt/test/profile/instrprof-merge.c +++ b/compiler-rt/test/profile/instrprof-merge.c @@ -14,7 +14,7 @@ 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 --- 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[]) {