[PATCH] D97239: [profile] Fix buffer overrun when parsing %c in filename string

[Vedant Kumar via Phabricator via llvm-commits]

vsk updated this revision to Diff 325612.
vsk added a comment.

Clean up the test by removing the call to set_dumped.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97239/new/

https://reviews.llvm.org/D97239

Files:
  compiler-rt/lib/profile/InstrProfilingFile.c
  compiler-rt/test/profile/ContinuousSyncMode/get-filename.c


Index: compiler-rt/test/profile/ContinuousSyncMode/get-filename.c
===================================================================
--- /dev/null
+++ compiler-rt/test/profile/ContinuousSyncMode/get-filename.c
@@ -0,0 +1,32 @@
+// REQUIRES: darwin
+
+// RUN: %clang_pgogen -o %t.exe %s
+// RUN: env LLVM_PROFILE_FILE="%c%t.profraw" %run %t.exe %t.profraw
+// RUN: env LLVM_PROFILE_FILE="%t%c.profraw" %run %t.exe %t.profraw
+// RUN: env LLVM_PROFILE_FILE="%t.profraw%c" %run %t.exe %t.profraw
+
+#include <string.h>
+#include <stdio.h>
+
+extern int __llvm_profile_is_continuous_mode_enabled(void);
+extern const char *__llvm_profile_get_filename();
+extern void __llvm_profile_set_dumped(void);
+
+int main(int argc, char **argv) {
+  if (!__llvm_profile_is_continuous_mode_enabled())
+    return 1;
+
+  // Check that the filename is "%t.profraw", followed by a null terminator.
+  size_t n = strlen(argv[1]) + 1;
+  const char *Filename = __llvm_profile_get_filename();
+
+  for (int i = 0; i < n; ++i) {
+    if (Filename[i] != argv[1][i]) {
+      printf("Difference at: %d, Got: %c, Expected: %c\n", i, Filename[i], argv[1][i]);
+      printf("Got: %s\n", Filename);
+      printf("Expected: %s\n", argv[1]);
+      return 1;
+    }
+  }
+  return 0;
+}
Index: compiler-rt/lib/profile/InstrProfilingFile.c
===================================================================
--- compiler-rt/lib/profile/InstrProfilingFile.c
+++ compiler-rt/lib/profile/InstrProfilingFile.c
@@ -8,6 +8,7 @@
 
 #if !defined(__Fuchsia__)
 
+#include <assert.h>
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -699,6 +700,13 @@
   return 0;
 }
 
+/* Return Str[Idx] after asserting that Idx does not go past the terminator. */
+static char getChar(const char *Str, int Idx, int Strlen) {
+  assert(Idx <= Strlen && "Indexing past string null terminator");
+  (void)Strlen;
+  return Str[Idx];
+}
+
 /* Parses the pattern string \p FilenamePat and stores the result to
  * lprofcurFilename structure. */
 static int parseFilenamePattern(const char *FilenamePat,
@@ -707,6 +715,7 @@
   char *PidChars = &lprofCurFilename.PidChars[0];
   char *Hostname = &lprofCurFilename.Hostname[0];
   int MergingEnabled = 0;
+  int FilenamePatLen = strlen(FilenamePat);
 
   /* Clean up cached prefix and filename.  */
   if (lprofCurFilename.ProfilePathPrefix)
@@ -725,9 +734,10 @@
     lprofCurFilename.OwnsFilenamePat = 1;
   }
   /* Check the filename for "%p", which indicates a pid-substitution. */
-  for (I = 0; FilenamePat[I]; ++I)
+  for (I = 0; getChar(FilenamePat, I, FilenamePatLen); ++I) {
     if (FilenamePat[I] == '%') {
-      if (FilenamePat[++I] == 'p') {
+      ++I; /* Advance to the next character. */
+      if (getChar(FilenamePat, I, FilenamePatLen) == 'p') {
         if (!NumPids++) {
           if (snprintf(PidChars, MAX_PID_SIZE, "%ld", (long)getpid()) <= 0) {
             PROF_WARN("Unable to get pid for filename pattern %s. Using the "
@@ -761,7 +771,6 @@
 
         __llvm_profile_set_page_size(getpagesize());
         __llvm_profile_enable_continuous_mode();
-        I++; /* advance to 'c' */
       } else {
         unsigned MergePoolSize = getMergePoolSize(FilenamePat, &I);
         if (!MergePoolSize)
@@ -775,6 +784,7 @@
         lprofCurFilename.MergePoolSize = MergePoolSize;
       }
     }
+  }
 
   lprofCurFilename.NumPids = NumPids;
   lprofCurFilename.NumHosts = NumHosts;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97239.325612.patch
Type: text/x-patch
Size: 3412 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210223/413c3414/attachment.bin>