Skip to content

Commit 689715f

Browse files
MaskRayMaskRay
committed
[Object] Fix handling of Elf_Nhdr with sh_addralign=8
The generic ABI says: > Padding is present, if necessary, to ensure 8 or 4-byte alignment for the next note entry (depending on whether the file is a 64-bit or 32-bit object). Such padding is not included in descsz. Our parsing code currently aligns n_namesz. Fix the bug by aligning the start offset of the descriptor instead. This issue has been benign because the primary uses of sh_addralign=8 notes are `.note.gnu.property`, where `sizeof(Elf_Nhdr) + sizeof("GNU") = 16` (already aligned by 8). In practice, many 64-bit systems incorrectly use sh_addralign=4 notes. We can use sh_addralign (= p_align) to decide the descriptor padding. Treat an alignment of 0 and 1 as 4. This approach matches modern GNU readelf (since 2018). We have a few tests incorrectly using sh_addralign=0. We may make our behavior stricter after fixing these tests. Linux kernel dumped core files use `p_align=0` notes, so we need to support the case for compatibility. Reviewed By: jhenderson Differential Revision: https://reviews.llvm.org/D150022
1 parent ae63d5b commit 689715f

File tree

11 files changed

+136
-50
lines changed

11 files changed

+136
-50
lines changed

‎lld/ELF/InputFiles.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -885,12 +885,13 @@ template <class ELFT> static uint32_t readAndFeatures(const InputSection &sec) {
885885
while (!data.empty()) {
886886
// Read one NOTE record.
887887
auto *nhdr = reinterpret_cast<const Elf_Nhdr *>(data.data());
888-
if (data.size() < sizeof(Elf_Nhdr) || data.size() < nhdr->getSize())
888+
if (data.size() < sizeof(Elf_Nhdr) ||
889+
data.size() < nhdr->getSize(sec.addralign))
889890
reportFatal(data.data(), "data is too short");
890891

891892
Elf_Note note(*nhdr);
892893
if (nhdr->n_type != NT_GNU_PROPERTY_TYPE_0 || note.getName() != "GNU") {
893-
data = data.slice(nhdr->getSize());
894+
data = data.slice(nhdr->getSize(sec.addralign));
894895
continue;
895896
}
896897

@@ -899,7 +900,7 @@ template <class ELFT> static uint32_t readAndFeatures(const InputSection &sec) {
899900
: GNU_PROPERTY_X86_FEATURE_1_AND;
900901

901902
// Read a body of a NOTE record, which consists of type-length-value fields.
902-
ArrayRef<uint8_t> desc = note.getDesc();
903+
ArrayRef<uint8_t> desc = note.getDesc(sec.addralign);
903904
while (!desc.empty()) {
904905
const uint8_t *place = desc.data();
905906
if (desc.size() < 8)
@@ -924,7 +925,7 @@ template <class ELFT> static uint32_t readAndFeatures(const InputSection &sec) {
924925
}
925926

926927
// Go to next NOTE record to look for more FEATURE_1_AND descriptions.
927-
data = data.slice(nhdr->getSize());
928+
data = data.slice(nhdr->getSize(sec.addralign));
928929
}
929930

930931
return featuresSet;

‎llvm/include/llvm/Object/ELF.h

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,16 @@ class ELFFile {
316316
") or size (0x" + Twine::utohexstr(Phdr.p_filesz) + ")");
317317
return Elf_Note_Iterator(Err);
318318
}
319-
return Elf_Note_Iterator(base() + Phdr.p_offset, Phdr.p_filesz, Err);
319+
// Allow 4, 8, and (for Linux core dumps) 0.
320+
// TODO: Disallow 1 after all tests are fixed.
321+
if (Phdr.p_align != 0 && Phdr.p_align != 1 && Phdr.p_align != 4 &&
322+
Phdr.p_align != 8) {
323+
Err =
324+
createError("alignment (" + Twine(Phdr.p_align) + ") is not 4 or 8");
325+
return Elf_Note_Iterator(Err);
326+
}
327+
return Elf_Note_Iterator(base() + Phdr.p_offset, Phdr.p_filesz,
328+
std::max<size_t>(Phdr.p_align, 4), Err);
320329
}
321330

322331
/// Get an iterator over notes in a section.
@@ -335,7 +344,15 @@ class ELFFile {
335344
") or size (0x" + Twine::utohexstr(Shdr.sh_size) + ")");
336345
return Elf_Note_Iterator(Err);
337346
}
338-
return Elf_Note_Iterator(base() + Shdr.sh_offset, Shdr.sh_size, Err);
347+
// TODO: Allow just 4 and 8 after all tests are fixed.
348+
if (Shdr.sh_addralign != 0 && Shdr.sh_addralign != 1 &&
349+
Shdr.sh_addralign != 4 && Shdr.sh_addralign != 8) {
350+
Err = createError("alignment (" + Twine(Shdr.sh_addralign) +
351+
") is not 4 or 8");
352+
return Elf_Note_Iterator(Err);
353+
}
354+
return Elf_Note_Iterator(base() + Shdr.sh_offset, Shdr.sh_size,
355+
std::max<size_t>(Shdr.sh_addralign, 4), Err);
339356
}
340357

