[llvm] [llvm][MachO] Fix infinite loop parsing bind table (PR #93897)

Zixu Wang via llvm-commits llvm-commits at lists.llvm.org
Thu May 30 17:50:41 PDT 2024


https://github.com/zixu-w updated https://github.com/llvm/llvm-project/pull/93897

>From a9009ee0dccde6b83c7d1ec6332b528898d5895e Mon Sep 17 00:00:00 2001
From: Zixu Wang <zixu_wang at apple.com>
Date: Thu, 30 May 2024 16:32:29 -0700
Subject: [PATCH] [MachO] Stop parsing past end of rebase/bind table

`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.
---
 llvm/lib/Object/MachOObjectFile.cpp           |  32 +-
 .../rebase-bind-table-trailing-opcode.yaml    | 389 ++++++++++++++++++
 ...se-bind-trailing-opcode-infinite-loop.test |  23 ++
 3 files changed, 430 insertions(+), 14 deletions(-)
 create mode 100644 llvm/test/Object/Inputs/MachO/rebase-bind-table-trailing-opcode.yaml
 create mode 100644 llvm/test/Object/macho-rebase-bind-trailing-opcode-infinite-loop.test

diff --git a/llvm/lib/Object/MachOObjectFile.cpp b/llvm/lib/Object/MachOObjectFile.cpp
index 863bc1219e7ac..61d880b19f751 100644
--- a/llvm/lib/Object/MachOObjectFile.cpp
+++ b/llvm/lib/Object/MachOObjectFile.cpp
@@ -3501,15 +3501,17 @@ void MachORebaseEntry::moveNext() {
     --RemainingLoopCount;
     return;
   }
-  // REBASE_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 REBASE_OPCODE_DONE.
-  if (Ptr == Opcodes.end()) {
-    Done = true;
-    return;
-  }
+
   bool More = true;
   while (More) {
+    // REBASE_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 REBASE_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++;
@@ -3838,15 +3840,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/rebase-bind-table-trailing-opcode.yaml b/llvm/test/Object/Inputs/MachO/rebase-bind-table-trailing-opcode.yaml
new file mode 100644
index 0000000000000..0857439329dcc
--- /dev/null
+++ b/llvm/test/Object/Inputs/MachO/rebase-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_SET_TYPE_IMM
+      Imm:             1
+  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-rebase-bind-trailing-opcode-infinite-loop.test b/llvm/test/Object/macho-rebase-bind-trailing-opcode-infinite-loop.test
new file mode 100644
index 0000000000000..6af2726fcc2e5
--- /dev/null
+++ b/llvm/test/Object/macho-rebase-bind-trailing-opcode-infinite-loop.test
@@ -0,0 +1,23 @@
+// 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[_*]`, following
+// a rebase table ending with an effectively no-op opcode
+// `REBASE_OPCODE_SET_TYPE_IMM(1)` instead of a `REBASE_OPCODE_DONE` or an
+// actual rebase `REBASE_OPCODE_DO_REBASE_*`.
+
+RUN: yaml2obj %p/Inputs/MachO/rebase-bind-table-trailing-opcode.yaml | \
+RUN: llvm-objdump --rebase --bind --weak-bind --macho - | \
+RUN: FileCheck %s
+
+CHECK:      Rebase table:
+CHECK-NEXT: segment      section            address     type
+CHECK-NEXT: __DATA_CONST __got              0x00008000  pointer
+CHECK-NEXT: __DATA       __la_symbol_ptr    0x0000C000  pointer
+
+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



More information about the llvm-commits mailing list