[MachO] Stop parsing past end of rebase/bind table #93897
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-binary-utilities Author: Zixu Wang (zixu-w) Changes
Full diff: https://github.com/llvm/llvm-project/pull/93897.diff 3 Files Affected:
diff --git a/llvm/lib/Object/MachOObjectFile.cpp b/llvm/lib/Object/MachOObjectFile.cpp
index 863bc1219e7ac..c3b0430d8ca9f 100644
--- a/llvm/lib/Object/MachOObjectFile.cpp
+++ b/llvm/lib/Object/MachOObjectFile.cpp
@@ -3838,15 +3838,17 @@ void MachOBindEntry::moveNext() {
--RemainingLoopCount;
return;
}
- // BIND_OPCODE_DONE is only used for padding if we are not aligned to
- // pointer size. Therefore it is possible to reach the end without ever having
- // seen BIND_OPCODE_DONE.
- if (Ptr == Opcodes.end()) {
- Done = true;
- return;
- }
+
bool More = true;
while (More) {
+ // BIND_OPCODE_DONE is only used for padding if we are not aligned to
+ // pointer size. Therefore it is possible to reach the end without ever
+ // having seen BIND_OPCODE_DONE.
+ if (Ptr == Opcodes.end()) {
+ Done = true;
+ return;
+ }
+
// Parse next opcode and set up next loop.
const uint8_t *OpcodeStart = Ptr;
uint8_t Byte = *Ptr++;
diff --git a/llvm/test/Object/Inputs/MachO/bind-table-trailing-opcode.yaml b/llvm/test/Object/Inputs/MachO/bind-table-trailing-opcode.yaml
new file mode 100644
index 0000000000000..bb07d133d2358
--- /dev/null
+++ b/llvm/test/Object/Inputs/MachO/bind-table-trailing-opcode.yaml
@@ -0,0 +1,389 @@
+--- !mach-o
+FileHeader:
+ magic: 0xFEEDFACF
+ cputype: 0x100000C
+ cpusubtype: 0x0
+ filetype: 0x6
+ ncmds: 15
+ sizeofcmds: 1232
+ flags: 0x118085
+ reserved: 0x0
+LoadCommands:
+ - cmd: LC_SEGMENT_64
+ cmdsize: 392
+ segname: __TEXT
+ vmaddr: 0
+ vmsize: 32768
+ fileoff: 0
+ filesize: 32768
+ maxprot: 5
+ initprot: 5
+ nsects: 4
+ flags: 0
+ Sections:
+ - sectname: __text
+ segname: __TEXT
+ addr: 0x4000
+ size: 32
+ offset: 0x4000
+ align: 2
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x80000400
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ content: C0035FD6FD7BBFA9FD03009105000094000080D206000094FD7BC1A8C0035FD6
+ - sectname: __stubs
+ segname: __TEXT
+ addr: 0x4020
+ size: 24
+ offset: 0x4020
+ align: 2
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x80000408
+ reserved1: 0x0
+ reserved2: 0xC
+ reserved3: 0x0
+ content: 30000090100240F900021FD650000090100240F900021FD6
+ - sectname: __stub_helper
+ segname: __TEXT
+ addr: 0x4038
+ size: 36
+ offset: 0x4038
+ align: 2
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x80000400
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ content: 5100009031220091F047BFA930000090100640F900021FD650000018F9FFFF1700000000
+ - sectname: __unwind_info
+ segname: __TEXT
+ addr: 0x405C
+ size: 96
+ offset: 0x405C
+ align: 2
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x0
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ content: 010000001C000000000000001C000000000000001C00000002000000004000004000000040000000204000000000000040000000000000000000000000000000030000000C000200140002000000000004000001000000020000000400000000
+ - cmd: LC_SEGMENT_64
+ cmdsize: 152
+ segname: __DATA_CONST
+ vmaddr: 32768
+ vmsize: 16384
+ fileoff: 32768
+ filesize: 16384
+ maxprot: 3
+ initprot: 3
+ nsects: 1
+ flags: 16
+ Sections:
+ - sectname: __got
+ segname: __DATA_CONST
+ addr: 0x8000
+ size: 16
+ offset: 0x8000
+ align: 3
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x6
+ reserved1: 0x2
+ reserved2: 0x0
+ reserved3: 0x0
+ content: '00400000000000000000000000000000'
+ - cmd: LC_SEGMENT_64
+ cmdsize: 232
+ segname: __DATA
+ vmaddr: 49152
+ vmsize: 16384
+ fileoff: 49152
+ filesize: 16384
+ maxprot: 3
+ initprot: 3
+ nsects: 2
+ flags: 0
+ Sections:
+ - sectname: __la_symbol_ptr
+ segname: __DATA
+ addr: 0xC000
+ size: 8
+ offset: 0xC000
+ align: 3
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x7
+ reserved1: 0x4
+ reserved2: 0x0
+ reserved3: 0x0
+ content: '5040000000000000'
+ - sectname: __data
+ segname: __DATA
+ addr: 0xC008
+ size: 8
+ offset: 0xC008
+ align: 3
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x0
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ content: '0000000000000000'
+ - cmd: LC_SEGMENT_64
+ cmdsize: 72
+ segname: __LINKEDIT
+ vmaddr: 65536
+ vmsize: 16384
+ fileoff: 65536
+ filesize: 272
+ maxprot: 1
+ initprot: 1
+ nsects: 0
+ flags: 0
+ - cmd: LC_ID_DYLIB
+ cmdsize: 48
+ dylib:
+ name: 24
+ timestamp: 1
+ current_version: 0
+ compatibility_version: 0
+ Content: '@rpath/libtest.dylib'
+ ZeroPadBytes: 4
+ - cmd: LC_DYLD_INFO_ONLY
+ cmdsize: 48
+ rebase_off: 65536
+ rebase_size: 8
+ bind_off: 65544
+ bind_size: 24
+ weak_bind_off: 65568
+ weak_bind_size: 16
+ lazy_bind_off: 65584
+ lazy_bind_size: 16
+ export_off: 65600
+ export_size: 40
+ - cmd: LC_SYMTAB
+ cmdsize: 24
+ symoff: 65648
+ nsyms: 5
+ stroff: 65752
+ strsize: 56
+ - cmd: LC_DYSYMTAB
+ cmdsize: 80
+ ilocalsym: 0
+ nlocalsym: 1
+ iextdefsym: 1
+ nextdefsym: 2
+ iundefsym: 3
+ nundefsym: 2
+ tocoff: 0
+ ntoc: 0
+ modtaboff: 0
+ nmodtab: 0
+ extrefsymoff: 0
+ nextrefsyms: 0
+ indirectsymoff: 65728
+ nindirectsyms: 5
+ extreloff: 0
+ nextrel: 0
+ locreloff: 0
+ nlocrel: 0
+ - cmd: LC_UUID
+ cmdsize: 24
+ uuid: 3A5ED8A0-F9D2-35D8-8C0E-4914289341CC
+ - cmd: LC_BUILD_VERSION
+ cmdsize: 32
+ platform: 2
+ minos: 1114112
+ sdk: 1179648
+ ntools: 1
+ Tools:
+ - tool: 3
+ version: 73073920
+ - cmd: LC_SOURCE_VERSION
+ cmdsize: 16
+ version: 0
+ - cmd: LC_ENCRYPTION_INFO_64
+ cmdsize: 24
+ cryptoff: 16384
+ cryptsize: 16384
+ cryptid: 0
+ pad: 0
+ - cmd: LC_LOAD_DYLIB
+ cmdsize: 56
+ dylib:
+ name: 24
+ timestamp: 2
+ current_version: 88539136
+ compatibility_version: 65536
+ Content: '/usr/lib/libSystem.B.dylib'
+ ZeroPadBytes: 6
+ - cmd: LC_FUNCTION_STARTS
+ cmdsize: 16
+ dataoff: 65640
+ datasize: 8
+ - cmd: LC_DATA_IN_CODE
+ cmdsize: 16
+ dataoff: 65648
+ datasize: 0
+LinkEditData:
+ RebaseOpcodes:
+ - Opcode: REBASE_OPCODE_SET_TYPE_IMM
+ Imm: 1
+ - Opcode: REBASE_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB
+ Imm: 1
+ ExtraData: [ 0x0 ]
+ - Opcode: REBASE_OPCODE_DO_REBASE_IMM_TIMES
+ Imm: 1
+ - Opcode: REBASE_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB
+ Imm: 2
+ ExtraData: [ 0x0 ]
+ - Opcode: REBASE_OPCODE_DO_REBASE_IMM_TIMES
+ Imm: 1
+ - Opcode: REBASE_OPCODE_DONE
+ Imm: 0
+ BindOpcodes:
+ - Opcode: BIND_OPCODE_SET_DYLIB_ORDINAL_IMM
+ Imm: 1
+ Symbol: ''
+ - Opcode: BIND_OPCODE_SET_SYMBOL_TRAILING_FLAGS_IMM
+ Imm: 0
+ Symbol: dyld_stub_binder
+ - Opcode: BIND_OPCODE_SET_TYPE_IMM
+ Imm: 1
+ Symbol: ''
+ - Opcode: BIND_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB
+ Imm: 1
+ ULEBExtraData: [ 0x8 ]
+ Symbol: ''
+ - Opcode: BIND_OPCODE_DO_BIND
+ Imm: 0
+ Symbol: ''
+ - Opcode: BIND_OPCODE_SET_TYPE_IMM
+ Imm: 1
+ Symbol: ''
+ WeakBindOpcodes:
+ - Opcode: BIND_OPCODE_SET_SYMBOL_TRAILING_FLAGS_IMM
+ Imm: 0
+ Symbol: _foo
+ - Opcode: BIND_OPCODE_SET_TYPE_IMM
+ Imm: 1
+ Symbol: ''
+ - Opcode: BIND_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB
+ Imm: 1
+ ULEBExtraData: [ 0x0 ]
+ Symbol: ''
+ - Opcode: BIND_OPCODE_DO_BIND
+ Imm: 0
+ Symbol: ''
+ - Opcode: BIND_OPCODE_DONE
+ Imm: 0
+ Symbol: ''
+ LazyBindOpcodes:
+ - Opcode: BIND_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB
+ Imm: 2
+ ULEBExtraData: [ 0x0 ]
+ Symbol: ''
+ - Opcode: BIND_OPCODE_SET_DYLIB_ORDINAL_IMM
+ Imm: 1
+ Symbol: ''
+ - Opcode: BIND_OPCODE_SET_SYMBOL_TRAILING_FLAGS_IMM
+ Imm: 0
+ Symbol: _free
+ - Opcode: BIND_OPCODE_DO_BIND
+ Imm: 0
+ Symbol: ''
+ - Opcode: BIND_OPCODE_DONE
+ Imm: 0
+ Symbol: ''
+ - Opcode: BIND_OPCODE_DONE
+ Imm: 0
+ Symbol: ''
+ - Opcode: BIND_OPCODE_DONE
+ Imm: 0
+ Symbol: ''
+ - Opcode: BIND_OPCODE_DONE
+ Imm: 0
+ Symbol: ''
+ - Opcode: BIND_OPCODE_DONE
+ Imm: 0
+ Symbol: ''
+ ExportTrie:
+ TerminalSize: 0
+ NodeOffset: 0
+ Name: ''
+ Flags: 0x0
+ Address: 0x0
+ Other: 0x0
+ ImportName: ''
+ Children:
+ - TerminalSize: 0
+ NodeOffset: 21
+ Name: _
+ Flags: 0x0
+ Address: 0x0
+ Other: 0x0
+ ImportName: ''
+ Children:
+ - TerminalSize: 4
+ NodeOffset: 9
+ Name: bar
+ Flags: 0x0
+ Address: 0x4004
+ Other: 0x0
+ ImportName: ''
+ - TerminalSize: 4
+ NodeOffset: 15
+ Name: foo
+ Flags: 0x4
+ Address: 0x4000
+ Other: 0x0
+ ImportName: ''
+ NameList:
+ - n_strx: 35
+ n_type: 0xE
+ n_sect: 7
+ n_desc: 0
+ n_value: 49160
+ - n_strx: 2
+ n_type: 0xF
+ n_sect: 1
+ n_desc: 0
+ n_value: 16388
+ - n_strx: 7
+ n_type: 0xF
+ n_sect: 1
+ n_desc: 128
+ n_value: 16384
+ - n_strx: 12
+ n_type: 0x1
+ n_sect: 0
+ n_desc: 256
+ n_value: 0
+ - n_strx: 18
+ n_type: 0x1
+ n_sect: 0
+ n_desc: 256
+ n_value: 0
+ StringTable:
+ - ' '
+ - _bar
+ - _foo
+ - _free
+ - dyld_stub_binder
+ - __dyld_private
+ - ''
+ - ''
+ - ''
+ - ''
+ - ''
+ - ''
+ IndirectSymbols: [ 0x2, 0x3, 0x2, 0x4, 0x3 ]
+ FunctionStarts: [ 0x4000, 0x4004 ]
+...
diff --git a/llvm/test/Object/macho-bind-trailing-opcode-infinite-loop.test b/llvm/test/Object/macho-bind-trailing-opcode-infinite-loop.test
new file mode 100644
index 0000000000000..18234868a6f0f
--- /dev/null
+++ b/llvm/test/Object/macho-bind-trailing-opcode-infinite-loop.test
@@ -0,0 +1,15 @@
+// A valid MachO object with a weak-bind table following a bind table ending
+// with an effectively no-op opcode `BIND_OPCODE_SET_TYPE_IMM(1)` instead of
+// a `BIND_OPCODE_DONE` or an actual bind `BIND_OPCODE_DO_BIND[_*]`.
+
+RUN: yaml2obj %p/Inputs/MachO/bind-table-trailing-opcode.yaml | \
+RUN: llvm-objdump --bind --weak-bind --macho - | \
+RUN: FileCheck %s
+
+CHECK: Bind table:
+CHECK-NEXT: segment section address type addend dylib symbol
+CHECK-NEXT: __DATA_CONST __got 0x00008008 pointer 0 libSystem dyld_stub_binder
+
+CHECK: Weak bind table:
+CHECK-NEXT: segment section address type addend symbol
+CHECK-NEXT: __DATA_CONST __got 0x00008000 pointer 0 _foo
|
|
The same logic also presents in |
zixu-w
force-pushed
the
llvm-macho-bind-table-infinite-loop
branch
from
b0e683e to
a9009ee
Compare
zixu-w
changed the title
|
Updated fix and test for the rebase table.
|
cyndyishida
approved these changes
May 31, 2024
llvm/test/Object/macho-rebase-bind-trailing-opcode-infinite-loop.test
Outdated
Show resolved
Hide resolved
`MachORebaseEntry::moveNext()` and `MachOBindEntry::moveNext()` assume
that the rebase/bind table ends with `{REBASE|BIND}_OPCODE_DONE` or an
actual rebase/bind. However a valid rebase/bind table might also end
with other effectively no-op opcodes, which caused the parser to move
past the end and go into the next table, resulting in corrupted entries
or infinite loops.
zixu-w
force-pushed
the
llvm-macho-bind-table-infinite-loop
branch
from
a9009ee to
c358b02
Compare
zixu-w
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Jun 3, 2024
`MachORebaseEntry::moveNext()` and `MachOBindEntry::moveNext()` assume
that the rebase/bind table ends with `{REBASE|BIND}_OPCODE_DONE` or an
actual rebase/bind. However a valid rebase/bind table might also end
with other effectively no-op opcodes, which caused the parser to move
past the end and go into the next table, resulting in corrupted entries
or infinite loops.
(cherry picked from commit 1fa073a)
fredriss
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Jun 7, 2024
🍒[release/6.0][MachO] Stop parsing past end of rebase/bind table (llvm#93897)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MachOBindEntry::moveNext()assumes that the bind table ends withBIND_OPCODE_DONEor a bind (BIND_OPCODE_DO_BIND[_*]). However a valid bind table might also end with other effectively no-op opcodes, which caused the parser to move past the end and go into the next table (weak bind table) and bounced back in a loop.