From: Greg McGary Date: Sat, 27 Feb 2021 07:28:52 +0000 (-0800) Subject: [lld-macho] check minimum header length when opening linkable input files X-Git-Tag: llvmorg-14-init~13844 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=0223ab035c199e537a0040857ba147ced87fd533;p=platform%2Fupstream%2Fllvm.git [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 --- diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp index ca6378a..c5ae0ae 100644 --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -263,7 +263,7 @@ static std::vector getArchiveMembers(MemoryBufferRef mb) { static InputFile *addFile(StringRef path, bool forceLoadArchive, bool isBundleLoader = false) { - Optional buffer = readFile(path); + Optional 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 buffer = readFile(path)) { + if (Optional buffer = readLinkableFile(path)) { for (const ArchiveMember &member : getArchiveMembers(*buffer)) { if (Optional 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 buffer = readFile(path)) { + if (Optional buffer = readLinkableFile(path)) { for (const ArchiveMember &member : getArchiveMembers(*buffer)) { if (Optional 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 buffer = readFile(path); + Optional 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 buffer = readFile(path); + Optional buffer = readRawFile(path); if (!buffer) { error("Could not read order file at " + path); return; @@ -945,7 +945,7 @@ bool macho::link(ArrayRef argsArr, bool canExitEarly, StringRef segName = arg->getValue(0); StringRef sectName = arg->getValue(1); StringRef fileName = arg->getValue(2); - Optional buffer = readFile(fileName); + Optional buffer = readRawFile(fileName); if (buffer) inputFiles.insert(make(*buffer, segName, sectName)); } diff --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp index 6d9413e..c01d0a8 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 buffer = readFile(arg->getValue())) + if (Optional 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 5712611..d11c3f4 100644 --- a/lld/MachO/InputFiles.cpp +++ b/lld/MachO/InputFiles.cpp @@ -91,7 +91,8 @@ std::unique_ptr macho::tar; int InputFile::idCount = 0; // Open a given file path and return it as a memory-mapped file. -Optional macho::readFile(StringRef path) { +// Perform no sanity checks--just open, map & return. +Optional macho::readRawFile(StringRef path) { // Open a file. auto mbOrErr = MemoryBuffer::getFile(path); if (auto ec = mbOrErr.getError()) { @@ -102,6 +103,27 @@ Optional macho::readFile(StringRef path) { std::unique_ptr &mb = *mbOrErr; MemoryBufferRef mbref = mb->getMemBufferRef(); make>(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 macho::readLinkableFile(StringRef path) { + Optional 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 loadDylib(StringRef path, DylibFile *umbrella) { - Optional mbref = readFile(path); + Optional 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 1093505..5ee3f78 100644 --- a/lld/MachO/InputFiles.h +++ b/lld/MachO/InputFiles.h @@ -173,7 +173,8 @@ public: extern llvm::SetVector inputFiles; -llvm::Optional readFile(StringRef path); +llvm::Optional readRawFile(StringRef path); +llvm::Optional 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 0000000..64409b9 --- /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 77885b1..2a05674 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 \