[PATCH] D126786: [Object][COFF] Add table ptr checks but don't hard-error

[Alvin Wong via Phabricator via llvm-commits]

alvinhochun created this revision.
Herald added subscribers: mstorsjo, hiraditya.
Herald added a project: All.
alvinhochun published this revision for review.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

When loading split debug files for PE/COFF executables (produced with
`objcopy --only-keep-debug`), the tables or directories in such files
may not exist. COFFObjectFile shall check the pointers to make sure they
are not out of bounds. One such check already exists for
initImportTablePtr. However, this check forces an error and prevents
the object file from being loaded, which means the file could not be
used as debug info in LLDB. To fix this, it is sufficient to just hide
the error and pretend the import table does not exist for the object
file.

In the same way, other functions which load the pointers for other
tables should also contain similar pointer checks, but none of these
functions actually have the check in place. This commit adds these
missing checks.

Fixes https://github.com/mstorsjo/llvm-mingw/issues/284


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126786

Files:
  llvm/lib/Object/COFFObjectFile.cpp


Index: llvm/lib/Object/COFFObjectFile.cpp
===================================================================
--- llvm/lib/Object/COFFObjectFile.cpp
+++ llvm/lib/Object/COFFObjectFile.cpp
@@ -579,8 +579,11 @@
   uintptr_t IntPtr = 0;
   if (Error E = getRvaPtr(ImportTableRva, IntPtr, "import table"))
     return E;
-  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size))
-    return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size)) {
+    consumeError(std::move(E));
+    return Error::success();
+  }
+
   ImportDirectory = reinterpret_cast<
       const coff_import_directory_table_entry *>(IntPtr);
   return Error::success();
@@ -602,6 +605,11 @@
   uintptr_t IntPtr = 0;
   if (Error E = getRvaPtr(RVA, IntPtr, "delay import table"))
     return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size)) { // !
+    consumeError(std::move(E));
+    return Error::success();
+  }
+
   DelayImportDirectory = reinterpret_cast<
       const delay_import_directory_table_entry *>(IntPtr);
   return Error::success();
@@ -623,6 +631,11 @@
   uintptr_t IntPtr = 0;
   if (Error E = getRvaPtr(ExportTableRva, IntPtr, "export table"))
     return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size)) { // !
+    consumeError(std::move(E));
+    return Error::success();
+  }
+
   ExportDirectory =
       reinterpret_cast<const export_directory_table_entry *>(IntPtr);
   return Error::success();
@@ -640,6 +653,11 @@
   if (Error E = getRvaPtr(DataEntry->RelativeVirtualAddress, IntPtr,
                           "base reloc table"))
     return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size)) { // !
+    consumeError(std::move(E));
+    return Error::success();
+  }
+
   BaseRelocHeader = reinterpret_cast<const coff_base_reloc_block_header *>(
       IntPtr);
   BaseRelocEnd = reinterpret_cast<coff_base_reloc_block_header *>(
@@ -668,6 +686,11 @@
   if (Error E = getRvaPtr(DataEntry->RelativeVirtualAddress, IntPtr,
                           "debug directory"))
     return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size)) { // !
+    consumeError(std::move(E));
+    return Error::success();
+  }
+
   DebugDirectoryBegin = reinterpret_cast<const debug_directory *>(IntPtr);
   DebugDirectoryEnd = reinterpret_cast<const debug_directory *>(
       IntPtr + DataEntry->Size);
@@ -700,6 +723,10 @@
   if (Error E =
           getRvaPtr(DataEntry->RelativeVirtualAddress, IntPtr, "TLS directory"))
     return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size)) {
+    consumeError(std::move(E));
+    return Error::success();
+  }
 
   if (is64())
     TLSDirectory64 = reinterpret_cast<const coff_tls_directory64 *>(IntPtr);
@@ -722,6 +749,10 @@
   if (Error E = getRvaPtr(DataEntry->RelativeVirtualAddress, IntPtr,
                           "load config table"))
     return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size)) { // !
+    consumeError(std::move(E));
+    return Error::success();
+  }
 
   LoadConfig = (const void *)IntPtr;
   return Error::success();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126786.433415.patch
Type: text/x-patch
Size: 3073 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220601/b5a17138/attachment.bin>