From 1857805719a0c32263f4356c3902c63d32db15c8 Mon Sep 17 00:00:00 2001 From: Ryan Mansfield Date: Fri, 31 Oct 2025 16:21:11 -0400 Subject: [PATCH 1/6] [yaml2obj][MachO] Fix crash from integer underflow with invalid cmdsize yaml2obj would crash when processing Mach-O load commands with cmdsize smaller than the actual structure size e.g. LC_SEGMENT_64 with cmdsize=56 instead of 72. The crash occurred due to integer underflow when calculating padding: cmdsize - BytesWritten wraps to a large value when negative, causing a massive allocation attempt. --- llvm/lib/ObjectYAML/MachOEmitter.cpp | 10 +++++- .../MachO/segment-cmdsize-too-small.yaml | 33 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 llvm/test/ObjectYAML/MachO/segment-cmdsize-too-small.yaml diff --git a/llvm/lib/ObjectYAML/MachOEmitter.cpp b/llvm/lib/ObjectYAML/MachOEmitter.cpp index 35d442e8e3437..a3555b211ab53 100644 --- a/llvm/lib/ObjectYAML/MachOEmitter.cpp +++ b/llvm/lib/ObjectYAML/MachOEmitter.cpp @@ -285,7 +285,15 @@ void MachOWriter::writeLoadCommands(raw_ostream &OS) { // Fill remaining bytes with 0. This will only get hit in partially // specified test cases. - auto BytesRemaining = LC.Data.load_command_data.cmdsize - BytesWritten; + // Prevent integer underflow if BytesWritten exceeds cmdsize. + if (BytesWritten > LC.Data.load_command_data.cmdsize) { + errs() << "warning: load command " << LC.Data.load_command_data.cmd + << " cmdsize too small (" << LC.Data.load_command_data.cmdsize + << " bytes) for actual size (" << BytesWritten << " bytes)\n"; + } + auto BytesRemaining = (BytesWritten < LC.Data.load_command_data.cmdsize) + ? LC.Data.load_command_data.cmdsize - BytesWritten + : 0; if (BytesRemaining > 0) { ZeroFillBytes(OS, BytesRemaining); } diff --git a/llvm/test/ObjectYAML/MachO/segment-cmdsize-too-small.yaml b/llvm/test/ObjectYAML/MachO/segment-cmdsize-too-small.yaml new file mode 100644 index 0000000000000..250a0631a5a9c --- /dev/null +++ b/llvm/test/ObjectYAML/MachO/segment-cmdsize-too-small.yaml @@ -0,0 +1,33 @@ +## Test that yaml2obj handles cmdsize that is too small for LC_SEGMENT_64 +## without crashing (due to integer underflow). + +# RUN: yaml2obj %s -o %t 2>&1 | FileCheck %s --check-prefix=WARNING +# RUN: not llvm-readobj --file-headers %t 2>&1 | FileCheck %s --check-prefix=MALFORMED + +# WARNING: warning: load command 25 cmdsize too small (56 bytes) for actual size (72 bytes) + +# MALFORMED: error: {{.*}}: truncated or malformed object (load command 0 LC_SEGMENT_64 cmdsize too small) + +--- !mach-o +FileHeader: + magic: 0xFEEDFACF + cputype: 0x01000007 + cpusubtype: 0x00000003 + filetype: 0x00000001 + ncmds: 1 + sizeofcmds: 56 + flags: 0x00002000 + reserved: 0x00000000 +LoadCommands: + - cmd: LC_SEGMENT_64 + cmdsize: 56 ## Should be 72 for LC_SEGMENT_64 + segname: '__TEXT' + vmaddr: 0x1000 + vmsize: 0x10 + fileoff: 0 + filesize: 0 + maxprot: 7 + initprot: 5 + nsects: 0 + flags: 0 +... From 356799ba47334c8c73e3c48d21f20e17a3f0d503 Mon Sep 17 00:00:00 2001 From: Ryan Mansfield Date: Mon, 3 Nov 2025 09:34:33 -0500 Subject: [PATCH 2/6] Address reviewer feedback. - Add getLoadCommandName() helper to map cmd values to names - Change to indexed loop to show load command index for disambiguation - Update warning format to match llvm-readobj style: Known commands: "load command 0 LC_SEGMENT_64 cmdsize too small" Unknown commands: "load command 0 (0xdeadbeef) cmdsize too small" - Use WithColor::warning() --- llvm/lib/ObjectYAML/MachOEmitter.cpp | 32 ++++++++++++++++--- ...small.yaml => load-cmdsize-too-small.yaml} | 6 ++-- 2 files changed, 30 insertions(+), 8 deletions(-) rename llvm/test/ObjectYAML/MachO/{segment-cmdsize-too-small.yaml => load-cmdsize-too-small.yaml} (77%) diff --git a/llvm/lib/ObjectYAML/MachOEmitter.cpp b/llvm/lib/ObjectYAML/MachOEmitter.cpp index a3555b211ab53..0fd2c588eaead 100644 --- a/llvm/lib/ObjectYAML/MachOEmitter.cpp +++ b/llvm/lib/ObjectYAML/MachOEmitter.cpp @@ -19,14 +19,26 @@ #include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/LEB128.h" -#include "llvm/Support/YAMLTraits.h" #include "llvm/Support/SystemZ/zOSSupport.h" +#include "llvm/Support/WithColor.h" +#include "llvm/Support/YAMLTraits.h" #include "llvm/Support/raw_ostream.h" using namespace llvm; namespace { +static const char *getLoadCommandName(uint32_t cmd) { + switch (cmd) { +#define HANDLE_LOAD_COMMAND(LCName, LCValue, LCStruct) \ + case MachO::LCName: \ + return #LCName; +#include "llvm/BinaryFormat/MachO.def" + default: + return nullptr; + } +} + class MachOWriter { public: MachOWriter(MachOYAML::Object &Obj) : Obj(Obj), fileStart(0) { @@ -244,7 +256,8 @@ void MachOWriter::ZeroToOffset(raw_ostream &OS, size_t Offset) { } void MachOWriter::writeLoadCommands(raw_ostream &OS) { - for (auto &LC : Obj.LoadCommands) { + for (size_t i = 0; i < Obj.LoadCommands.size(); ++i) { + auto &LC = Obj.LoadCommands[i]; size_t BytesWritten = 0; llvm::MachO::macho_load_command Data = LC.Data; @@ -287,9 +300,18 @@ void MachOWriter::writeLoadCommands(raw_ostream &OS) { // specified test cases. // Prevent integer underflow if BytesWritten exceeds cmdsize. if (BytesWritten > LC.Data.load_command_data.cmdsize) { - errs() << "warning: load command " << LC.Data.load_command_data.cmd - << " cmdsize too small (" << LC.Data.load_command_data.cmdsize - << " bytes) for actual size (" << BytesWritten << " bytes)\n"; + const char *name = getLoadCommandName(LC.Data.load_command_data.cmd); + if (name) + WithColor::warning() + << "load command " << i << " " << name << " cmdsize too small (" + << LC.Data.load_command_data.cmdsize << " bytes) for actual size (" + << BytesWritten << " bytes)\n"; + else + WithColor::warning() + << "load command " << i << " (0x" + << Twine::utohexstr(LC.Data.load_command_data.cmd) + << ") cmdsize too small (" << LC.Data.load_command_data.cmdsize + << " bytes) for actual size (" << BytesWritten << " bytes)\n"; } auto BytesRemaining = (BytesWritten < LC.Data.load_command_data.cmdsize) ? LC.Data.load_command_data.cmdsize - BytesWritten diff --git a/llvm/test/ObjectYAML/MachO/segment-cmdsize-too-small.yaml b/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml similarity index 77% rename from llvm/test/ObjectYAML/MachO/segment-cmdsize-too-small.yaml rename to llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml index 250a0631a5a9c..ef18a959b12ec 100644 --- a/llvm/test/ObjectYAML/MachO/segment-cmdsize-too-small.yaml +++ b/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml @@ -1,10 +1,10 @@ -## Test that yaml2obj handles cmdsize that is too small for LC_SEGMENT_64 -## without crashing (due to integer underflow). +## Test that yaml2obj handles load commands with cmdsize smaller than the +## actual structure size without crashing (due to integer underflow). # RUN: yaml2obj %s -o %t 2>&1 | FileCheck %s --check-prefix=WARNING # RUN: not llvm-readobj --file-headers %t 2>&1 | FileCheck %s --check-prefix=MALFORMED -# WARNING: warning: load command 25 cmdsize too small (56 bytes) for actual size (72 bytes) +# WARNING: warning: load command 0 LC_SEGMENT_64 cmdsize too small (56 bytes) for actual size (72 bytes) # MALFORMED: error: {{.*}}: truncated or malformed object (load command 0 LC_SEGMENT_64 cmdsize too small) From e2fc33872b77c07453c40cb5308a762a4a3b9162 Mon Sep 17 00:00:00 2001 From: Ryan Mansfield Date: Tue, 4 Nov 2025 07:24:20 -0500 Subject: [PATCH 3/6] Address reviewer feedback. - Refactor warning message - Add testcase for unknown load command --- llvm/lib/ObjectYAML/MachOEmitter.cpp | 22 +++++++-------- .../MachO/load-cmdsize-too-small.yaml | 28 +++++++++++++++++-- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/llvm/lib/ObjectYAML/MachOEmitter.cpp b/llvm/lib/ObjectYAML/MachOEmitter.cpp index 0fd2c588eaead..63ff8729450ca 100644 --- a/llvm/lib/ObjectYAML/MachOEmitter.cpp +++ b/llvm/lib/ObjectYAML/MachOEmitter.cpp @@ -300,18 +300,18 @@ void MachOWriter::writeLoadCommands(raw_ostream &OS) { // specified test cases. // Prevent integer underflow if BytesWritten exceeds cmdsize. if (BytesWritten > LC.Data.load_command_data.cmdsize) { - const char *name = getLoadCommandName(LC.Data.load_command_data.cmd); - if (name) - WithColor::warning() - << "load command " << i << " " << name << " cmdsize too small (" - << LC.Data.load_command_data.cmdsize << " bytes) for actual size (" - << BytesWritten << " bytes)\n"; + std::string Name; + const char *NameCStr = getLoadCommandName(LC.Data.load_command_data.cmd); + if (NameCStr) + Name = NameCStr; else - WithColor::warning() - << "load command " << i << " (0x" - << Twine::utohexstr(LC.Data.load_command_data.cmd) - << ") cmdsize too small (" << LC.Data.load_command_data.cmdsize - << " bytes) for actual size (" << BytesWritten << " bytes)\n"; + Name = "(0x" + Twine::utohexstr(LC.Data.load_command_data.cmd).str() + ")"; + + WithColor::warning() << "load command " << i << " " << Name + << " cmdsize too small (" + << LC.Data.load_command_data.cmdsize + << " bytes) for actual size (" << BytesWritten + << " bytes)\n"; } auto BytesRemaining = (BytesWritten < LC.Data.load_command_data.cmdsize) ? LC.Data.load_command_data.cmdsize - BytesWritten diff --git a/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml b/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml index ef18a959b12ec..ec460a6bc618a 100644 --- a/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml +++ b/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml @@ -1,13 +1,18 @@ ## Test that yaml2obj handles load commands with cmdsize smaller than the ## actual structure size without crashing (due to integer underflow). -# RUN: yaml2obj %s -o %t 2>&1 | FileCheck %s --check-prefix=WARNING -# RUN: not llvm-readobj --file-headers %t 2>&1 | FileCheck %s --check-prefix=MALFORMED +# RUN: yaml2obj %s --docnum=1 -o %t1 2>&1 | FileCheck %s --check-prefix=WARNING-KNOWN +# RUN: not llvm-readobj --file-headers %t1 2>&1 | FileCheck %s --check-prefix=MALFORMED -# WARNING: warning: load command 0 LC_SEGMENT_64 cmdsize too small (56 bytes) for actual size (72 bytes) +# RUN: yaml2obj %s --docnum=2 -o %t2 2>&1 | FileCheck %s --check-prefix=WARNING-UNKNOWN + +# WARNING-KNOWN: warning: load command 0 LC_SEGMENT_64 cmdsize too small (56 bytes) for actual size (72 bytes) # MALFORMED: error: {{.*}}: truncated or malformed object (load command 0 LC_SEGMENT_64 cmdsize too small) +# WARNING-UNKNOWN: warning: load command 0 (0xdeadbeef) cmdsize too small (8 bytes) for actual size (20 bytes) + +## Test with a known load command (LC_SEGMENT_64) --- !mach-o FileHeader: magic: 0xFEEDFACF @@ -31,3 +36,20 @@ LoadCommands: nsects: 0 flags: 0 ... + +## Test with an unknown load command value +--- !mach-o +FileHeader: + magic: 0xFEEDFACF + cputype: 0x01000007 + cpusubtype: 0x00000003 + filetype: 0x00000001 + ncmds: 1 + sizeofcmds: 20 + flags: 0x00002000 + reserved: 0x00000000 +LoadCommands: + - cmd: 0xDEADBEEF + cmdsize: 8 + PayloadBytes: [0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C] +... From d2f1eee485e18fcac1e75e2f4f41a12982a4bcc6 Mon Sep 17 00:00:00 2001 From: Ryan Mansfield Date: Tue, 4 Nov 2025 07:41:55 -0500 Subject: [PATCH 4/6] Fix formatting. --- llvm/lib/ObjectYAML/MachOEmitter.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llvm/lib/ObjectYAML/MachOEmitter.cpp b/llvm/lib/ObjectYAML/MachOEmitter.cpp index 63ff8729450ca..9367e6b17383c 100644 --- a/llvm/lib/ObjectYAML/MachOEmitter.cpp +++ b/llvm/lib/ObjectYAML/MachOEmitter.cpp @@ -305,7 +305,8 @@ void MachOWriter::writeLoadCommands(raw_ostream &OS) { if (NameCStr) Name = NameCStr; else - Name = "(0x" + Twine::utohexstr(LC.Data.load_command_data.cmd).str() + ")"; + Name = + "(0x" + Twine::utohexstr(LC.Data.load_command_data.cmd).str() + ")"; WithColor::warning() << "load command " << i << " " << Name << " cmdsize too small (" From 5c1704953366fc2127b71e317c9a4bb6bb2bb618 Mon Sep 17 00:00:00 2001 From: Ryan Mansfield Date: Tue, 4 Nov 2025 08:00:30 -0500 Subject: [PATCH 5/6] Address reviewer feedback. - Do .str() around the full string concatenation - Reorganize test --- llvm/lib/ObjectYAML/MachOEmitter.cpp | 4 ++-- .../ObjectYAML/MachO/load-cmdsize-too-small.yaml | 12 +++++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/llvm/lib/ObjectYAML/MachOEmitter.cpp b/llvm/lib/ObjectYAML/MachOEmitter.cpp index 9367e6b17383c..46c91811d0a67 100644 --- a/llvm/lib/ObjectYAML/MachOEmitter.cpp +++ b/llvm/lib/ObjectYAML/MachOEmitter.cpp @@ -305,8 +305,8 @@ void MachOWriter::writeLoadCommands(raw_ostream &OS) { if (NameCStr) Name = NameCStr; else - Name = - "(0x" + Twine::utohexstr(LC.Data.load_command_data.cmd).str() + ")"; + Name = ("(0x" + Twine::utohexstr(LC.Data.load_command_data.cmd) + ")") + .str(); WithColor::warning() << "load command " << i << " " << Name << " cmdsize too small (" diff --git a/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml b/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml index ec460a6bc618a..fbb1ceb52343e 100644 --- a/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml +++ b/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml @@ -1,18 +1,13 @@ ## Test that yaml2obj handles load commands with cmdsize smaller than the ## actual structure size without crashing (due to integer underflow). +## Test with a known load command (LC_SEGMENT_64). # RUN: yaml2obj %s --docnum=1 -o %t1 2>&1 | FileCheck %s --check-prefix=WARNING-KNOWN # RUN: not llvm-readobj --file-headers %t1 2>&1 | FileCheck %s --check-prefix=MALFORMED -# RUN: yaml2obj %s --docnum=2 -o %t2 2>&1 | FileCheck %s --check-prefix=WARNING-UNKNOWN - # WARNING-KNOWN: warning: load command 0 LC_SEGMENT_64 cmdsize too small (56 bytes) for actual size (72 bytes) # MALFORMED: error: {{.*}}: truncated or malformed object (load command 0 LC_SEGMENT_64 cmdsize too small) - -# WARNING-UNKNOWN: warning: load command 0 (0xdeadbeef) cmdsize too small (8 bytes) for actual size (20 bytes) - -## Test with a known load command (LC_SEGMENT_64) --- !mach-o FileHeader: magic: 0xFEEDFACF @@ -37,7 +32,10 @@ LoadCommands: flags: 0 ... -## Test with an unknown load command value +## Test with an unknown load command value. +# RUN: yaml2obj %s --docnum=2 -o %t2 2>&1 | FileCheck %s --check-prefix=WARNING-UNKNOWN + +# WARNING-UNKNOWN: warning: load command 0 (0xdeadbeef) cmdsize too small (8 bytes) for actual size (20 bytes) --- !mach-o FileHeader: magic: 0xFEEDFACF From d576e7628849bf5285cb28b6167c84c10fee5d59 Mon Sep 17 00:00:00 2001 From: Ryan Mansfield Date: Wed, 5 Nov 2025 08:28:55 -0500 Subject: [PATCH 6/6] Add empty lines before YAML blocks. --- llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml b/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml index fbb1ceb52343e..ef11711e0487e 100644 --- a/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml +++ b/llvm/test/ObjectYAML/MachO/load-cmdsize-too-small.yaml @@ -8,6 +8,7 @@ # WARNING-KNOWN: warning: load command 0 LC_SEGMENT_64 cmdsize too small (56 bytes) for actual size (72 bytes) # MALFORMED: error: {{.*}}: truncated or malformed object (load command 0 LC_SEGMENT_64 cmdsize too small) + --- !mach-o FileHeader: magic: 0xFEEDFACF @@ -36,6 +37,7 @@ LoadCommands: # RUN: yaml2obj %s --docnum=2 -o %t2 2>&1 | FileCheck %s --check-prefix=WARNING-UNKNOWN # WARNING-UNKNOWN: warning: load command 0 (0xdeadbeef) cmdsize too small (8 bytes) for actual size (20 bytes) + --- !mach-o FileHeader: magic: 0xFEEDFACF