[lld] 0223ab0 - [lld-macho] check minimum header length when opening linkable input files
Greg McGary via llvm-commits
llvm-commits at lists.llvm.org
Sat Feb 27 14:41:53 PST 2021
Author: Greg McGary
Date: 2021-02-27T14:41:40-08:00
New Revision: 0223ab035c199e537a0040857ba147ced87fd533
URL: https://github.com/llvm/llvm-project/commit/0223ab035c199e537a0040857ba147ced87fd533
DIFF: https://github.com/llvm/llvm-project/commit/0223ab035c199e537a0040857ba147ced87fd533.diff
LOG: [lld-macho] check minimum header length when opening linkable input files
Bifurcate the `readFile()` API into ...
* `readRawFile()` which performs no checks, and
* `readLinkableFile()` which enforces minimum length of 20 bytes, same as ld64
There are no new tests because tweaks to existing tests are sufficient.
Differential Revision: https://reviews.llvm.org/D97610
Added:
lld/test/MachO/invalid/tiny-input.s
Modified:
lld/MachO/Driver.cpp
lld/MachO/DriverUtils.cpp
lld/MachO/InputFiles.cpp
lld/MachO/InputFiles.h
lld/test/MachO/rename.s
Removed:
################################################################################
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index ca6378aa5560..c5ae0aee7817 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -263,7 +263,7 @@ static std::vector<ArchiveMember> getArchiveMembers(MemoryBufferRef mb) {
static InputFile *addFile(StringRef path, bool forceLoadArchive,
bool isBundleLoader = false) {
- Optional<MemoryBufferRef> buffer = readFile(path);
+ Optional<MemoryBufferRef> buffer = readLinkableFile(path);
if (!buffer)
return nullptr;
MemoryBufferRef mbref = *buffer;
@@ -279,7 +279,7 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
error(path + ": archive has no index; run ranlib to add one");
if (config->allLoad || forceLoadArchive) {
- if (Optional<MemoryBufferRef> buffer = readFile(path)) {
+ if (Optional<MemoryBufferRef> buffer = readLinkableFile(path)) {
for (const ArchiveMember &member : getArchiveMembers(*buffer)) {
if (Optional<InputFile *> file = loadArchiveMember(
member.mbref, member.modTime, path, /*objCOnly=*/false)) {
@@ -300,7 +300,7 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
// we already found that it contains an ObjC symbol. We should also
// consider creating a LazyObjFile class in order to avoid double-loading
// these files here and below (as part of the ArchiveFile).
- if (Optional<MemoryBufferRef> buffer = readFile(path)) {
+ if (Optional<MemoryBufferRef> buffer = readLinkableFile(path)) {
for (const ArchiveMember &member : getArchiveMembers(*buffer)) {
if (Optional<InputFile *> file = loadArchiveMember(
member.mbref, member.modTime, path, /*objCOnly=*/true)) {
@@ -403,7 +403,7 @@ void macho::parseLCLinkerOption(InputFile* f, unsigned argc, StringRef data) {
}
static void addFileList(StringRef path) {
- Optional<MemoryBufferRef> buffer = readFile(path);
+ Optional<MemoryBufferRef> buffer = readRawFile(path);
if (!buffer)
return;
MemoryBufferRef mbref = *buffer;
@@ -426,7 +426,7 @@ static void addFileList(StringRef path) {
//
// The file can also have line comments that start with '#'.
static void parseOrderFile(StringRef path) {
- Optional<MemoryBufferRef> buffer = readFile(path);
+ Optional<MemoryBufferRef> buffer = readRawFile(path);
if (!buffer) {
error("Could not read order file at " + path);
return;
@@ -945,7 +945,7 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
StringRef segName = arg->getValue(0);
StringRef sectName = arg->getValue(1);
StringRef fileName = arg->getValue(2);
- Optional<MemoryBufferRef> buffer = readFile(fileName);
+ Optional<MemoryBufferRef> buffer = readRawFile(fileName);
if (buffer)
inputFiles.insert(make<OpaqueFile>(*buffer, segName, sectName));
}
diff --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp
index 6d9413ea4e57..c01d0a8ed8f1 100644
--- a/lld/MachO/DriverUtils.cpp
+++ b/lld/MachO/DriverUtils.cpp
@@ -132,7 +132,7 @@ std::string macho::createResponseFile(const opt::InputArgList &args) {
os << "-o " << quote(path::filename(arg->getValue())) << "\n";
break;
case OPT_filelist:
- if (Optional<MemoryBufferRef> buffer = readFile(arg->getValue()))
+ if (Optional<MemoryBufferRef> buffer = readRawFile(arg->getValue()))
for (StringRef path : args::getLines(*buffer))
os << quote(rewritePath(path)) << "\n";
break;
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 57126117c30c..d11c3f404b0c 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -91,7 +91,8 @@ std::unique_ptr<TarWriter> macho::tar;
int InputFile::idCount = 0;
// Open a given file path and return it as a memory-mapped file.
-Optional<MemoryBufferRef> macho::readFile(StringRef path) {
+// Perform no sanity checks--just open, map & return.
+Optional<MemoryBufferRef> macho::readRawFile(StringRef path) {
// Open a file.
auto mbOrErr = MemoryBuffer::getFile(path);
if (auto ec = mbOrErr.getError()) {
@@ -102,6 +103,27 @@ Optional<MemoryBufferRef> macho::readFile(StringRef path) {
std::unique_ptr<MemoryBuffer> &mb = *mbOrErr;
MemoryBufferRef mbref = mb->getMemBufferRef();
make<std::unique_ptr<MemoryBuffer>>(std::move(mb)); // take mb ownership
+ return mbref;
+}
+
+// Open a given file path and return it as a memory-mapped file.
+// Assume the file has one of a variety of linkable formats and
+// perform some basic sanity checks, notably minimum length.
+Optional<MemoryBufferRef> macho::readLinkableFile(StringRef path) {
+ Optional<MemoryBufferRef> maybeMbref = readRawFile(path);
+ if (!maybeMbref) {
+ return None;
+ }
+ MemoryBufferRef mbref = *maybeMbref;
+
+ // LD64 hard-codes 20 as minimum header size, which is presumably
+ // the smallest header among the the various linkable input formats
+ // LLD are less demanding. We insist on having only enough data for
+ // a magic number.
+ if (mbref.getBufferSize() < sizeof(uint32_t)) {
+ error("file is too small to contain a magic number: " + path);
+ return None;
+ }
// If this is a regular non-fat file, return it.
const char *buf = mbref.getBufferStart();
@@ -544,7 +566,7 @@ void ObjFile::parseDebugInfo() {
// The path can point to either a dylib or a .tbd file.
static Optional<DylibFile *> loadDylib(StringRef path, DylibFile *umbrella) {
- Optional<MemoryBufferRef> mbref = readFile(path);
+ Optional<MemoryBufferRef> mbref = readLinkableFile(path);
if (!mbref) {
error("could not read dylib file at " + path);
return {};
diff --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index 10935050584d..5ee3f7835098 100644
--- a/lld/MachO/InputFiles.h
+++ b/lld/MachO/InputFiles.h
@@ -173,7 +173,8 @@ class BitcodeFile : public InputFile {
extern llvm::SetVector<InputFile *> inputFiles;
-llvm::Optional<MemoryBufferRef> readFile(StringRef path);
+llvm::Optional<MemoryBufferRef> readRawFile(StringRef path);
+llvm::Optional<MemoryBufferRef> readLinkableFile(StringRef path);
const llvm::MachO::load_command *
findCommand(const llvm::MachO::mach_header_64 *, uint32_t type);
diff --git a/lld/test/MachO/invalid/tiny-input.s b/lld/test/MachO/invalid/tiny-input.s
new file mode 100644
index 000000000000..64409b96d9ca
--- /dev/null
+++ b/lld/test/MachO/invalid/tiny-input.s
@@ -0,0 +1,18 @@
+# REQUIRES: x86
+
+## Check that files too short to have a magic number are rejected as inputs
+# RUN: echo -n 1 >%t-1.o
+# RUN: echo -n 12 >%t-2.o
+# RUN: echo -n 123 >%t-3.o
+# RUN: echo -n 1234 >%t-4.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
+# RUN: not %lld -o %t %t.o %t-1.o %t-2.o %t-3.o %t-4.o 2>&1 | FileCheck %s
+
+# CHECK: error: file is too small to contain a magic number: {{.*}}-1.o
+# CHECK: error: file is too small to contain a magic number: {{.*}}-2.o
+# CHECK: error: file is too small to contain a magic number: {{.*}}-3.o
+# CHECK: error: {{.*}}-4.o: unhandled file type
+
+.global _main
+_main:
+ ret
diff --git a/lld/test/MachO/rename.s b/lld/test/MachO/rename.s
index 77885b19fdef..2a0567416c52 100644
--- a/lld/test/MachO/rename.s
+++ b/lld/test/MachO/rename.s
@@ -14,7 +14,7 @@
# BAD1-DAG: error: invalid name for segment or section: S/ASHY_SEG
# BAD1-DAG: error: invalid name for segment or section: st*rry_sect
# BAD1-DAG: error: invalid name for segment or section: -o
-# BAD1-DAG: error: /dev/null: unhandled file type
+# BAD1-DAG: error: file is too small to contain a magic number:
# RUN: not %lld \
# RUN: -rename_segment H#SHY_SEG PL+SSY_SEG \
@@ -24,7 +24,7 @@
# BAD2-DAG: error: invalid name for segment or section: H#SHY_SEG
# BAD2-DAG: error: invalid name for segment or section: PL+SSY_SEG
# BAD2-DAG: error: invalid name for segment or section: -o
-# BAD2-DAG: error: /dev/null: unhandled file type
+# BAD2-DAG: error: file is too small to contain a magic number:
## Check that section and segment renames happen
# RUN: %lld \
More information about the llvm-commits
mailing list