Extract LC_CODE_SIGNATURE related implementation out of LLD

[nuriamari]

Summary

Move the functionality in lld that handles writing of the LC_CODE_SIGNATURE load command and associated data section to a central reusable location.

This change is in preparation for another change that modifies llvm-objcopy to reproduce the LC_CODE_SIGNATURE load command and corresponding
data section to maintain the validity of signed macho object files passed through llvm-objcopy.

[thakis]

Move itself looks fine, but why is there a new lld test in here? This should be behavior-preserving, so I wouldn't expect test updates.

If you want to add a new test to lld independently from this change (?), I'd put that in a separate patch.

[nuriamari]

In D109803#3001863, @thakis wrote:

Move itself looks fine, but why is there a new lld test in here? This should be behavior-preserving, so I wouldn't expect test updates.

If you want to add a new test to lld independently from this change (?), I'd put that in a separate patch.

The new test is to ensure no behavior is broken. The existing code signature related test does not test that the hash was performed properly, just that the load command is included. This test is to make sure the move doesn't break the signature generation.

[thakis]

In D109803#3001885, @nuriamari wrote:

In D109803#3001863, @thakis wrote:

Move itself looks fine, but why is there a new lld test in here? This should be behavior-preserving, so I wouldn't expect test updates.

If you want to add a new test to lld independently from this change (?), I'd put that in a separate patch.

The new test is to ensure no behavior is broken. The existing code signature related test does not test that the hash was performed properly, just that the load command is included. This test is to make sure the move doesn't break the signature generation.

Can we land that in its own patch? :)

[nuriamari]

Remove new test, now included in separate patch

[nuriamari]

In D109803#3001996, @thakis wrote:

In D109803#3001885, @nuriamari wrote:

In D109803#3001863, @thakis wrote:

Move itself looks fine, but why is there a new lld test in here? This should be behavior-preserving, so I wouldn't expect test updates.

If you want to add a new test to lld independently from this change (?), I'd put that in a separate patch.

The new test is to ensure no behavior is broken. The existing code signature related test does not test that the hash was performed properly, just that the load command is included. This test is to make sure the move doesn't break the signature generation.

Can we land that in its own patch? :)

Sure :) https://reviews.llvm.org/D109840

[thevinster]

lld/MachO/SyntheticSections.cpp

1148–1149

Any specific reason for not namespacing object? :)

1170–1171

It seems redundant to have both writeTo and writeHashes now that it's combined. Comment makes since in the context of this diff but it doesn't make too much sense outside of the context unless people are referring to this diff. Personally, I would rename all places that were using writeHashes to writeTo and get rid of writeHashes completely from this class (unless there other issues preventing this)

[oontvoo]

lld/MachO/SyntheticSections.cpp

1148–1149

this is now a stray comment?

llvm/include/llvm/Object/MachO.h

737–750

Please consider re-arranging the members. I think the convention is to leave private fields at the end of the class, after private methods. (Also explicit modifier is preferred).

More generally, the order of members are <PUBLIC>, <PROTECTED>, <PRIVATE>, then within each, methods go before fields.

llvm/lib/Object/CodeSignatureSection.cpp

28

not needed - unless you're going to add some text here?

[nuriamari]

Fix stray comment, rearrange type definition and clean up static asserts

lld/MachO/SyntheticSections.cpp

1148–1149

If I namespace object, there is a collision between the CodeSignatureSection defined in SyntheticSections.h and the desired definition in MachO.h.

1170–1171

The base class requires writeTo to be defined and writeTo can't cleanly be used to write the hashes since they can only be written once everything else in the binary is written.

[oontvoo]

LGTM

llvm/lib/Object/CodeSignatureSection.cpp

28

Oops, sorry about this suggestion - I forgot LLVM is still on C++14.

[nuriamari]

Satisfy clang-tidy / clang-format

[nuriamari]

Restore static asserts

[int3]

lld/MachO/SyntheticSections.cpp

1155–1157

can we construct this once in CodeSignatureSection::finalize()? (so CodeSignatureSection would have a std::unique_ptr<object::CodeSignatureSection> member that can be reused in writeHashes)

[nuriamari]

Construct object::CodeSignature object once

[int3]

lgtm

[drodriguez]

Reverted in https://reviews.llvm.org/D110974

lld/MachO/SyntheticSections.h

 #include "OutputSegment.h"
 #include "Target.h"
 #include "Writer.h"
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/MC/StringTableBuilder.h"
+#include "llvm/Object/MachO.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include <unordered_map>
 
 namespace llvm {
 class DWARFUnit;
 } // namespace llvm
