[MachO] Detect overflow in section offset. #98685
Conversation
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/pr-subscribers-mc Author: Eli Friedman (efriedma-quic) ChangesThe section offset field is only 32 bits; if the computed section offset is larger, make sure we don't emit a corrupt object file. Full diff: https://github.com/llvm/llvm-project/pull/98685.diff 2 Files Affected:
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
|
| @@ -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())); | |||
There was a problem hiding this comment.
Mind adding another one for RelocationStart? Looks like it might have the same problem.
There was a problem hiding this comment.
We can call Asm.getContext().reportError instead to not trigger a crash.
There was a problem hiding this comment.
Perhaps let the caller (MachObjectWriter::writeObject) check this and early return.
There was a problem hiding this comment.
The point of the early return would be to avoid duplicate error messages? That makes sense, I guess.
There was a problem hiding this comment.
Tried rearranging as suggested.
llvm/lib/MC/MachObjectWriter.cpp
Outdated
| if (!cast<MCSectionMachO>(Sec).isVirtualSection() && | ||
| !isUInt<32>(SectionStart)) { | ||
| Asm.getContext().reportError( | ||
| SMLoc(), "Cannot encode offset of section; object file too large"); |
There was a problem hiding this comment.
reportError messages are generally not capitalized per https://llvm.org/docs/CodingStandards.html#error-and-warning-messages
| @@ -0,0 +1,9 @@ | |||
| // 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 | |||
There was a problem hiding this comment.
check the error: prefix of the diagnostic
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.
Summary: 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. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251010
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.