[Lldb-commits] [lldb] 20a2329 - [lldb][Mach-O] Fix load-command loops spinning on cmdsize = 0 (#205134)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Aug 4 00:56:22 PDT 2026
Author: Yao Qi
Date: 2026-08-04T08:56:17+01:00
New Revision: 20a2329148626aa66db555558af9d607035a8bf2
URL: https://github.com/llvm/llvm-project/commit/20a2329148626aa66db555558af9d607035a8bf2
DIFF: https://github.com/llvm/llvm-project/commit/20a2329148626aa66db555558af9d607035a8bf2.diff
LOG: [lldb][Mach-O] Fix load-command loops spinning on cmdsize = 0 (#205134)
Every function in `ObjectFileMachO` and `ObjectContainerMachOFileset`
that iterates over load commands advances the file offset by
`lc.cmdsize` after reading each command. A malformed command with
cmdsize smaller than `sizeof(load_command)` (in particular cmdsize = 0)
does not make forward progress, so the loop spins for ncmds iterations.
With `ncmds` close to `INT_MAX` the function never returns in practice.
Factor the read-and-validate step into a static template helper
`ReadMachOCommand<T>` in each plugin's translation unit. It reads the
8-byte cmd/cmdsize header and returns false on EOF or on a cmdsize that
is too small to make forward progress. All load-command loops now use
this helper, replacing the previously duplicated GetU32 + cmdsize
check. `T` may be `llvm::MachO::load_command` or any of its richer
variants (uuid_command, dylib_command, thread_command, ident_command,
encryption_info_command, ...). The helper only touches the leading
cmd/cmdsize fields, leaving the rest of `T` for the caller to fill in.
Affected loops in `ObjectFileMachO`:
IsStripped, GetEncryptedFileRanges, CreateSections, ParseSymtab,
GetUUID (static), GetAllArchSpecs (two loops), GetDependentModules,
GetEntryPointAddress, GetNumThreadContexts, FindLC_NOTEByName,
GetIdentifierString, GetVersion, FindMinimumVersionInfo
And in ObjectContainerMachOFileset:
ParseFileset
Add unit tests (`ObjectFileMachOTest::ZeroCmdSize` and
`ObjectContainerMachOFilesetTest::ZeroCmdSize`) that feed a 40-byte
Mach-O with `ncmds = 0x7FFFFFFF` and `cmdsize = 0` into the relevant
parsers. Without the fix the tests spin ~2 billion iterations; with
the fix they return immediately. Found by lldb-target-fuzzer.
Assisted-by: Claude
Added:
Modified:
lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/unittests/ObjectContainer/CMakeLists.txt
lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp
Removed:
################################################################################
diff --git a/lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp b/lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp
index 706b8e38e9510..bc60c00e64317 100644
--- a/lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp
+++ b/lldb/source/Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.cpp
@@ -21,6 +21,31 @@ using namespace lldb;
using namespace lldb_private;
using namespace llvm::MachO;
+/// Read a Mach-O load-command header (cmd + cmdsize) from \p data at
+/// \p offset into \p cmd, advancing \p offset by 8 bytes. \p T may be
+/// \c llvm::MachO::load_command or any of its richer variants
+/// (\c thread_command, \c dylib_command, \c encryption_info_command, ...);
+/// only the leading cmd/cmdsize fields are touched by this read. Returns
+/// false on EOF or on a cmdsize smaller than sizeof(load_command), in which
+/// case callers should break out of their load-command loop to avoid spinning
+/// on malformed input.
+template <typename T>
+static bool ReadMachOCommand(DataExtractor &data, lldb::offset_t &offset,
+ T &cmd) {
+ static_assert(offsetof(T, cmd) == 0, "T::cmd must be the first field");
+ static_assert(offsetof(T, cmdsize) == sizeof(uint32_t),
+ "T::cmdsize must immediately follow T::cmd");
+ static_assert(std::is_same<decltype(T::cmd), uint32_t>::value,
+ "T::cmd must be uint32_t");
+ static_assert(std::is_same<decltype(T::cmdsize), uint32_t>::value,
+ "T::cmdsize must be uint32_t");
+ if (data.GetU32(&offset, &cmd, 2) == nullptr)
+ return false;
+ if (cmd.cmdsize < sizeof(load_command))
+ return false;
+ return true;
+}
+
LLDB_PLUGIN_DEFINE(ObjectContainerMachOFileset)
void ObjectContainerMachOFileset::Initialize() {
@@ -142,7 +167,7 @@ ParseFileset(DataExtractor &extractor, mach_header header,
for (uint32_t i = 0; i < header.ncmds; ++i) {
const lldb::offset_t load_cmd_offset = offset;
load_command lc = {};
- if (extractor.GetU32(&offset, &lc.cmd, 2) == nullptr)
+ if (!ReadMachOCommand(extractor, offset, lc))
break;
// If we know the load address we can compute the slide.
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 6e49f211326ff..60b90309bee00 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -139,6 +139,31 @@ static constexpr llvm::StringLiteral g_executable_path = "@executable_path";
LLDB_PLUGIN_DEFINE(ObjectFileMachO)
+/// Read a Mach-O load-command header (cmd + cmdsize) from \p data at
+/// \p offset into \p cmd, advancing \p offset by 8 bytes. \p T may be
+/// \c llvm::MachO::load_command or any of its richer variants
+/// (\c thread_command, \c dylib_command, \c encryption_info_command, ...);
+/// only the leading cmd/cmdsize fields are touched by this read. Returns
+/// false on EOF or on a cmdsize smaller than sizeof(load_command), in which
+/// case callers should break out of their load-command loop to avoid spinning
+/// on malformed input.
+template <typename T>
+static bool ReadMachOCommand(const DataExtractor &data, lldb::offset_t &offset,
+ T &cmd) {
+ static_assert(offsetof(T, cmd) == 0, "T::cmd must be the first field");
+ static_assert(offsetof(T, cmdsize) == sizeof(uint32_t),
+ "T::cmdsize must immediately follow T::cmd");
+ static_assert(std::is_same<decltype(T::cmd), uint32_t>::value,
+ "T::cmd must be uint32_t");
+ static_assert(std::is_same<decltype(T::cmdsize), uint32_t>::value,
+ "T::cmdsize must be uint32_t");
+ if (data.GetU32(&offset, &cmd, 2) == nullptr)
+ return false;
+ if (cmd.cmdsize < sizeof(load_command))
+ return false;
+ return true;
+}
+
static void PrintRegisterValue(RegisterContext *reg_ctx, const char *name,
const char *alt_name, size_t reg_byte_size,
Stream &data) {
@@ -1306,7 +1331,7 @@ bool ObjectFileMachO::IsStripped() {
const lldb::offset_t load_cmd_offset = offset;
llvm::MachO::load_command lc = {};
- if (m_data_nsp->GetU32(&offset, &lc.cmd, 2) == nullptr)
+ if (!ReadMachOCommand(*m_data_nsp, offset, lc))
break;
if (lc.cmd == LC_DYSYMTAB) {
m_dysymtab.cmd = lc.cmd;
@@ -1335,7 +1360,7 @@ ObjectFileMachO::EncryptedFileRanges ObjectFileMachO::GetEncryptedFileRanges() {
llvm::MachO::encryption_info_command encryption_cmd;
for (uint32_t i = 0; i < m_header.ncmds; ++i) {
const lldb::offset_t load_cmd_offset = offset;
- if (m_data_nsp->GetU32(&offset, &encryption_cmd, 2) == nullptr)
+ if (!ReadMachOCommand(*m_data_nsp, offset, encryption_cmd))
break;
// LC_ENCRYPTION_INFO and LC_ENCRYPTION_INFO_64 have the same sizes for the
@@ -1881,7 +1906,7 @@ void ObjectFileMachO::CreateSections(SectionList &unified_section_list) {
llvm::MachO::load_command load_cmd;
for (uint32_t i = 0; i < m_header.ncmds; ++i) {
const lldb::offset_t load_cmd_offset = offset;
- if (m_data_nsp->GetU32(&offset, &load_cmd, 2) == nullptr)
+ if (!ReadMachOCommand(*m_data_nsp, offset, load_cmd))
break;
if (load_cmd.cmd == LC_SEGMENT || load_cmd.cmd == LC_SEGMENT_64)
@@ -2111,7 +2136,7 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
const lldb::offset_t cmd_offset = offset;
// Read in the load command and load command size
llvm::MachO::load_command lc;
- if (m_data_nsp->GetU32(&offset, &lc, 2) == nullptr)
+ if (!ReadMachOCommand(*m_data_nsp, offset, lc))
break;
// Watch for the symbol table load command
switch (lc.cmd) {
@@ -4452,7 +4477,7 @@ UUID ObjectFileMachO::GetUUID(const llvm::MachO::mach_header &header,
lldb::offset_t offset = lc_offset;
for (i = 0; i < header.ncmds; ++i) {
const lldb::offset_t cmd_offset = offset;
- if (data.GetU32(&offset, &load_cmd, 2) == nullptr)
+ if (!ReadMachOCommand(data, offset, load_cmd))
break;
if (load_cmd.cmd == LC_UUID) {
@@ -4611,7 +4636,7 @@ void ObjectFileMachO::GetAllArchSpecs(const llvm::MachO::mach_header &header,
lldb::offset_t offset = lc_offset;
for (uint32_t i = 0; i < header.ncmds; ++i) {
const lldb::offset_t cmd_offset = offset;
- if (data.GetU32(&offset, &load_cmd, 2) == nullptr)
+ if (!ReadMachOCommand(data, offset, load_cmd))
break;
llvm::MachO::version_min_command version_min;
@@ -4661,7 +4686,7 @@ void ObjectFileMachO::GetAllArchSpecs(const llvm::MachO::mach_header &header,
offset = lc_offset;
for (uint32_t i = 0; i < header.ncmds; ++i) {
const lldb::offset_t cmd_offset = offset;
- if (data.GetU32(&offset, &load_cmd, 2) == nullptr)
+ if (!ReadMachOCommand(data, offset, load_cmd))
break;
do {
@@ -4747,7 +4772,7 @@ uint32_t ObjectFileMachO::GetDependentModules(FileSpecList &files) {
uint32_t i;
for (i = 0; i < m_header.ncmds; ++i) {
const uint32_t cmd_offset = offset;
- if (m_data_nsp->GetU32(&offset, &load_cmd, 2) == nullptr)
+ if (!ReadMachOCommand(*m_data_nsp, offset, load_cmd))
break;
switch (load_cmd.cmd) {
@@ -4895,7 +4920,7 @@ lldb_private::Address ObjectFileMachO::GetEntryPointAddress() {
for (i = 0; i < m_header.ncmds; ++i) {
const lldb::offset_t cmd_offset = offset;
- if (m_data_nsp->GetU32(&offset, &load_cmd, 2) == nullptr)
+ if (!ReadMachOCommand(*m_data_nsp, offset, load_cmd))
break;
switch (load_cmd.cmd) {
@@ -5035,7 +5060,7 @@ uint32_t ObjectFileMachO::GetNumThreadContexts() {
llvm::MachO::thread_command thread_cmd;
for (uint32_t i = 0; i < m_header.ncmds; ++i) {
const uint32_t cmd_offset = offset;
- if (m_data_nsp->GetU32(&offset, &thread_cmd, 2) == nullptr)
+ if (!ReadMachOCommand(*m_data_nsp, offset, thread_cmd))
break;
if (thread_cmd.cmd == LC_THREAD) {
@@ -5061,7 +5086,7 @@ ObjectFileMachO::FindLC_NOTEByName(std::string name) {
for (uint32_t i = 0; i < m_header.ncmds; ++i) {
const uint32_t cmd_offset = offset;
llvm::MachO::load_command lc = {};
- if (m_data_nsp->GetU32(&offset, &lc.cmd, 2) == nullptr)
+ if (!ReadMachOCommand(*m_data_nsp, offset, lc))
break;
if (lc.cmd == LC_NOTE) {
char data_owner[17];
@@ -5111,7 +5136,7 @@ std::string ObjectFileMachO::GetIdentifierString() {
for (uint32_t i = 0; i < m_header.ncmds; ++i) {
const uint32_t cmd_offset = offset;
llvm::MachO::ident_command ident_command;
- if (m_data_nsp->GetU32(&offset, &ident_command, 2) == nullptr)
+ if (!ReadMachOCommand(*m_data_nsp, offset, ident_command))
break;
if (ident_command.cmd == LC_IDENT && ident_command.cmdsize != 0) {
std::string result(ident_command.cmdsize, '\0');
@@ -5535,7 +5560,7 @@ llvm::VersionTuple ObjectFileMachO::GetVersion() {
uint32_t i;
for (i = 0; i < m_header.ncmds; ++i) {
const lldb::offset_t cmd_offset = offset;
- if (m_data_nsp->GetU32(&offset, &load_cmd, 2) == nullptr)
+ if (!ReadMachOCommand(*m_data_nsp, offset, load_cmd))
break;
if (load_cmd.cmd == LC_ID_DYLIB) {
@@ -5695,7 +5720,7 @@ static llvm::VersionTuple FindMinimumVersionInfo(DataExtractor &data,
for (size_t i = 0; i < ncmds; i++) {
const lldb::offset_t load_cmd_offset = offset;
llvm::MachO::load_command lc = {};
- if (data.GetU32(&offset, &lc.cmd, 2) == nullptr)
+ if (!ReadMachOCommand(data, offset, lc))
break;
uint32_t version = 0;
diff --git a/lldb/unittests/ObjectContainer/CMakeLists.txt b/lldb/unittests/ObjectContainer/CMakeLists.txt
index 80778d1c32f17..8acfccf12f4dd 100644
--- a/lldb/unittests/ObjectContainer/CMakeLists.txt
+++ b/lldb/unittests/ObjectContainer/CMakeLists.txt
@@ -3,6 +3,7 @@ add_lldb_unittest(ObjectContainerTests
LINK_LIBS
lldbPluginObjectContainerMachOArchive
+ lldbPluginObjectContainerMachOFileset
lldbCore
lldbUtilityHelpers
LLVMTestingSupport
diff --git a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
index 50ab4c9557b66..2d4cbacc5169f 100644
--- a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
+++ b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h"
+#include "Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.h"
#include "TestingSupport/SubsystemRAII.h"
#include "TestingSupport/TestUtilities.h"
#include "lldb/Core/ModuleSpec.h"
@@ -14,6 +15,7 @@
#include "lldb/Symbol/ObjectFile.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/DataBufferHeap.h"
+#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/FileSpec.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Testing/Support/Error.h"
@@ -120,6 +122,47 @@ TEST_F(ObjectContainerUniversalMachOTest, SliceOffsetZero) {
ASSERT_THAT_ERROR(TmpFile->discard(), llvm::Succeeded());
}
+// Regression fixture: a Mach-O fileset whose single load command has
+// cmdsize = 0. With ncmds set near INT_MAX the function hangs. The
+// fix breaks out of the loop as soon as
+// cmdsize < sizeof(load_command). Found by lldb-target-fuzzer.
+namespace {
+class ObjectContainerMachOFilesetTest : public ::testing::Test {
+ SubsystemRAII<FileSystem, ObjectContainerMachOFileset> subsystems;
+};
+} // namespace
+
+TEST_F(ObjectContainerMachOFilesetTest, ZeroCmdSize) {
+ // Minimal little-endian x86_64 Mach-O fileset: mach_header_64 (32 bytes)
+ // followed by a single load_command with cmdsize = 0. ncmds is set to
+ // 0x7FFFFFFF so that without the fix ParseFileset spins ~2 billion times and
+ // never returns in practice; with the fix it breaks on the first iteration.
+ // Reaching the assertion below is the regression check.
+ auto ExpectedFile = TestFile::fromYaml(R"(
+--- !mach-o
+FileHeader:
+ magic: 0xFEEDFACF
+ cputype: 0x01000007
+ cpusubtype: 0x80000003
+ filetype: 0x0000000C
+ ncmds: 0x7FFFFFFF
+ sizeofcmds: 8
+ flags: 0x00000000
+ reserved: 0x00000000
+LoadCommands:
+ - cmd: LC_THREAD
+ cmdsize: 0
+...
+)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+ ModuleSpec Spec = ExpectedFile->moduleSpec();
+ lldb::DataExtractorSP DataSP = Spec.GetExtractor();
+ // Before the fix ParseFileset loops ~0x7FFFFFFF times and never returns.
+ (void)ObjectContainerMachOFileset::GetModuleSpecifications(
+ FileSpec(), DataSP, 0, DataSP->GetByteSize());
+}
+
// Regression fixture: a universal (fat) Mach-O whose header claims a huge
// nfat_arch (here 0xAFAFAFAF) but provides no fat_arch entries beyond the
// header bytes. Found by lldb-target-fuzzer.
diff --git a/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp b/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp
index b3a238022aa57..7ebbcd6d1974d 100644
--- a/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp
+++ b/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp
@@ -17,6 +17,7 @@
#include "lldb/Host/HostInfo.h"
#include "lldb/Symbol/ObjectFile.h"
#include "lldb/Symbol/Symtab.h"
+#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/lldb-defines.h"
#include "llvm/Testing/Support/Error.h"
@@ -112,6 +113,41 @@ TEST_F(ObjectFileMachOTest, IndirectSymbolsInTheSharedCache) {
}
#endif
+// Regression fixture: a Mach-O whose load commands all have cmdsize = 0.
+// With ncmds set near INT_MAX the functions hang. The fix breaks out of
+// the loop as soon as cmdsize < sizeof(load_command). Found by
+// lldb-target-fuzzer.
+TEST_F(ObjectFileMachOTest, ZeroCmdSize) {
+ // Minimal little-endian x86_64 Mach-O: mach_header_64 (32 bytes) followed
+ // by a single load_command with cmdsize = 0. ncmds is set to 0x7FFFFFFF so
+ // that without the fix the loops in GetAllArchSpecs/GetUUID never return in
+ // practice; with the fix they break on the very first iteration.
+ // Reaching the assertion below is the regression check.
+ auto ExpectedFile = TestFile::fromYaml(R"(
+--- !mach-o
+FileHeader:
+ magic: 0xFEEDFACF
+ cputype: 0x01000007
+ cpusubtype: 0x80000003
+ filetype: 0x00000002
+ ncmds: 0x7FFFFFFF
+ sizeofcmds: 8
+ flags: 0x00000000
+ reserved: 0x00000000
+LoadCommands:
+ - cmd: LC_THREAD
+ cmdsize: 0
+...
+)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+ ModuleSpec Spec = ExpectedFile->moduleSpec();
+ lldb::DataExtractorSP DataSP = Spec.GetExtractor();
+ // Before the fix GetAllArchSpecs loops ~0x7FFFFFFF times and never returns.
+ (void)ObjectFile::GetModuleSpecifications(FileSpec(), DataSP, 0,
+ DataSP->GetByteSize());
+}
+
// A Mach-O whose MH_DYLIB_IN_CACHE flag is set but which has no __LINKEDIT
// segment.
TEST_F(ObjectFileMachOTest, ParseSymtabSharedCacheMissingLinkedit) {
More information about the lldb-commits
mailing list