From 5c3bf329088f62094ddfd24e3b1c15a312102ce8 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 26 Apr 2017 21:52:23 -0400 Subject: [PATCH] Don't allow anything with a small alignment in our PE files. When I added 4990d3f I inadvertantly made .data.ident and .rela.got sections appear in the top-level section headers at file offsets not aligned with PE->OptionalHeader.FileAlignment. This results in a section table that looks like: Sections: Idx Name Size VMA LMA File off Algn 0 .eh_frame 00018648 0000000000005000 0000000000005000 00000400 2**3 CONTENTS, ALLOC, LOAD, READONLY, DATA 1 .text 00093f45 000000000001e000 000000000001e000 00018c00 2**4 CONTENTS, ALLOC, LOAD, READONLY, CODE 2 .reloc 0000000a 00000000000b2000 00000000000b2000 000acc00 2**0 CONTENTS, ALLOC, LOAD, READONLY, DATA 3 .data.ident 000000e4 00000000000b3040 00000000000b3040 000ace40 2**5 CONTENTS, ALLOC, LOAD, DATA 4 .data 000291e8 00000000000b4000 00000000000b4000 000ad200 2**5 CONTENTS, ALLOC, LOAD, DATA 5 .vendor_cert 000003e2 00000000000de000 00000000000de000 000d6400 2**0 CONTENTS, ALLOC, LOAD, READONLY, DATA 6 .dynamic 000000f0 00000000000df000 00000000000df000 000d6800 2**3 CONTENTS, ALLOC, LOAD, DATA 7 .rela 0001aef8 00000000000e0000 00000000000e0000 000d6a00 2**3 CONTENTS, ALLOC, LOAD, READONLY, DATA 8 .rela.got 00000060 00000000000faef8 00000000000faef8 000f1af8 2**3 CONTENTS, ALLOC, LOAD, READONLY, DATA 9 .dynsym 0000ecd0 00000000000fb000 00000000000fb000 000f1e00 2**3 CONTENTS, ALLOC, LOAD, READONLY, DATA rather than: Sections: Idx Name Size VMA LMA File off Algn 0 .eh_frame 00018118 0000000000005000 0000000000005000 00000400 2**3 CONTENTS, ALLOC, LOAD, READONLY, DATA 1 .text 00091898 000000000001e000 000000000001e000 00018600 2**4 CONTENTS, ALLOC, LOAD, READONLY, CODE 2 .reloc 0000000a 00000000000b0000 00000000000b0000 000aa000 2**0 CONTENTS, ALLOC, LOAD, READONLY, DATA 3 .data 00028848 00000000000b1000 00000000000b1000 000aa200 2**5 CONTENTS, ALLOC, LOAD, DATA 4 .vendor_cert 00000449 00000000000da000 00000000000da000 000d2c00 2**0 CONTENTS, ALLOC, LOAD, READONLY, DATA 5 .dynamic 00000100 00000000000db000 00000000000db000 000d3200 2**3 CONTENTS, ALLOC, LOAD, DATA 6 .rela 0001ae50 00000000000dc000 00000000000dc000 000d3400 2**3 CONTENTS, ALLOC, LOAD, READONLY, DATA 7 .dynsym 0000ea78 00000000000f7000 00000000000f7000 000ee400 2**3 CONTENTS, ALLOC, LOAD, READONLY, DATA (Note "File off" on sections #3 and #8 on the top one.) This seems to work fine with edk2's loader and shim's loader, as well as their Authenticode implementation, and pesign's as well. While PE loaders seem to be fine with sections with alignments smaller than PE->OptionalHeader.FileAlignment, MS's signtool.exe does ... something else with them. I'm not sure what. What it definitely does *not* do is extend the digest based on their file offset and size. So just don't allow anything that small, and don't allow anything smaller than SectionAlignment either, just to be on the safe side. Since most of our stuff gets stripped into the debuginfo anyway, and shim has relatively few sections, this should not be a very large burden. So just to be clear: If you have a binary with a section that's not aligned on PE->OptionalHeader.FileAlignment: - pesign hashes it to A - tiano hashes it to A - shim hashes it to A - signtool.exe hashes it to B Because that makes sense. This patch works around the bug in signtool.exe . Signed-off-by: Peter Jones --- elf_aarch64_efi.lds | 17 +++++++++++++---- elf_arm_efi.lds | 15 +++++++++++---- elf_ia32_efi.lds | 1 + elf_x86_64_efi.lds | 10 ++++------ 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/elf_aarch64_efi.lds b/elf_aarch64_efi.lds index 4324dc9..96f15d5 100644 --- a/elf_aarch64_efi.lds +++ b/elf_aarch64_efi.lds @@ -14,12 +14,16 @@ SECTIONS . = ALIGN(16); _etext = .; } + + . = ALIGN(4096); .dynamic : { *(.dynamic) } . = ALIGN(4096); .note.gnu.build-id : { *(.note.gnu.build-id) } + + . = ALIGN(4096); .data.ident : { *(.data.ident) } @@ -55,10 +59,15 @@ SECTIONS } . = ALIGN(4096); - .rela.dyn : { *(.rela.dyn) } - .rela.plt : { *(.rela.plt) } - .rela.got : { *(.rela.got) } - .rela.data : { *(.rela.data) *(.rela.data*) } + . = ALIGN(4096); + .rela : + { + *(.rela.dyn) + *(.rela.plt) + *(.rela.got) + *(.rela.data) + *(.rela.data*) + } _edata = .; _data_size = . - _data; diff --git a/elf_arm_efi.lds b/elf_arm_efi.lds index 0287293..b12424e 100644 --- a/elf_arm_efi.lds +++ b/elf_arm_efi.lds @@ -20,6 +20,8 @@ SECTIONS .note.gnu.build-id : { *(.note.gnu.build-id) } + + . = ALIGN(4096); .data.ident : { *(.data.ident) } @@ -55,10 +57,15 @@ SECTIONS } . = ALIGN(4096); - .rel.dyn : { *(.rel.dyn) } - .rel.plt : { *(.rel.plt) } - .rel.got : { *(.rel.got) } - .rel.data : { *(.rel.data) *(.rel.data*) } + . = ALIGN(4096); + .rel : + { + *(.rel.dyn) + *(.rel.plt) + *(.rel.got) + *(.rel.data) + *(.rel.data*) + } _edata = .; _data_size = . - _data; diff --git a/elf_ia32_efi.lds b/elf_ia32_efi.lds index 2ba18c7..deec2ec 100644 --- a/elf_ia32_efi.lds +++ b/elf_ia32_efi.lds @@ -23,6 +23,7 @@ SECTIONS .note.gnu.build-id : { *(.note.gnu.build-id) } + . = ALIGN(4096); .data.ident : { *(.data.ident) } diff --git a/elf_x86_64_efi.lds b/elf_x86_64_efi.lds index 81d21a0..1f561b2 100644 --- a/elf_x86_64_efi.lds +++ b/elf_x86_64_efi.lds @@ -28,6 +28,8 @@ SECTIONS .note.gnu.build-id : { *(.note.gnu.build-id) } + + . = ALIGN(4096); .data.ident : { *(.data.ident) } @@ -62,12 +64,8 @@ SECTIONS .rela : { *(.rela.data*) - } - .rela.got : { - *(.rela.got) - } - .rela.stab : { - *(.rela.stab) + *(.rela.got*) + *(.rela.stab*) } _edata = .; _data_size = . - _data;