341358
/// Get the end iterator for notes.

‎llvm/include/llvm/Object/ELFTypes.h

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -599,15 +599,13 @@ struct Elf_Nhdr_Impl {
599599
Elf_Word n_descsz;
600600
Elf_Word n_type;
601601

602-
/// The alignment of the name and descriptor.
603-
///
604-
/// Implementations differ from the specification here: in practice all
605-
/// variants align both the name and descriptor to 4-bytes.
606-
static const unsigned int Align = 4;
607-
608-
/// Get the size of the note, including name, descriptor, and padding.
609-
size_t getSize() const {
610-
return sizeof(*this) + alignTo<Align>(n_namesz) + alignTo<Align>(n_descsz);
602+
/// Get the size of the note, including name, descriptor, and padding. Both
603+
/// the start and the end of the descriptor are aligned by the section
604+
/// alignment. In practice many 64-bit systems deviate from the generic ABI by
605+
/// using sh_addralign=4.
606+
size_t getSize(size_t Align) const {
607+
return alignToPowerOf2(sizeof(*this) + n_namesz, Align) +
608+
alignToPowerOf2(n_descsz, Align);
611609
}
612610
};
613611

@@ -635,18 +633,18 @@ class Elf_Note_Impl {
635633
}
636634

637635
/// Get the note's descriptor.
638-
ArrayRef<uint8_t> getDesc() const {
636+
ArrayRef<uint8_t> getDesc(size_t Align) const {
639637
if (!Nhdr.n_descsz)
640638
return ArrayRef<uint8_t>();
641639
return ArrayRef<uint8_t>(
642-
reinterpret_cast<const uint8_t *>(&Nhdr) + sizeof(Nhdr) +
643-
alignTo<Elf_Nhdr_Impl<ELFT>::Align>(Nhdr.n_namesz),
640+
reinterpret_cast<const uint8_t *>(&Nhdr) +
641+
alignToPowerOf2(sizeof(Nhdr) + Nhdr.n_namesz, Align),
644642
Nhdr.n_descsz);
645643
}
646644

647645
/// Get the note's descriptor as StringRef
648-
StringRef getDescAsStringRef() const {
649-
ArrayRef<uint8_t> Desc = getDesc();
646+
StringRef getDescAsStringRef(size_t Align) const {
647+
ArrayRef<uint8_t> Desc = getDesc(Align);
650648
return StringRef(reinterpret_cast<const char *>(Desc.data()), Desc.size());
651649
}
652650

@@ -666,6 +664,7 @@ template <class ELFT> class Elf_Note_Iterator_Impl {
666664
// Nhdr being a nullptr marks the end of iteration.
667665
const Elf_Nhdr_Impl<ELFT> *Nhdr = nullptr;
668666
size_t RemainingSize = 0u;
667+
size_t Align = 0;
669668
Error *Err = nullptr;
670669

671670
template <class ELFFileELFT> friend class ELFFile;
@@ -693,7 +692,7 @@ template <class ELFT> class Elf_Note_Iterator_Impl {
693692
stopWithOverflowError();
694693
else {
695694
Nhdr = reinterpret_cast<const Elf_Nhdr_Impl<ELFT> *>(NhdrPos + NoteSize);
696-
if (Nhdr->getSize() > RemainingSize)
695+
if (Nhdr->getSize(Align) > RemainingSize)
697696
stopWithOverflowError();
698697
else
699698
*Err = Error::success();
@@ -702,8 +701,9 @@ template <class ELFT> class Elf_Note_Iterator_Impl {
702701

703702
Elf_Note_Iterator_Impl() = default;
704703
explicit Elf_Note_Iterator_Impl(Error &Err) : Err(&Err) {}
705-
Elf_Note_Iterator_Impl(const uint8_t *Start, size_t Size, Error &Err)
706-
: RemainingSize(Size), Err(&Err) {
704+
Elf_Note_Iterator_Impl(const uint8_t *Start, size_t Size, size_t Align,
705+
Error &Err)
706+
: RemainingSize(Size), Align(Align), Err(&Err) {
707707
consumeError(std::move(Err));
708708
assert(Start && "ELF note iterator starting at NULL");
709709
advanceNhdr(Start, 0u);
@@ -713,7 +713,7 @@ template <class ELFT> class Elf_Note_Iterator_Impl {
713713
Elf_Note_Iterator_Impl &operator++() {
714714
assert(Nhdr && "incremented ELF note end iterator");
715715
const uint8_t *NhdrPos = reinterpret_cast<const uint8_t *>(Nhdr);
716-
size_t NoteSize = Nhdr->getSize();
716+
size_t NoteSize = Nhdr->getSize(Align);
717717
advanceNhdr(NhdrPos, NoteSize);
718718
return *this;
719719
}

‎llvm/lib/Object/BuildID.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ template <typename ELFT> BuildIDRef getBuildID(const ELFFile<ELFT> &Obj) {
3636
for (auto N : Obj.notes(P, Err))
3737
if (N.getType() == ELF::NT_GNU_BUILD_ID &&
3838
N.getName() == ELF::ELF_NOTE_GNU)
39-
return N.getDesc();
39+
return N.getDesc(P.p_align);
4040
consumeError(std::move(Err));
4141
}
4242
return {};

‎llvm/test/tools/llvm-readobj/ELF/AArch64/aarch64-note-gnu-property.s

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
// LLVM-NEXT: ]
2525

2626
.section ".note.gnu.property", "a"
27-
.align 4
2827
.long 4 /* Name length is always 4 ("GNU") */
2928
.long end - begin /* Data length */
3029
.long 5 /* Type: NT_GNU_PROPERTY_TYPE_0 */

‎llvm/test/tools/llvm-readobj/ELF/note-alignment-invalid.test

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# RUN: yaml2obj --docnum=1 %s -o %t1.so
2+
# RUN: llvm-readelf --notes %t1.so 2>&1 | FileCheck %s -DFILE=%t1.so
3+
# RUN: llvm-readobj --notes %t1.so 2>&1 | FileCheck %s -DFILE=%t1.so
4+
5+
# CHECK: warning: '[[FILE]]': unable to read notes from the SHT_NOTE section with index 1: alignment (6) is not 4 or 8
6+
7+
--- !ELF
8+
FileHeader:
9+
Class: ELFCLASS64
10+
Data: ELFDATA2LSB
11+
Type: ET_EXEC
12+
Sections:
13+
- Name: .note.invalid
14+
Type: SHT_NOTE
15+
AddressAlign: 0x0000000000000006
16+
Content: 0400000004000000cdab0000474E550061626364
17+
ProgramHeaders:
18+
- Type: PT_NOTE
19+
FileSize: 0x20
20+
FirstSec: .note.invalid
21+
LastSec: .note.invalid

‎llvm/test/tools/llvm-readobj/ELF/note-gnu-property2.s

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66
// GNU: Displaying notes found in: .note.gnu.property
77
// GNU-NEXT: Owner Data size Description
88
// GNU-NEXT: GNU 0x00000004 NT_GNU_PROPERTY_TYPE_0 (property note)
9-
// GNU-NEXT: Properties: <corrupted GNU_PROPERTY_TYPE_0>
9+
// GNU-NEXT: Properties: <corrupted GNU_PROPERTY_TYPE_0>
1010

1111
// LLVM: Notes [
1212
// LLVM-NEXT: NoteSection {
1313
// LLVM-NEXT: Name: .note.gnu.property
1414
// LLVM-NEXT: Offset: 0x40
15-
// LLVM-NEXT: Size: 0x14
15+
// LLVM-NEXT: Size: 0x18
1616
// LLVM-NEXT: Note {
1717
// LLVM-NEXT: Owner: GNU
1818
// LLVM-NEXT: Data size: 0x4
@@ -29,10 +29,11 @@
2929
.section ".note.gnu.property", "a"
3030
.align 4
3131
.long 4 /* Name length is always 4 ("GNU") */
32-
.long end - begin /* Data length */
32+
.long 4 /* Data length (corrupted) */
3333
.long 5 /* Type: NT_GNU_PROPERTY_TYPE_0 */
3434
.asciz "GNU" /* Name */
3535
.p2align 3
3636
begin:
3737
.long 1 /* Type: GNU_PROPERTY_STACK_SIZE */
38+
.p2align 3
3839
end:

‎llvm/test/tools/llvm-readobj/ELF/note-unknown.s

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,63 @@
3939
// LLVM-NEXT: )
4040
// LLVM-NEXT: }
4141
// LLVM-NEXT: }
42+
// LLVM-NEXT: NoteSection {
43+
// LLVM-NEXT: Name: .note.8
44+
// LLVM-NEXT: Offset: 0x80
45+
// LLVM-NEXT: Size: 0x40
46+
// LLVM-NEXT: Note {
47+
// LLVM-NEXT: Owner: WXYZ
48+
// LLVM-NEXT: Data size: 0x8
49+
// LLVM-NEXT: Type: Unknown (0x00000006)
50+
// LLVM-NEXT: Description data (
51+
// LLVM-NEXT: 0000: 4C6F7265 6D000000 |Lorem...|
52+
// LLVM-NEXT: )
53+
// LLVM-NEXT: }
54+
// LLVM-NEXT: Note {
55+
// LLVM-NEXT: Owner: VWXYZ
56+
// LLVM-NEXT: Data size: 0x8
57+
// LLVM-NEXT: Type: Unknown (0x00000006)
58+
// LLVM-NEXT: Description data (
59+
// LLVM-NEXT: 0000: 78787800 00000000 |xxx.....|
60+
// LLVM-NEXT: )
61+
// LLVM-NEXT: }
62+
// LLVM-NEXT: }
4263
// LLVM-NEXT: ]
4364

4465
.section ".note.foo", "a"
45-
.align 4
4666
.long 4 /* namesz */
4767
.long 0 /* descsz */
4868
.long 3 /* type */
4969
.asciz "XYZ"
70+
.align 4
5071
.section ".note.bar", "a"
51-
.align 4
5272
.long 4 /* namesz */
5373
.long end - begin /* descsz */
5474
.long 3 /* type */
5575
.asciz "XYZ"
76+
.align 4
5677
begin:
5778
.asciz "Lorem ipsum dolor sit amet"
5879
.align 4
5980
end:
81+
82+
.section ".note.8", "a"
83+
.long 5 /* namesz */
84+
.long 2f - 1f /* descsz */
85+
.long 6 /* type */
86+
.asciz "WXYZ"
87+
.align 8
88+
1:
89+
.asciz "Lorem"
90+
.align 8
91+
2:
92+
93+
.long 6 /* namesz */
94+
.long 2f - 1f /* descsz */
95+
.long 6 /* type */
96+
.asciz "VWXYZ"
97+
.align 8
98+
1:
99+
.asciz "xxx"
100+
.align 8
101+
2:

‎llvm/tools/llvm-readobj/ELFDumper.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5828,7 +5828,7 @@ template <class ELFT>
58285828
static void processNotesHelper(
58295829
const ELFDumper<ELFT> &Dumper,
58305830
llvm::function_ref<void(std::optional<StringRef>, typename ELFT::Off,
5831-
typename ELFT::Addr)>
5831+
typename ELFT::Addr, size_t)>
58325832
StartNotesFn,
58335833
llvm::function_ref<Error(const typename ELFT::Note &, bool)> ProcessNoteFn,
58345834
llvm::function_ref<void()> FinishNotesFn) {
@@ -5841,7 +5841,7 @@ static void processNotesHelper(
58415841
if (S.sh_type != SHT_NOTE)
58425842
continue;
58435843
StartNotesFn(expectedToStdOptional(Obj.getSectionName(S)), S.sh_offset,
5844-
S.sh_size);
5844+
S.sh_size, S.sh_addralign);
58455845
Error Err = Error::success();
58465846
size_t I = 0;
58475847
for (const typename ELFT::Note Note : Obj.notes(S, Err)) {
@@ -5872,7 +5872,7 @@ static void processNotesHelper(
58725872
const typename ELFT::Phdr &P = (*PhdrsOrErr)[I];
58735873
if (P.p_type != PT_NOTE)
58745874
continue;
5875-
StartNotesFn(/*SecName=*/std::nullopt, P.p_offset, P.p_filesz);
5875+
StartNotesFn(/*SecName=*/std::nullopt, P.p_offset, P.p_filesz, P.p_align);
58765876
Error Err = Error::success();
58775877
size_t Index = 0;
58785878
for (const typename ELFT::Note Note : Obj.notes(P, Err)) {
@@ -5892,10 +5892,12 @@ static void processNotesHelper(
58925892
}
58935893

58945894
template <class ELFT> void GNUELFDumper<ELFT>::printNotes() {
5895+
size_t Align = 0;
58955896
bool IsFirstHeader = true;
58965897
auto PrintHeader = [&](std::optional<StringRef> SecName,
58975898
const typename ELFT::Off Offset,
5898-
const typename ELFT::Addr Size) {
5899+
const typename ELFT::Addr Size, size_t Al) {
5900+
Align = std::max<size_t>(Al, 4);
58995901
// Print a newline between notes sections to match GNU readelf.
59005902
if (!IsFirstHeader) {
59015903
OS << '\n';
@@ -5916,7 +5918,7 @@ template <class ELFT> void GNUELFDumper<ELFT>::printNotes() {
59165918

59175919
auto ProcessNote = [&](const Elf_Note &Note, bool IsCore) -> Error {
59185920
StringRef Name = Note.getName();
5919-
ArrayRef<uint8_t> Descriptor = Note.getDesc();
5921+
ArrayRef<uint8_t> Descriptor = Note.getDesc(Align);
59205922
Elf_Word Type = Note.getType();
59215923

59225924
// Print the note owner/type.
@@ -6048,15 +6050,15 @@ template <typename ELFT> void ELFDumper<ELFT>::printMemtag() {
60486050
auto FindAndroidNote = [&](const Elf_Note &Note, bool IsCore) -> Error {
60496051
if (Note.getName() == "Android" &&
60506052
Note.getType() == ELF::NT_ANDROID_TYPE_MEMTAG)
6051-
AndroidNoteDesc = Note.getDesc();
6053+
AndroidNoteDesc = Note.getDesc(4);
60526054
return Error::success();
60536055
};
60546056

60556057
processNotesHelper(
60566058
*this,
60576059
/*StartNotesFn=*/
60586060
[](std::optional<StringRef>, const typename ELFT::Off,
6059-
const typename ELFT::Addr) {},
6061+
const typename ELFT::Addr, size_t) {},
60606062
/*ProcessNoteFn=*/FindAndroidNote, /*FinishNotesFn=*/[]() {});
60616063

60626064
ArrayRef<uint8_t> Contents = getMemtagGlobalsSectionContents(MemtagGlobals);
@@ -7590,9 +7592,11 @@ template <class ELFT> void LLVMELFDumper<ELFT>::printNotes() {
75907592
ListScope L(W, "Notes");
75917593

75927594
std::unique_ptr<DictScope> NoteScope;
7595+
size_t Align = 0;
75937596
auto StartNotes = [&](std::optional<StringRef> SecName,
75947597
const typename ELFT::Off Offset,
7595-
const typename ELFT::Addr Size) {
7598+
const typename ELFT::Addr Size, size_t Al) {
7599+
Align = std::max<size_t>(Al, 4);
75967600
NoteScope = std::make_unique<DictScope>(W, "NoteSection");
75977601
W.printString("Name", SecName ? *SecName : "<?>");
75987602
W.printHex("Offset", Offset);
@@ -7604,7 +7608,7 @@ template <class ELFT> void LLVMELFDumper<ELFT>::printNotes() {
76047608
auto ProcessNote = [&](const Elf_Note &Note, bool IsCore) -> Error {
76057609
DictScope D2(W, "Note");
76067610
StringRef Name = Note.getName();
7607-
ArrayRef<uint8_t> Descriptor = Note.getDesc();
7611+
ArrayRef<uint8_t> Descriptor = Note.getDesc(Align);
76087612
Elf_Word Type = Note.getType();
76097613

76107614
// Print the note owner/type.

0 commit comments

Comments
 (0)