[ELF] - Alternative fix to prevent possible crash on large output.

[grimar]

Summary

This is alternative solution for D25279.

Initial problem was that because of overflow in calculations wierd thinks could happen.
Most heavy is a crash in writeTo because of overflow in FileSize calculation.

This patch does not introduce new class and uses 2 check-methods.

[ruiu]

The new functions seem too complicated. There are a lot of ways to crash the linker by giving incorrect input. We deliberately ignore some cases such as relocations having too large values. Why do you want to fix this specifically? This is not even the only place where overflow can happen. I think we should leave it as is.

[ruiu]

Here's my proposal to detect offset overflow without checking integer overflow everywhere.

• Use uint64_t for Off instead of uintX_t everywhere
• If the final Off is greater than sizeof(uintX_t), reject it. This check should suffice for detecting any overflow on 32-bit targets.
• Reject insanely large sections and alignments such as >2^40 when reading a file. This suffices to prevent any overflow on 64-bit targets.

That being said, I doubt this is a top priority thing to do.

[grimar]

In D25467#567497, @ruiu wrote:

The new functions seem too complicated. There are a lot of ways to crash the linker by giving incorrect input. We deliberately ignore some cases such as relocations having too large values. Why do you want to fix this specifically? This is not even the only place where overflow can happen. I think we should leave it as is.

I am not fixing this specifically, I am trying to fix every crash or hang I see during AFL runs. There is no high or low priority for me, each crash is a problem I think.

[grimar]

Depricated.

ELF/Writer.cpp

if (needsPtLoad(Sec)) {
       uintX_t TVA = VA + ThreadBssOffset;
       TVA = alignTo(TVA, Alignment);
       Sec->setVA(TVA);
       ThreadBssOffset = TVA - VA + Sec->getSize();
     }
   }
 }
 
+template <class T1, class T2>
+static T1 checkedAdd(T1 Val1, T2 Val2, StringRef Msg) {
+  const T1 Max = std::numeric_limits<T1>::max();
+  if (Val2 > Max || Val1 > Max - Val2)
+    fatal(Msg);
+  return Val1 + Val2;
+}
+
+template <class T>
+static T checkedAlignTo(T Value, uint64_t Align, uint64_t Skew, StringRef Msg) {
+  const T Ret = alignTo(Value, Align, Skew);
+  if (Ret < Value)
+    fatal(Msg);
+  return Ret;
+}
+
 // Adjusts the file alignment for a given output section and returns
 // its new file offset. The file offset must be the same with its
 // virtual address (modulo the page size) so that the loader can load
 // executables without any address adjustment.
 template <class ELFT, class uintX_t>
 static uintX_t getFileAlignment(uintX_t Off, OutputSectionBase<ELFT> *Sec) {
   uintX_t Alignment = Sec->getAlignment();
   if (Sec->PageAlign)
     Alignment = std::max<uintX_t>(Alignment, Config->MaxPageSize);
-  Off = alignTo(Off, Alignment);
+  Off = checkedAlignTo(Off, Alignment, 0,
+                       "getFileAlignment failed: operation overflow");
 
   OutputSectionBase<ELFT> *First = Sec->FirstInPtLoad;
   // If the section is not in a PT_LOAD, we have no other constraint.
   if (!First)
     return Off;
 
   // If two sections share the same PT_LOAD the file offset is calculated using
   // this formula: Off2 = Off1 + (VA2 - VA1).
   if (Sec == First)
-    return alignTo(Off, Target->MaxPageSize, Sec->getVA());
-  return First->getFileOffset() + Sec->getVA() - First->getVA();
+    return checkedAlignTo(Off, Target->MaxPageSize, Sec->getVA(),
+                          "getFileAlignment failed: operation overflow");
+  return checkedAdd(First->getFileOffset(), Sec->getVA() - First->getVA(),
+                    "getFileAlignment failed: operation overflow");
 }
 
 template <class ELFT, class uintX_t>
 void setOffset(OutputSectionBase<ELFT> *Sec, uintX_t &Off) {
   if (Sec->getType() == SHT_NOBITS) {
     Sec->setFileOffset(Off);
     return;
   }
 
   Off = getFileAlignment<ELFT>(Off, Sec);
   Sec->setFileOffset(Off);
-  Off += Sec->getSize();
+  Off = checkedAdd(Off, Sec->getSize(), "setOffset failed: operation overflow");
 }
 
 template <class ELFT> void Writer<ELFT>::assignFileOffsetsBinary() {
   uintX_t Off = 0;
   for (OutputSectionBase<ELFT> *Sec : OutputSections)
     if (Sec->getFlags() & SHF_ALLOC)
       setOffset(Sec, Off);
   FileSize = alignTo(Off, sizeof(uintX_t));

test/ELF/invalid/too-large-output-i386.s

+# REQUIRES: x86
+
+## too-large-output-i386.elf contains section with address align.
+## of 0xFFFFFFFF. Total file size calculation overflows because of that.
+# RUN: not ld.lld -shared %S/Inputs/too-large-output-i386.elf -o %t 2>&1 \
+# RUN:  | FileCheck %s
+# CHECK: getFileAlignment failed: operation overflow