public:
   }
   bool isNeeded() const override;
   void writeTo(uint8_t *buf) const override;
 };
 
 // The code signature comes at the very end of the linked output file.
 class CodeSignatureSection final : public LinkEditSection {
 public:
-  static constexpr uint8_t blockSizeShift = 12;
-  static constexpr size_t blockSize = (1 << blockSizeShift); // 4 KiB
-  static constexpr size_t hashSize = 256 / 8;
-  static constexpr size_t blobHeadersSize = llvm::alignTo<8>(
-      sizeof(llvm::MachO::CS_SuperBlob) + sizeof(llvm::MachO::CS_BlobIndex));
-  static constexpr uint32_t fixedHeadersSize =
-      blobHeadersSize + sizeof(llvm::MachO::CS_CodeDirectory);
-
-  uint32_t fileNamePad = 0;
-  uint32_t allHeadersSize = 0;
-  StringRef fileName;
-
   CodeSignatureSection();
   uint64_t getRawSize() const override;
   bool isNeeded() const override { return true; }
   void writeTo(uint8_t *buf) const override;
-  uint32_t getBlockCount() const;
   void writeHashes(uint8_t *buf) const;
 };
 
 class BitcodeBundleSection final : public SyntheticSection {
 public:
   BitcodeBundleSection();
   uint64_t getSize() const override { return xarSize; }
   void finalize() override;

lld/MachO/SyntheticSections.cpp

 #include "lld/Common/ErrorHandler.h"
 #include "lld/Common/Memory.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/EndianStream.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/SHA256.h"
-
-#if defined(__APPLE__)
-#include <sys/mman.h>
-#endif
 
 #ifdef LLVM_HAVE_LIBXAR
 #include <fcntl.h>
 #include <xar/xar.h>
 #endif
 
 using namespace llvm;
 using namespace llvm::MachO;

 void StringTableSection::writeTo(uint8_t *buf) const {
   uint32_t off = 0;
   for (StringRef str : strings) {
     memcpy(buf + off, str.data(), str.size());
     off += str.size() + 1; // account for null terminator
   }
 }
 
-static_assert((CodeSignatureSection::blobHeadersSize % 8) == 0, "");
-static_assert((CodeSignatureSection::fixedHeadersSize % 8) == 0, "");
-
 CodeSignatureSection::CodeSignatureSection()
     : LinkEditSection(segment_names::linkEdit, section_names::codeSignature) {
-  align = 16; // required by libstuff
-  // FIXME: Consider using finalOutput instead of outputFile.
-  fileName = config->outputFile;
-  size_t slashIndex = fileName.rfind("/");
-  if (slashIndex != std::string::npos)
-    fileName = fileName.drop_front(slashIndex + 1);
-  allHeadersSize = alignTo<16>(fixedHeadersSize + fileName.size() + 1);
-  fileNamePad = allHeadersSize - fixedHeadersSize - fileName.size();
-}
-
-uint32_t CodeSignatureSection::getBlockCount() const {
-  return (fileOff + blockSize - 1) / blockSize;
+  align = object::CodeSignatureSection::Align; // required by libstuff
 }
 
 uint64_t CodeSignatureSection::getRawSize() const {
-  return allHeadersSize + getBlockCount() * hashSize;
+  OutputSegment *textSeg = getOrCreateOutputSegment(segment_names::text);
+  // NOTE: ld64 seems to also use outputFile instead of finalOutput
+  object::CodeSignatureSection sectionBuilder = object::CodeSignatureSection(
+      fileOff, config->outputFile, config->outputType, textSeg->fileOff,
+      textSeg->fileSize);
+  return static_cast<uint64_t>(sectionBuilder.getRawSize());
 }
 
 void CodeSignatureSection::writeHashes(uint8_t *buf) const {
-  uint8_t *code = buf;
-  uint8_t *codeEnd = buf + fileOff;
-  uint8_t *hashes = codeEnd + allHeadersSize;
-  while (code < codeEnd) {
-    StringRef block(reinterpret_cast<char *>(code),
-                    std::min(codeEnd - code, static_cast<ssize_t>(blockSize)));
-    SHA256 hasher;
-    hasher.update(block);
-    StringRef hash = hasher.final();
-    assert(hash.size() == hashSize);
-    memcpy(hashes, hash.data(), hashSize);
-    code += blockSize;
-    hashes += hashSize;
-  }
-#if defined(__APPLE__)
-  // This is macOS-specific work-around and makes no sense for any
-  // other host OS. See https://openradar.appspot.com/FB8914231
-  //
-  // The macOS kernel maintains a signature-verification cache to
-  // quickly validate applications at time of execve(2).  The trouble
-  // is that for the kernel creates the cache entry at the time of the
-  // mmap(2) call, before we have a chance to write either the code to
-  // sign or the signature header+hashes.  The fix is to invalidate
-  // all cached data associated with the output file, thus discarding
-  // the bogus prematurely-cached signature.
-  msync(buf, fileOff + getSize(), MS_INVALIDATE);
-#endif
+  OutputSegment *textSeg = getOrCreateOutputSegment(segment_names::text);
+  // NOTE: ld64 seems to also use outputFile instead of finalOutput
+  object::CodeSignatureSection sectionBuilder = object::CodeSignatureSection(
+      fileOff, config->outputFile, config->outputType, textSeg->fileOff,
+      textSeg->fileSize);
+  sectionBuilder.write(buf);
 }
 
 void CodeSignatureSection::writeTo(uint8_t *buf) const {
-  uint32_t signatureSize = static_cast<uint32_t>(getSize());
-  auto *superBlob = reinterpret_cast<CS_SuperBlob *>(buf);
-  write32be(&superBlob->magic, CSMAGIC_EMBEDDED_SIGNATURE);
-  write32be(&superBlob->length, signatureSize);
-  write32be(&superBlob->count, 1);
-  auto *blobIndex = reinterpret_cast<CS_BlobIndex *>(&superBlob[1]);
-  write32be(&blobIndex->type, CSSLOT_CODEDIRECTORY);
-  write32be(&blobIndex->offset, blobHeadersSize);
-  auto *codeDirectory =
-      reinterpret_cast<CS_CodeDirectory *>(buf + blobHeadersSize);
-  write32be(&codeDirectory->magic, CSMAGIC_CODEDIRECTORY);
-  write32be(&codeDirectory->length, signatureSize - blobHeadersSize);
-  write32be(&codeDirectory->version, CS_SUPPORTSEXECSEG);
-  write32be(&codeDirectory->flags, CS_ADHOC | CS_LINKER_SIGNED);
-  write32be(&codeDirectory->hashOffset,
-            sizeof(CS_CodeDirectory) + fileName.size() + fileNamePad);
-  write32be(&codeDirectory->identOffset, sizeof(CS_CodeDirectory));
-  codeDirectory->nSpecialSlots = 0;
-  write32be(&codeDirectory->nCodeSlots, getBlockCount());
-  write32be(&codeDirectory->codeLimit, fileOff);
-  codeDirectory->hashSize = static_cast<uint8_t>(hashSize);
-  codeDirectory->hashType = kSecCodeSignatureHashSHA256;
-  codeDirectory->platform = 0;
-  codeDirectory->pageSize = blockSizeShift;
-  codeDirectory->spare2 = 0;
-  codeDirectory->scatterOffset = 0;
-  codeDirectory->teamOffset = 0;
-  codeDirectory->spare3 = 0;
-  codeDirectory->codeLimit64 = 0;
-  OutputSegment *textSeg = getOrCreateOutputSegment(segment_names::text);
-  write64be(&codeDirectory->execSegBase, textSeg->fileOff);
-  write64be(&codeDirectory->execSegLimit, textSeg->fileSize);
-  write64be(&codeDirectory->execSegFlags,
-            config->outputType == MH_EXECUTE ? CS_EXECSEG_MAIN_BINARY : 0);
-  auto *id = reinterpret_cast<char *>(&codeDirectory[1]);
-  memcpy(id, fileName.begin(), fileName.size());
-  memset(id + fileName.size(), 0, fileNamePad);
+  // The entire code section including header is written
+  // in CodeSignatureSection::writeHashes above.
 }
 
 BitcodeBundleSection::BitcodeBundleSection()
     : SyntheticSection(segment_names::llvm, section_names::bitcodeBundle) {}
 
 class ErrorCodeWrapper {
 public:
   explicit ErrorCodeWrapper(std::error_code ec) : errorCode(ec.value()) {}

llvm/include/llvm/Object/MachO.h

 inline DataRefImpl DiceRef::getRawDataRefImpl() const {
   return DicePimpl;
 }
 
 inline const ObjectFile *DiceRef::getObjectFile() const {
   return OwningObject;
 }
 
+class CodeSignatureSection {
+public:
+  uint32_t getRawSize() const;
+  uint32_t getSize() const;
+
+  static constexpr int Align = 16;
+  static constexpr uint8_t BlockSizeShift = 12;
+  static constexpr size_t BlockSize = (1 << BlockSizeShift); // 4 KiB
+  static constexpr size_t HashSize = 256 / 8;
+  static constexpr size_t BlobHeadersSize =
+      alignTo<8>(sizeof(MachO::CS_SuperBlob) + sizeof(MachO::CS_BlobIndex));
+  static constexpr uint32_t FixedHeadersSize =
+      BlobHeadersSize + sizeof(MachO::CS_CodeDirectory);
+
+  CodeSignatureSection(uint64_t FileOff, StringRef OutputFilePath,
+                       MachO::HeaderFileType OutputFileType,
+                       uint64_t TextSegmentFileOff,
+                       uint64_t TextSegmentFileSize);
+
+  void write(uint8_t *Buf) const;
+
+private:
+  uint32_t getAllHeadersSize() const;
+  uint32_t getBlockCount() const;
+  uint32_t getFileNamePad() const;
+
+  StringRef stripOutputFilePath(const StringRef OutputFilePath);
+
+  // FileOff is the offset relative to the start of the file
+  // used to access the start of code signature section
+  // in __LINKEDIT segment
+  uint64_t FileOff;
+  StringRef OutputFileName;
+  MachO::HeaderFileType OutputFileType;
+  uint64_t TextSegmentFileOff;
+  uint64_t TextSegmentFileSize;
+};
+
 } // end namespace object
 } // end namespace llvm
 
 #endif // LLVM_OBJECT_MACHO_H

