[llvm] [llvm][MachO] Fix integer truncation in rebase/bind parsing (PR #89337)
Zixu Wang via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 18 18:27:56 PDT 2024
https://github.com/zixu-w updated https://github.com/llvm/llvm-project/pull/89337
>From b43c97b292e192180408029f59b862f3502e5311 Mon Sep 17 00:00:00 2001
From: Zixu Wang <zixu_wang at apple.com>
Date: Thu, 18 Apr 2024 18:08:48 -0700
Subject: [PATCH 1/2] [llvm][MachO] Fix integer truncation in rebase/bind
parsing
`Skip` could use up the full 64 bits decoded by `readULEB128` for some
rebase/bind opcodes.
In `*_OPCODE_DO_*_ULEB_TIMES_SKIPPING_ULEB`, `Skip` could be encoded as
a two's complement for moving `SegmentOffset` backwards. Having a 32-bit
`Skip` truncates the encoded value and leads to a malformed `AdvanceAmount`
and invalid `SegmentOffset` that extends past valid sections.
---
llvm/include/llvm/Object/MachO.h | 6 +++---
llvm/lib/Object/MachOObjectFile.cpp | 20 +++++++++++++++-----
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/llvm/include/llvm/Object/MachO.h b/llvm/include/llvm/Object/MachO.h
index 24f9954584ed5d..bf35d9c4e9905b 100644
--- a/llvm/include/llvm/Object/MachO.h
+++ b/llvm/include/llvm/Object/MachO.h
@@ -136,7 +136,7 @@ class BindRebaseSegInfo {
// Used to check a Mach-O Bind or Rebase entry for errors when iterating.
const char* checkSegAndOffsets(int32_t SegIndex, uint64_t SegOffset,
uint8_t PointerSize, uint32_t Count=1,
- uint32_t Skip=0);
+ uint64_t Skip=0);
// Used with valid SegIndex/SegOffset values from checked entries.
StringRef segmentName(int32_t SegIndex);
StringRef sectionName(int32_t SegIndex, uint64_t SegOffset);
@@ -577,7 +577,7 @@ class MachOObjectFile : public ObjectFile {
// This is used by MachOBindEntry::moveNext() to validate a MachOBindEntry.
const char *BindEntryCheckSegAndOffsets(int32_t SegIndex, uint64_t SegOffset,
uint8_t PointerSize, uint32_t Count=1,
- uint32_t Skip=0) const {
+ uint64_t Skip=0) const {
return BindRebaseSectionTable->checkSegAndOffsets(SegIndex, SegOffset,
PointerSize, Count, Skip);
}
@@ -592,7 +592,7 @@ class MachOObjectFile : public ObjectFile {
uint64_t SegOffset,
uint8_t PointerSize,
uint32_t Count=1,
- uint32_t Skip=0) const {
+ uint64_t Skip=0) const {
return BindRebaseSectionTable->checkSegAndOffsets(SegIndex, SegOffset,
PointerSize, Count, Skip);
}
diff --git a/llvm/lib/Object/MachOObjectFile.cpp b/llvm/lib/Object/MachOObjectFile.cpp
index 1cfd0a069463e9..deeabef7e7501f 100644
--- a/llvm/lib/Object/MachOObjectFile.cpp
+++ b/llvm/lib/Object/MachOObjectFile.cpp
@@ -3515,7 +3515,12 @@ void MachORebaseEntry::moveNext() {
uint8_t Byte = *Ptr++;
uint8_t ImmValue = Byte & MachO::REBASE_IMMEDIATE_MASK;
uint8_t Opcode = Byte & MachO::REBASE_OPCODE_MASK;
- uint32_t Count, Skip;
+ uint32_t Count;
+ // For opcode REBASE_OPCODE_DO_REBASE_ULEB_TIMES_SKIPPING_ULEB, the ULEB128
+ // encoded Skip amount could be two's complements for moving SegmentOffset
+ // to a lower address. Skip should be the same integer width as
+ // SegmentOffset and AdvanceAmount to prevent truncating.
+ uint64_t Skip;
const char *error = nullptr;
switch (Opcode) {
case MachO::REBASE_OPCODE_DONE:
@@ -3854,7 +3859,12 @@ void MachOBindEntry::moveNext() {
uint8_t Opcode = Byte & MachO::BIND_OPCODE_MASK;
int8_t SignExtended;
const uint8_t *SymStart;
- uint32_t Count, Skip;
+ uint32_t Count;
+ // For opcode BIND_OPCODE_DO_BIND_ULEB_TIMES_SKIPPING_ULEB, the ULEB128
+ // encoded Skip amount could be two's complements for moving SegmentOffset
+ // to a lower address. Skip should be the same integer width as
+ // SegmentOffset and AdvanceAmount to prevent truncating.
+ uint64_t Skip;
const char *error = nullptr;
switch (Opcode) {
case MachO::BIND_OPCODE_DONE:
@@ -4388,14 +4398,14 @@ const char * BindRebaseSegInfo::checkSegAndOffsets(int32_t SegIndex,
uint64_t SegOffset,
uint8_t PointerSize,
uint32_t Count,
- uint32_t Skip) {
+ uint64_t Skip) {
if (SegIndex == -1)
return "missing preceding *_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB";
if (SegIndex >= MaxSegIndex)
return "bad segIndex (too large)";
for (uint32_t i = 0; i < Count; ++i) {
- uint32_t Start = SegOffset + i * (PointerSize + Skip);
- uint32_t End = Start + PointerSize;
+ uint64_t Start = SegOffset + i * (PointerSize + Skip);
+ uint64_t End = Start + PointerSize;
bool Found = false;
for (const SectionInfo &SI : Sections) {
if (SI.SegmentIndex != SegIndex)
>From a638890f31faedb345da2d5db86e4ff45bfd7c64 Mon Sep 17 00:00:00 2001
From: Zixu Wang <zixu_wang at apple.com>
Date: Thu, 18 Apr 2024 18:27:19 -0700
Subject: [PATCH 2/2] [NFC] Apply code formatting
---
llvm/include/llvm/Object/MachO.h | 15 ++++++++-------
llvm/lib/Object/MachOObjectFile.cpp | 10 +++++-----
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/llvm/include/llvm/Object/MachO.h b/llvm/include/llvm/Object/MachO.h
index bf35d9c4e9905b..1d41ad212a8ded 100644
--- a/llvm/include/llvm/Object/MachO.h
+++ b/llvm/include/llvm/Object/MachO.h
@@ -134,9 +134,9 @@ class BindRebaseSegInfo {
BindRebaseSegInfo(const MachOObjectFile *Obj);
// Used to check a Mach-O Bind or Rebase entry for errors when iterating.
- const char* checkSegAndOffsets(int32_t SegIndex, uint64_t SegOffset,
- uint8_t PointerSize, uint32_t Count=1,
- uint64_t Skip=0);
+ const char *checkSegAndOffsets(int32_t SegIndex, uint64_t SegOffset,
+ uint8_t PointerSize, uint32_t Count = 1,
+ uint64_t Skip = 0);
// Used with valid SegIndex/SegOffset values from checked entries.
StringRef segmentName(int32_t SegIndex);
StringRef sectionName(int32_t SegIndex, uint64_t SegOffset);
@@ -576,8 +576,9 @@ class MachOObjectFile : public ObjectFile {
//
// This is used by MachOBindEntry::moveNext() to validate a MachOBindEntry.
const char *BindEntryCheckSegAndOffsets(int32_t SegIndex, uint64_t SegOffset,
- uint8_t PointerSize, uint32_t Count=1,
- uint64_t Skip=0) const {
+ uint8_t PointerSize,
+ uint32_t Count = 1,
+ uint64_t Skip = 0) const {
return BindRebaseSectionTable->checkSegAndOffsets(SegIndex, SegOffset,
PointerSize, Count, Skip);
}
@@ -591,8 +592,8 @@ class MachOObjectFile : public ObjectFile {
const char *RebaseEntryCheckSegAndOffsets(int32_t SegIndex,
uint64_t SegOffset,
uint8_t PointerSize,
- uint32_t Count=1,
- uint64_t Skip=0) const {
+ uint32_t Count = 1,
+ uint64_t Skip = 0) const {
return BindRebaseSectionTable->checkSegAndOffsets(SegIndex, SegOffset,
PointerSize, Count, Skip);
}
diff --git a/llvm/lib/Object/MachOObjectFile.cpp b/llvm/lib/Object/MachOObjectFile.cpp
index deeabef7e7501f..04e0589341e7db 100644
--- a/llvm/lib/Object/MachOObjectFile.cpp
+++ b/llvm/lib/Object/MachOObjectFile.cpp
@@ -4394,11 +4394,11 @@ BindRebaseSegInfo::BindRebaseSegInfo(const object::MachOObjectFile *Obj) {
// that fully contains a pointer at that location. Multiple fixups in a bind
// (such as with the BIND_OPCODE_DO_BIND_ULEB_TIMES_SKIPPING_ULEB opcode) can
// be tested via the Count and Skip parameters.
-const char * BindRebaseSegInfo::checkSegAndOffsets(int32_t SegIndex,
- uint64_t SegOffset,
- uint8_t PointerSize,
- uint32_t Count,
- uint64_t Skip) {
+const char *BindRebaseSegInfo::checkSegAndOffsets(int32_t SegIndex,
+ uint64_t SegOffset,
+ uint8_t PointerSize,
+ uint32_t Count,
+ uint64_t Skip) {
if (SegIndex == -1)
return "missing preceding *_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB";
if (SegIndex >= MaxSegIndex)
More information about the llvm-commits
mailing list