[llvm] [MachO] Detect overflow in section offset. (PR #98685)

Eli Friedman via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 16 14:50:21 PDT 2024


https://github.com/efriedma-quic updated https://github.com/llvm/llvm-project/pull/98685

>From 8d428dd919d2e081e25c102c571a7ca4790b81f5 Mon Sep 17 00:00:00 2001
From: Eli Friedman <efriedma at quicinc.com>
Date: Fri, 12 Jul 2024 12:18:25 -0700
Subject: [PATCH 1/4] [MachO] Detect overflow in section offset.

The section offset field is only 32 bits; if the computed section offset
is larger, make sure we don't emit a corrupt object file.
---
 llvm/lib/MC/MachObjectWriter.cpp             | 2 ++
 llvm/test/MC/MachO/section-offset-overflow.s | 9 +++++++++
 2 files changed, 11 insertions(+)
 create mode 100644 llvm/test/MC/MachO/section-offset-overflow.s

diff --git a/llvm/lib/MC/MachObjectWriter.cpp b/llvm/lib/MC/MachObjectWriter.cpp
index 53eed0092a5b4..f890ecc1d20ee 100644
--- a/llvm/lib/MC/MachObjectWriter.cpp
+++ b/llvm/lib/MC/MachObjectWriter.cpp
@@ -277,6 +277,8 @@ void MachObjectWriter::writeSection(const MCAssembler &Asm,
     W.write<uint32_t>(VMAddr);      // address
     W.write<uint32_t>(SectionSize); // size
   }
+  if (!isUInt<32>(FileOffset))
+    report_fatal_error("Cannot encode offset of section");
   W.write<uint32_t>(FileOffset);
 
   W.write<uint32_t>(Log2(Section.getAlign()));
diff --git a/llvm/test/MC/MachO/section-offset-overflow.s b/llvm/test/MC/MachO/section-offset-overflow.s
new file mode 100644
index 0000000000000..51fc90c2e3479
--- /dev/null
+++ b/llvm/test/MC/MachO/section-offset-overflow.s
@@ -0,0 +1,9 @@
+// RUN: not --crash llvm-mc -triple x86_64-apple-macosx -filetype=obj -o /dev/null %s 2>&1 | FileCheck  %s
+
+// CHECK: Cannot encode offset of section
+
+        .data
+        .long 1
+        .zero 0x100000000
+        .const
+        .long 1

>From a72cfa1dce6b2dbf7718943ec321ae7eafe678f8 Mon Sep 17 00:00:00 2001
From: Eli Friedman <efriedma at quicinc.com>
Date: Fri, 12 Jul 2024 13:04:29 -0700
Subject: [PATCH 2/4] fixup! Also check offset of relocations.

---
 llvm/lib/MC/MachObjectWriter.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/llvm/lib/MC/MachObjectWriter.cpp b/llvm/lib/MC/MachObjectWriter.cpp
index f890ecc1d20ee..4418f68050cd5 100644
--- a/llvm/lib/MC/MachObjectWriter.cpp
+++ b/llvm/lib/MC/MachObjectWriter.cpp
@@ -282,6 +282,8 @@ void MachObjectWriter::writeSection(const MCAssembler &Asm,
   W.write<uint32_t>(FileOffset);
 
   W.write<uint32_t>(Log2(Section.getAlign()));
+  if (NumRelocations && !isUInt<32>(RelocationsStart))
+    report_fatal_error("Cannot encode offset of relocations");
   W.write<uint32_t>(NumRelocations ? RelocationsStart : 0);
   W.write<uint32_t>(NumRelocations);
   W.write<uint32_t>(Flags);

>From a6977631808af3dd97274d233ca0281cce40f24b Mon Sep 17 00:00:00 2001
From: Eli Friedman <efriedma at quicinc.com>
Date: Fri, 12 Jul 2024 15:42:59 -0700
Subject: [PATCH 3/4] fixup! Improve error handling.

---
 llvm/lib/MC/MachObjectWriter.cpp             | 22 +++++++++++++++-----
 llvm/test/MC/MachO/section-offset-overflow.s |  2 +-
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/llvm/lib/MC/MachObjectWriter.cpp b/llvm/lib/MC/MachObjectWriter.cpp
index 4418f68050cd5..dfe206e934e1c 100644
--- a/llvm/lib/MC/MachObjectWriter.cpp
+++ b/llvm/lib/MC/MachObjectWriter.cpp
@@ -277,13 +277,12 @@ void MachObjectWriter::writeSection(const MCAssembler &Asm,
     W.write<uint32_t>(VMAddr);      // address
     W.write<uint32_t>(SectionSize); // size
   }
-  if (!isUInt<32>(FileOffset))
-    report_fatal_error("Cannot encode offset of section");
+  assert(isUInt<32>(FileOffset) && "Cannot encode offset of section");
   W.write<uint32_t>(FileOffset);
 
   W.write<uint32_t>(Log2(Section.getAlign()));
-  if (NumRelocations && !isUInt<32>(RelocationsStart))
-    report_fatal_error("Cannot encode offset of relocations");
+  assert((!NumRelocations || isUInt<32>(RelocationsStart)) &&
+         "Cannot encode offset of relocations");
   W.write<uint32_t>(NumRelocations ? RelocationsStart : 0);
   W.write<uint32_t>(NumRelocations);
   W.write<uint32_t>(Flags);
@@ -779,6 +778,7 @@ void MachObjectWriter::populateAddrSigSection(MCAssembler &Asm) {
 
 uint64_t MachObjectWriter::writeObject(MCAssembler &Asm) {
   uint64_t StartOffset = W.OS.tell();
+  auto NumBytesWritten = [&] { return W.OS.tell() - StartOffset; };
 
   populateAddrSigSection(Asm);
 
@@ -908,6 +908,18 @@ uint64_t MachObjectWriter::writeObject(MCAssembler &Asm) {
     unsigned Flags = Sec.getTypeAndAttributes();
     if (Sec.hasInstructions())
       Flags |= MachO::S_ATTR_SOME_INSTRUCTIONS;
+    if (!cast<MCSectionMachO>(Sec).isVirtualSection() &&
+        !isUInt<32>(SectionStart)) {
+      Asm.getContext().reportError(
+          SMLoc(), "Cannot encode offset of section; object file too large");
+      return NumBytesWritten();
+    }
+    if (NumRelocs && !isUInt<32>(RelocTableEnd)) {
+      Asm.getContext().reportError(
+          SMLoc(),
+          "Cannot encode offset of relocations; object file too large");
+      return NumBytesWritten();
+    }
     writeSection(Asm, Sec, getSectionAddress(&Sec), SectionStart, Flags,
                  RelocTableEnd, NumRelocs);
     RelocTableEnd += NumRelocs * sizeof(MachO::any_relocation_info);
@@ -1092,7 +1104,7 @@ uint64_t MachObjectWriter::writeObject(MCAssembler &Asm) {
     StringTable.write(W.OS);
   }
 
-  return W.OS.tell() - StartOffset;
+  return NumBytesWritten();
 }
 
 std::unique_ptr<MCObjectWriter>
diff --git a/llvm/test/MC/MachO/section-offset-overflow.s b/llvm/test/MC/MachO/section-offset-overflow.s
index 51fc90c2e3479..56a056c882e48 100644
--- a/llvm/test/MC/MachO/section-offset-overflow.s
+++ b/llvm/test/MC/MachO/section-offset-overflow.s
@@ -1,4 +1,4 @@
-// RUN: not --crash llvm-mc -triple x86_64-apple-macosx -filetype=obj -o /dev/null %s 2>&1 | FileCheck  %s
+// RUN: not llvm-mc -triple x86_64-apple-macosx -filetype=obj -o /dev/null %s 2>&1 | FileCheck  %s
 
 // CHECK: Cannot encode offset of section
 

>From 0b07c9b8ae980ca2c0dd4717edaba39a6a23078e Mon Sep 17 00:00:00 2001
From: Eli Friedman <efriedma at quicinc.com>
Date: Tue, 16 Jul 2024 14:49:53 -0700
Subject: [PATCH 4/4] fixup! address review comments

---
 llvm/lib/MC/MachObjectWriter.cpp             | 4 ++--
 llvm/test/MC/MachO/section-offset-overflow.s | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/MC/MachObjectWriter.cpp b/llvm/lib/MC/MachObjectWriter.cpp
index dfe206e934e1c..e58e095252d05 100644
--- a/llvm/lib/MC/MachObjectWriter.cpp
+++ b/llvm/lib/MC/MachObjectWriter.cpp
@@ -911,13 +911,13 @@ uint64_t MachObjectWriter::writeObject(MCAssembler &Asm) {
     if (!cast<MCSectionMachO>(Sec).isVirtualSection() &&
         !isUInt<32>(SectionStart)) {
       Asm.getContext().reportError(
-          SMLoc(), "Cannot encode offset of section; object file too large");
+          SMLoc(), "cannot encode offset of section; object file too large");
       return NumBytesWritten();
     }
     if (NumRelocs && !isUInt<32>(RelocTableEnd)) {
       Asm.getContext().reportError(
           SMLoc(),
-          "Cannot encode offset of relocations; object file too large");
+          "cannot encode offset of relocations; object file too large");
       return NumBytesWritten();
     }
     writeSection(Asm, Sec, getSectionAddress(&Sec), SectionStart, Flags,
diff --git a/llvm/test/MC/MachO/section-offset-overflow.s b/llvm/test/MC/MachO/section-offset-overflow.s
index 56a056c882e48..f652cdb9f7e5c 100644
--- a/llvm/test/MC/MachO/section-offset-overflow.s
+++ b/llvm/test/MC/MachO/section-offset-overflow.s
@@ -1,6 +1,6 @@
-// RUN: not llvm-mc -triple x86_64-apple-macosx -filetype=obj -o /dev/null %s 2>&1 | FileCheck  %s
+// RUN: not llvm-mc -triple x86_64-apple-macosx -filetype=obj -o /dev/null %s 2>&1 | FileCheck %s
 
-// CHECK: Cannot encode offset of section
+// CHECK: error: cannot encode offset of section
 
         .data
         .long 1



More information about the llvm-commits mailing list