llvm/lib/Object/CMakeLists.txt

 add_llvm_component_library(LLVMObject
   Archive.cpp
   ArchiveWriter.cpp
   Binary.cpp
+  CodeSignatureSection.cpp
   COFFImportFile.cpp
   COFFModuleDefinition.cpp
   COFFObjectFile.cpp
   Decompressor.cpp
   ELF.cpp
   ELFObjectFile.cpp
   Error.cpp
   FaultMapParser.cpp

llvm/lib/Object/CodeSignatureSection.cpp

+//===- CodeSignatureSection.cpp - CodeSignatureSection class definition ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines the CodeSignatureSection class
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/BinaryFormat/MachO.h"
+#include "llvm/Object/MachO.h"
+#include "llvm/Support/Endian.h"
+#include "llvm/Support/SHA256.h"
+#include <cassert>
+
+#if defined(__APPLE__)
+#include <sys/mman.h>
+#endif
+
+using namespace llvm;
+using namespace object;
+using namespace support::endian;
+
+static_assert((CodeSignatureSection::BlobHeadersSize % 8) == 0, "");
+static_assert((CodeSignatureSection::FixedHeadersSize % 8) == 0, "");
+
+CodeSignatureSection::CodeSignatureSection(uint64_t FileOff,
+                                           StringRef OutputFilePath,
+                                           MachO::HeaderFileType OutputFileType,
+                                           uint64_t TextSegmentFileOff,
+                                           uint64_t TextSegmentFileSize)
+    : FileOff{FileOff}, OutputFileName{stripOutputFilePath(OutputFilePath)},
+      OutputFileType{OutputFileType}, TextSegmentFileOff{TextSegmentFileOff},
+      TextSegmentFileSize{TextSegmentFileSize} {}
+
+StringRef
+CodeSignatureSection::stripOutputFilePath(const StringRef OutputFilePath) {
+  const size_t LastSlashIndex = OutputFilePath.rfind("/");
+  if (LastSlashIndex == std::string::npos)
+    return OutputFilePath;
+
+  return OutputFilePath.drop_front(LastSlashIndex + 1);
+}
+
+uint32_t CodeSignatureSection::getAllHeadersSize() const {
+  return alignTo<Align>(FixedHeadersSize + OutputFileName.size() + 1);
+}
+
+uint32_t CodeSignatureSection::getBlockCount() const {
+  return (FileOff + BlockSize - 1) / BlockSize;
+}
+
+uint32_t CodeSignatureSection::getFileNamePad() const {
+  return getAllHeadersSize() - FixedHeadersSize - OutputFileName.size();
+}
+
+uint32_t CodeSignatureSection::getRawSize() const {
+  return getAllHeadersSize() + getBlockCount() * HashSize;
+}
+
+uint32_t CodeSignatureSection::getSize() const {
+  return alignTo<Align>(getRawSize());
+}
+
+void CodeSignatureSection::write(uint8_t *Buf) const {
+  const uint32_t AllHeadersSize = getAllHeadersSize();
+  const uint32_t BlockCount = getBlockCount();
+  const uint32_t FileNamePad = getFileNamePad();
+  const uint32_t Size = getSize();
+
+  uint8_t *Code = Buf;
+  uint8_t *CodeEnd = Buf + FileOff;
+  uint8_t *Hashes = CodeEnd + AllHeadersSize;
+
+  // Write code section header.
+  auto *SuperBlob = reinterpret_cast<MachO::CS_SuperBlob *>(CodeEnd);
+  write32be(&SuperBlob->magic, MachO::CSMAGIC_EMBEDDED_SIGNATURE);
+  write32be(&SuperBlob->length, Size);
+  write32be(&SuperBlob->count, 1);
+  auto *BlobIndex = reinterpret_cast<MachO::CS_BlobIndex *>(&SuperBlob[1]);
+  write32be(&BlobIndex->type, MachO::CSSLOT_CODEDIRECTORY);
+  write32be(&BlobIndex->offset, BlobHeadersSize);
+  auto *CodeDirectory =
+      reinterpret_cast<MachO::CS_CodeDirectory *>(CodeEnd + BlobHeadersSize);
+  write32be(&CodeDirectory->magic, MachO::CSMAGIC_CODEDIRECTORY);
+  write32be(&CodeDirectory->length, Size - BlobHeadersSize);
+  write32be(&CodeDirectory->version, MachO::CS_SUPPORTSEXECSEG);
+  write32be(&CodeDirectory->flags, MachO::CS_ADHOC | MachO::CS_LINKER_SIGNED);
+  write32be(&CodeDirectory->hashOffset, sizeof(MachO::CS_CodeDirectory) +
+                                            OutputFileName.size() +
+                                            FileNamePad);
+  write32be(&CodeDirectory->identOffset, sizeof(MachO::CS_CodeDirectory));
+  CodeDirectory->nSpecialSlots = 0;
+  write32be(&CodeDirectory->nCodeSlots, BlockCount);
+  write32be(&CodeDirectory->codeLimit, FileOff);
+  CodeDirectory->hashSize = static_cast<uint8_t>(HashSize);
+  CodeDirectory->hashType = MachO::kSecCodeSignatureHashSHA256;
+  CodeDirectory->platform = 0;
+  CodeDirectory->pageSize = BlockSizeShift;
+  CodeDirectory->spare2 = 0;
+  CodeDirectory->scatterOffset = 0;
+  CodeDirectory->teamOffset = 0;
+  CodeDirectory->spare3 = 0;
+  CodeDirectory->codeLimit64 = 0;
+  write64be(&CodeDirectory->execSegBase, TextSegmentFileOff);
+  write64be(&CodeDirectory->execSegLimit, TextSegmentFileSize);
+  write64be(&CodeDirectory->execSegFlags, OutputFileType == MachO::MH_EXECUTE
+                                              ? MachO::CS_EXECSEG_MAIN_BINARY
+                                              : 0);
+  auto *Id = reinterpret_cast<char *>(&CodeDirectory[1]);
+  memcpy(Id, OutputFileName.begin(), OutputFileName.size());
+  memset(Id + OutputFileName.size(), 0, FileNamePad);
+
+  // Write code section signature.
+  while (Code < CodeEnd) {
+    StringRef Block(reinterpret_cast<char *>(Code),
+                    std::min(CodeEnd - Code, static_cast<ssize_t>(BlockSize)));
+    SHA256 Hasher;
+    Hasher.update(Block);
+    StringRef Hash = Hasher.final();
+    assert(Hash.size() == HashSize);
+    memcpy(Hashes, Hash.data(), HashSize);
+    Code += BlockSize;
+    Hashes += HashSize;
+  }
+#if defined(__APPLE__)
+  // This is macOS-specific work-around and makes no sense for any
+  // other host OS. See https://openradar.appspot.com/FB8914231
+  //
+  // The macOS kernel maintains a signature-verification cache to
+  // quickly validate applications at time of execve(2).  The trouble
+  // is that for the kernel creates the cache entry at the time of the
+  // mmap(2) call, before we have a chance to write either the code to
+  // sign or the signature header+hashes.  The fix is to invalidate
+  // all cached data associated with the output file, thus discarding
+  // the bogus prematurely-cached signature.
+  msync(Buf, FileOff + Size, MS_INVALIDATE);
+#endif
+}