Enable stack protector#808
Draft
vathpela wants to merge 15 commits into
Draft
Conversation
vathpela
force-pushed
the
stack-protector
branch
2 times, most recently
from
July 8, 2026 14:52
dcf3d9e to
8a96dbc
Compare
We've had a comment since forever, borrowed from some old ia64 code somewhere else, that inexplicably says .hash (or sometimes .gnu.hash) absolutely must come first. As far as I can tell there's no reason for it, and it isn't in any way true. I don't think it was true on Itanic, either. What *is* special is that .hash winds up /defaulting/ to having a VMA of 0 set with a file offset of 0x1000, which winds up clearing space for PE headers that are otherwise not being reserved. So when we objcopy to our PE, by virtue of being after .hash, whatever comes next lands after the PE headers. If we don't have .hash but just move . to 0x1000, that also works just fine. This patch does the following: - adds that offset and an explanation of why - removes the wrong comment In theory we could also move .hash, .gnu.hash, and .eh_frame to later in the binary (which would save a few KiB since we're not copying it), but I'm not doing that here. Signed-off-by: Peter Jones <pjones@redhat.com>
vathpela
force-pushed
the
stack-protector
branch
3 times, most recently
from
July 15, 2026 19:57
d23bbcf to
614e0a1
Compare
This adds a minimal backtrace facility, which is only built when we've built with ENABLE_SHIM_DEVEL. This backtrace facility depends on __builtin_return_address(1) being relatively safe (i.e. not crashing further) on the platform. The gcc documentation isn't particularly clear on when that isn't true, but it seems to be true for the platforms we generally care about. Signed-off-by: Peter Jones <pjones@redhat.com>
This implements support for using `gcc -fstack-protector...` in all of our .efi objects. If we're built with ENABLE_SHIM_DEVEL, this will also include a backtrace in the message to the user. Signed-off-by: Peter Jones <pjones@redhat.com>
vathpela
force-pushed
the
stack-protector
branch
from
July 15, 2026 20:13
614e0a1 to
2830f26
Compare
"make fuzz-clean" is trying to remove valgrind files that fuzz no longer generates, as well as log files that are no longer in the top-level directory. This removes the valgrind bits and fixes the directory. Signed-off-by: Peter Jones <pjones@redhat.com>
During some testing I noticed "make clean" when cross-building for Aarch64 included gcc complaining about "-mstrict-align", which is weird because aarch64-linux-gnu-gcc supports that just fine. Turns out it's because the test makefile is always using the host CC*, and that means it's calling 'gcc ... $(ARCH_CFLAGS) ... -print-file-name=include-fixed' to figure out compiler arguments that don't matter for the clean target. Since we don't actually have any support for using the cross-compiler to build test or fuzz targets, as we would have no way to run them, this will always happen and it'll always be dumb. This change makes it so Instead we just don't descend into those makefiles if we're just cleaning and not actually building/running the tests or fuzzers. [*] Actually was always wrongly using "gcc" and not "$(HOSTCC)" Signed-off-by: Peter Jones <pjones@redhat.com>
Currently there are three "make fuzz" targets: "fuzz", "fuzz-lto", and "fuzz-coverage". The last two were copy pasted from "make test" rules, and they make not sense here. Additionally, the "make fuzz" rule itself makes no sense as given, since it'll try to run all the fuzzers, one at a time, each with no time limit. This patch removes the two dumb rules and adds a 60-second limit to the main rule. Signed-off-by: Peter Jones <pjones@redhat.com>
When I added the ARCH_blah definitions in Make.defaults, a couple of arches wound up with multiple ARCH_CFLAGS definitions. They're harmless, but they could wind up being confusing later. This patch removes them. Signed-off-by: Peter Jones <pjones@redhat.com>
For reasons beyond my comprehension, currently when building aarch64 debuginfo binaries, we get this: aarch64-linux-gnu-objcopy: shimaa64.efi.debug: not enough room for program headers, try linking with -N aarch64-linux-gnu-objcopy: shimaa64.efi.debug[.eh_frame]: bad value make: *** [../Makefile:458: shimaa64.efi.debug] Error 1 This sounds like it's about the space for the program headers before the first section, but adding more space changes nothing. What *does* change something is adding another section later on. This patch adds .data.ident to the objcopy section list, which should make absolutely no meaningful difference either in the link map or in using the debuginfo, but results in objcopy working just fine. With regret, Signed-off-by: Peter Jones <pjones@redhat.com>
Previously post-process-pe would complain like:
validate_nx_compat():467: Section 9 has Virtual Address 0x00227000 that isn't section aligned (0x00010000)
which is moderately useful, but stealthily tricks the user into trying
to figure out if we're counting from 0 or 1, which then makes them have
to resolve this by looking at the actual addresses we're complaining
about to match it up.
This patch changes that output to:
validate_nx_compat():467: Section 9 (".rodata") has Virtual Address 0x00227000 that isn't section aligned (0x00010000)
Signed-off-by: Peter Jones <pjones@redhat.com>
While investigating how to make post-process-pe show section names, I
discovered yet another way our objcopy magic binaries are amazing.
If you dump, say, our shimx64.efi binary with objdump, you'll see
something like:
$ objdump -h shimx64.efi
shimx64.efi: file format pei-x86-64
Sections:
Idx Name Size VMA LMA File off Algn
0 .eh_frame 00038f6c 000000000000a000 000000000000a000 00001000 2**2
CONTENTS, ALLOC, LOAD, READONLY, DATA
1 .text 000e4d52 0000000000043000 0000000000043000 0003a000 2**4
CONTENTS, ALLOC, LOAD, READONLY, CODE
2 .data.ident 00000066 0000000000129000 0000000000129000 0011f000 2**4
CONTENTS, ALLOC, LOAD, DATA
3 .sbatlevel 0000004f 000000000012a000 000000000012a000 00120000 2**2
CONTENTS, ALLOC, LOAD, READONLY, DATA
4 .data 0005d128 000000000012b000 000000000012b000 00121000 2**4
CONTENTS, ALLOC, LOAD, DATA
5 .reloc 0000000c 0000000000189000 0000000000189000 0017f000 2**2
CONTENTS, ALLOC, LOAD, READONLY, DATA
6 .vendor_cert 000008fa 000000000018a000 000000000018a000 00180000 2**2
CONTENTS, ALLOC, LOAD, READONLY, DATA
7 .dynamic 00000130 000000000018b000 000000000018b000 00181000 2**2
CONTENTS, ALLOC, LOAD, DATA
8 .rela 00034de8 000000000018c000 000000000018c000 00182000 2**2
CONTENTS, ALLOC, LOAD, READONLY, DATA
9 .sbat 00000083 00000000001c1000 00000000001c1000 001b7000 2**2
CONTENTS, ALLOC, LOAD, READONLY, DATA
10 .dynsym 0001cd70 00000000001c2000 00000000001c2000 001b8000 2**2
CONTENTS, ALLOC, LOAD, READONLY, DATA
11 .dynstr 0001a063 00000000001df000 00000000001df000 001d5000 2**2
CONTENTS, ALLOC, LOAD, READONLY, DATA
There's no .strtab or .symtab (and objdump would hide them from you
anyway...), because we post-processed this from a shared object, so
everything is in .dynsym and .dynstr. So you figure the section names
are in there, right? And this binary is approximately 2027619 bytes
long, plus maybe a few KiB for signatures at the end. Right?
But then you look harder at ".eh_frame", the first section name longer
than 8 characters, has this in the raw section header:
00000188 2f 34 00 00 00 00 00 00 |/4......|
00000190 6c 8f 03 00 00 a0 00 00 00 90 03 00 00 10 00 00 |l...............|
000001a0 00 00 00 00 00 00 00 00 00 00 00 00 40 00 00 40 |............@..@|
In PE, the /4 means it's at position 4 in the strings. But it's not:
String section [15] '.dynstr' contains 106595 bytes at offset 0x1df000:
[ 0]
[ 1] ImageBase
[ b] _relocate
[ 15] __stack_chk_guard
[ 27] SHIM_LOCK_GUID
[ 36] get_variable
[ 43] console_print
[ 51] __stack_chk_fail
Because it's in another string table that's completely hidden! Which
objdump knows how to look up but doesn't tell you about because it's not
in a section for no good reason.
If you dump the COFF file header, you'll see:
0x8C 0x8 PointerToSymbolTable: 0x1F0000
0x90 0xC NumberOfSymbols: 0x3B1B
And sure enough if you hex dump at 0x1f0000 you'll see a COFF symbol
table that absolutely nothing is using, and if you dump
0x1f0000 + 0x3b1b * 18, you'll see:
002327e6 ff 4f 02 00 2e 65 68 5f 66 72 |..O...eh_fr|
002327f0 61 6d 65 00 2e 64 61 74 61 2e 69 64 65 6e 74 00 |ame..data.ident.|
00232800 2e 73 62 61 74 6c 65 76 65 6c 00 2e 76 65 6e 64 |.sbatlevel..vend|
00232810 6f 72 5f 63 65 72 74 00 64 65 62 75 67 5f 68 6f |or_cert.debug_ho|
00232820 6f 6b 00 77 61 69 74 5f 66 6f 72 5f 64 65 62 75 |ok.wait_for_debu|
Which is a uint32_t that's 0x024fff and then a perfectly normal string
table that starts with, that's right, .eh_frame. And if you add that
length up you get 0x2577e5. And sure enough:
002577c0 61 72 69 61 62 6c 65 5f 64 61 74 61 00 6f 73 73 |ariable_data.oss|
002577d0 6c 5f 64 65 72 5f 6f 69 64 5f 63 32 74 6e 62 31 |l_der_oid_c2tnb1|
002577e0 39 31 76 31 00 |91v1.|
002577e5
That's the end of the binary.
tl;dr: there's 394KiB hanging around at the end of this binary, and the
only thing that's using it is using it is objdump, which is using... 49
bytes of it to show us section names.
Signed-off-by: Peter Jones <pjones@redhat.com>
I don't see how this could be it, but who knows. Signed-off-by: Peter Jones <pjones@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This enables stack protector for shim, mm, and fb.
Note that it needs to go after the openssl 3.5 merge.