[llvm] [llvm-objdump] Error with relevant message when adding invalid notes (PR #90458)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 29 04:35:16 PDT 2024
https://github.com/serge-sans-paille created https://github.com/llvm/llvm-project/pull/90458
None
>From 2aaf91d8e329c5951f3ef8861c8f3322a37c5fe4 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Mon, 29 Apr 2024 13:27:04 +0200
Subject: [PATCH] [llvm-objdump] Error with relevant message when adding
invalid notes
---
llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp | 70 +++++++++++++++++--
.../llvm-objcopy/ELF/add-invalid-note.test | 35 ++++++++++
2 files changed, 99 insertions(+), 6 deletions(-)
create mode 100644 llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
index f343d1447e0554..90e59a5228d6fe 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
@@ -623,6 +623,58 @@ handleUserSection(const NewSectionInfo &NewSection,
return F(NewSection.SectionName, Data);
}
+static Error verifyNoteSection(StringRef Name, llvm::endianness E,
+ ArrayRef<uint8_t> Data) {
+
+// An ELF note have the following structure:
+// Name Size: 4 bytes (integer)
+// Desc Size: 4 bytes (integer)
+// Type : 4 bytes
+// Name : variable size, padded to a 4 byte boundary
+// Desc : variable size, padded to a 4 byte boundary
+
+ if (Data.empty())
+ return Error::success();
+
+ if (Data.size() < 12) {
+ std::string msg;
+ raw_string_ostream(msg)
+ << Name << " data must be either empty or at least 12 bytes long.";
+ return createStringError(errc::invalid_argument, msg);
+ }
+ if (Data.size() % 4 != 0) {
+ std::string msg;
+ raw_string_ostream(msg)
+ << Name << " data size must be a multiple of 4 bytes.";
+ return createStringError(errc::invalid_argument, msg);
+ }
+ ArrayRef<uint8_t> NameSize = Data.slice(0, 4);
+ ArrayRef<uint8_t> DescSize = Data.slice(4, 4);
+
+ uint32_t NameSizeValue, DescSizeValue;
+ std::memcpy(&NameSizeValue, NameSize.data(), 4);
+ std::memcpy(&DescSizeValue, DescSize.data(), 4);
+
+ if (llvm::endianness::native != E) {
+ NameSizeValue = byteswap(NameSizeValue);
+ DescSizeValue = byteswap(DescSizeValue);
+ }
+ uint64_t ExpectedDataSize =
+ 4 + 4 + 4 + (NameSizeValue + 3) / 4 * 4 + (DescSizeValue + 3) / 4 * 4;
+ uint64_t ActualDataSize = Data.size();
+ if (ActualDataSize != ExpectedDataSize) {
+ std::string msg;
+ raw_string_ostream(msg) << Name
+ << " data size is incompatible with the content of "
+ "the name and description size fields:"
+ << " expecting " << ExpectedDataSize << ", found "
+ << ActualDataSize << ".";
+ return createStringError(errc::invalid_argument, msg);
+ }
+
+ return Error::success();
+}
+
// This function handles the high level operations of GNU objcopy including
// handling command line options. It's important to outline certain properties
// we expect to hold of the command line operations. Any operation that "keeps"
@@ -631,7 +683,7 @@ handleUserSection(const NewSectionInfo &NewSection,
// depend a) on the order the options occur in or b) on some opaque priority
// system. The only priority is that keeps/copies overrule removes.
static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig,
- Object &Obj) {
+ ElfType OutputElfType, Object &Obj) {
if (Config.OutputArch) {
Obj.Machine = Config.OutputArch->EMachine;
Obj.OSABI = Config.OutputArch->OSABI;
@@ -675,12 +727,18 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig,
if (Sec.Flags & SHF_ALLOC && Sec.Type != SHT_NOTE)
Sec.Type = SHT_NOBITS;
+ endianness E = OutputElfType == ELFT_ELF32LE || OutputElfType == ELFT_ELF64LE
+ ? endianness::little
+ : endianness::big;
+
for (const NewSectionInfo &AddedSection : Config.AddSection) {
- auto AddSection = [&](StringRef Name, ArrayRef<uint8_t> Data) {
+ auto AddSection = [&](StringRef Name, ArrayRef<uint8_t> Data) -> Error {
OwnedDataSection &NewSection =
Obj.addSection<OwnedDataSection>(Name, Data);
- if (Name.starts_with(".note") && Name != ".note.GNU-stack")
+ if (Name.starts_with(".note") && Name != ".note.GNU-stack") {
NewSection.Type = SHT_NOTE;
+ return verifyNoteSection(Name, E, Data);
+ }
return Error::success();
};
if (Error E = handleUserSection(AddedSection, AddSection))
@@ -813,7 +871,7 @@ Error objcopy::elf::executeObjcopyOnIHex(const CommonConfig &Config,
const ElfType OutputElfType =
getOutputElfType(Config.OutputArch.value_or(MachineInfo()));
- if (Error E = handleArgs(Config, ELFConfig, **Obj))
+ if (Error E = handleArgs(Config, ELFConfig, OutputElfType, **Obj))
return E;
return writeOutput(Config, **Obj, Out, OutputElfType);
}
@@ -831,7 +889,7 @@ Error objcopy::elf::executeObjcopyOnRawBinary(const CommonConfig &Config,
// (-B<arch>).
const ElfType OutputElfType =
getOutputElfType(Config.OutputArch.value_or(MachineInfo()));
- if (Error E = handleArgs(Config, ELFConfig, **Obj))
+ if (Error E = handleArgs(Config, ELFConfig, OutputElfType, **Obj))
return E;
return writeOutput(Config, **Obj, Out, OutputElfType);
}
@@ -850,7 +908,7 @@ Error objcopy::elf::executeObjcopyOnBinary(const CommonConfig &Config,
? getOutputElfType(*Config.OutputArch)
: getOutputElfType(In);
- if (Error E = handleArgs(Config, ELFConfig, **Obj))
+ if (Error E = handleArgs(Config, ELFConfig, OutputElfType, **Obj))
return createFileError(Config.InputFilename, std::move(E));
if (Error E = writeOutput(Config, **Obj, Out, OutputElfType))
diff --git a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
new file mode 100644
index 00000000000000..396b6ebaaaf4e5
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
@@ -0,0 +1,35 @@
+# Verify that --add-section can be used to add a note section which is
+# successfully interpreted by tools that read notes.
+
+# Add [namesz, descsz, type, name, desc] for a build id.
+#
+# RUN: echo -e -n "\x04\x00\x00\x00" > %t-miss-padding-note.bin
+# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-miss-padding-note.bin
+# RUN: echo -e -n "\x03\x00\x00\x00" >> %t-miss-padding-note.bin
+# RUN: echo -e -n "GNU\x00" >> %t-miss-padding-note.bin
+# RUN: echo -e -n "\x0c\x0d\x0e" >> %t-miss-padding-note.bin
+#
+# RUN: echo -e -n "\x08\x00\x00\x00" > %t-invalid-size-note.bin
+# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-invalid-size-note.bin
+# RUN: echo -e -n "\x03\x00\x00\x00" >> %t-invalid-size-note.bin
+# RUN: echo -e -n "GNU\x00" >> %t-invalid-size-note.bin
+# RUN: echo -e -n "\x0c\x0d\x0e\x00" >> %t-invalid-size-note.bin
+
+# RUN: echo -e -n "\x08\x00\x00\x00" > %t-short-note.bin
+# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-short-note.bin
+
+# RUN: yaml2obj %s -o %t.o
+# RUN: not llvm-objcopy --add-section=.note.miss.padding=%t-miss-padding-note.bin %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-MISS-PADDING %s
+# RUN: not llvm-objcopy --add-section=.note.invalid.size=%t-invalid-size-note.bin %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-INVALID-SIZE %s
+# RUN: not llvm-objcopy --add-section=.note.short=%t-short-note.bin %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-SHORT %s
+
+!ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_REL
+ Machine: EM_X86_64
+
+# CHECK-MISS-PADDING: .note.miss.padding data size must be a multiple of 4 bytes.
+# CHECK-INVALID-SIZE: .note.invalid.size data size is incompatible with the content of the name and description size fields: expecting 28, found 20.
+# CHECK-SHORT: .note.short data must be either empty or at least 12 bytes long.
More information about the llvm-commits
mailing list