Skip to content

Commit

Permalink
[ELF] .note.gnu.property: error for invalid pr_datasize
Browse files Browse the repository at this point in the history
A n_type==NT_GNU_PROPERTY_TYPE_0 note encodes a program property.
If pr_datasize is invalid, LLD may crash
(ClangBuiltLinux/linux#1141)

This patch adds some error checking, supports big-endian, and add some tests
for invalid n_descsz.

Differential Revision: https://reviews.llvm.org/D86422
  • Loading branch information
MaskRay committed Aug 25, 2020
1 parent 8a1926c commit 25863cc
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 20 deletions.
41 changes: 21 additions & 20 deletions lld/ELF/InputFiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -780,20 +780,21 @@ static void updateSupportedARMFeatures(const ARMAttributeParser &attributes) {
// of zero or more type-length-value fields. We want to find a field of a
// certain type. It seems a bit too much to just store a 32-bit value, perhaps
// the ABI is unnecessarily complicated.
template <class ELFT>
static uint32_t readAndFeatures(ObjFile<ELFT> *obj, ArrayRef<uint8_t> data) {
template <class ELFT> static uint32_t readAndFeatures(const InputSection &sec) {
using Elf_Nhdr = typename ELFT::Nhdr;
using Elf_Note = typename ELFT::Note;

uint32_t featuresSet = 0;
ArrayRef<uint8_t> data = sec.data();
auto reportFatal = [&](const uint8_t *place, const char *msg) {
fatal(toString(sec.file) + ":(" + sec.name + "+0x" +
Twine::utohexstr(place - sec.data().data()) + "): " + msg);
};
while (!data.empty()) {
// Read one NOTE record.
if (data.size() < sizeof(Elf_Nhdr))
fatal(toString(obj) + ": .note.gnu.property: section too short");

auto *nhdr = reinterpret_cast<const Elf_Nhdr *>(data.data());
if (data.size() < nhdr->getSize())
fatal(toString(obj) + ": .note.gnu.property: section too short");
if (data.size() < sizeof(Elf_Nhdr) || data.size() < nhdr->getSize())
reportFatal(data.data(), "data is too short");

Elf_Note note(*nhdr);
if (nhdr->n_type != NT_GNU_PROPERTY_TYPE_0 || note.getName() != "GNU") {
Expand All @@ -808,25 +809,26 @@ static uint32_t readAndFeatures(ObjFile<ELFT> *obj, ArrayRef<uint8_t> data) {
// Read a body of a NOTE record, which consists of type-length-value fields.
ArrayRef<uint8_t> desc = note.getDesc();
while (!desc.empty()) {
const uint8_t *place = desc.data();
if (desc.size() < 8)
fatal(toString(obj) + ": .note.gnu.property: section too short");

uint32_t type = read32le(desc.data());
uint32_t size = read32le(desc.data() + 4);
reportFatal(place, "program property is too short");
uint32_t type = read32<ELFT::TargetEndianness>(desc.data());
uint32_t size = read32<ELFT::TargetEndianness>(desc.data() + 4);
desc = desc.slice(8);
if (desc.size() < size)
reportFatal(place, "program property is too short");

if (type == featureAndType) {
// We found a FEATURE_1_AND field. There may be more than one of these
// in a .note.gnu.property section, for a relocatable object we
// accumulate the bits set.
featuresSet |= read32le(desc.data() + 8);
if (size < 4)
reportFatal(place, "FEATURE_1_AND entry is too short");
featuresSet |= read32<ELFT::TargetEndianness>(desc.data());
}

// On 64-bit, a payload may be followed by a 4-byte padding to make its
// size a multiple of 8.
if (ELFT::Is64Bits)
size = alignTo(size, 8);

desc = desc.slice(size + 8); // +8 for Type and Size
// Padding is present in the note descriptor, if necessary.
desc = desc.slice(alignTo<(ELFT::Is64Bits ? 8 : 4)>(size));
}

// Go to next NOTE record to look for more FEATURE_1_AND descriptions.
Expand Down Expand Up @@ -985,8 +987,7 @@ InputSectionBase *ObjFile<ELFT>::createInputSection(const Elf_Shdr &sec) {
// .note.gnu.property containing a single AND'ed bitmap, we discard an input
// file's .note.gnu.property section.
if (name == ".note.gnu.property") {
ArrayRef<uint8_t> contents = check(this->getObj().getSectionContents(&sec));
this->andFeatures = readAndFeatures(this, contents);
this->andFeatures = readAndFeatures<ELFT>(InputSection(*this, sec, name));
return &InputSection::discarded;
}

Expand Down
55 changes: 55 additions & 0 deletions lld/test/ELF/gnu-property-err.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# REQUIRES: aarch64
# RUN: split-file %s %t

# RUN: llvm-mc -filetype=obj -triple=aarch64 %t/1.s -o %t1.o
# RUN: not ld.lld %t1.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR1

# ERR1: error: {{.*}}.o:(.note.gnu.property+0x0): data is too short

# RUN: llvm-mc -filetype=obj -triple=aarch64 %t/2.s -o %t2.o
# RUN: not ld.lld %t2.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR2
# RUN: llvm-mc -filetype=obj -triple=aarch64_be %t/2.s -o %t2be.o
# RUN: not ld.lld %t2be.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR2

# ERR2: error: {{.*}}.o:(.note.gnu.property+0x10): program property is too short

# RUN: llvm-mc -filetype=obj -triple=aarch64 %t/3.s -o %t3.o
# RUN: not ld.lld %t3.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR3
# RUN: llvm-mc -filetype=obj -triple=aarch64_be %t/3.s -o %t3be.o
# RUN: not ld.lld %t3be.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR3

# ERR3: error: {{.*}}.o:(.note.gnu.property+0x10): FEATURE_1_AND entry is too short

#--- 1.s
.section ".note.gnu.property", "a"
.long 4
.long 17 // n_descsz too long
.long 5 // NT_GNU_PROPERTY_TYPE_0
.asciz "GNU"

.long 0xc0000000 // GNU_PROPERTY_AARCH64_FEATURE_1_AND
.long 4 // pr_datasz
.long 1 // GNU_PROPERTY_AARCH64_FEATURE_1_BTI
.long 0

#--- 2.s
.section ".note.gnu.property", "a"
.long 4
.long 16 // n_descsz
.long 5 // NT_GNU_PROPERTY_TYPE_0
.asciz "GNU"

.long 0xc0000000 // GNU_PROPERTY_AARCH64_FEATURE_1_AND
.long 9 // pr_datasz too long
.long 1 // GNU_PROPERTY_AARCH64_FEATURE_1_BTI
.long 0

#--- 3.s
.section ".note.gnu.property", "a"
.long 4
.long 8 // n_descsz
.long 5 // NT_GNU_PROPERTY_TYPE_0
.asciz "GNU"

.long 0xc0000000 // GNU_PROPERTY_AARCH64_FEATURE_1_AND
.long 0 // pr_datasz too short

0 comments on commit 25863cc

Please sign in to comment.