[RFC PATCH] efi: Disable stack smashing protection on grub_efi_init()

[Glenn Washburn]

GCC is electing to instrument grub_efi_init() to give it stack smashing
protection when configuring with --enable-stack-protector on the x86_64-efi
target. In the function prologue, the canary at the top of the stack frame
is set to the value of the stack guard. And in the epilogue, the canary is
checked to verify if it is equal to the guard and if not to call the stack
check fail function. The issue is that grub_efi_init() sets up the guard
by initializing it with random bytes, if the firmware supports the RNG
protocol. So in its prologue the canary will be set with the value of the
uninitialized guard, likely NULL bytes. Then the guard is initialized, and
finally the epilogue checks the canary against the guard, which will almost
certainly be different. This causes the code path for a smashed stack to be
taken, causing the machine to print out a message that stack smashing was
detected, wait 5 seconds, and then reboot. Disable grub_efi_init()
instrumentation so there is no stack smashing false positive generated.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
I recently discovered that my OVMF (debian version 2020.11-2+deb11u1) was
not initializing the stack guard because the firmware was did enable support
for the RNG protocol. It turns out that the RNG protocol is not enabled
unless a source of randomness is found, which can be added with the QEMU
arguments "-device virtio-rng-pci". This issue did not occur when the
guard was not initialized in grub_efi_init() because the uninitialized
value of the guard would be used for the canary, and sinice nothing changed,
the canary and guard would be the same at the end of grub_efi_init().

Initially to solve this, I tried to set the canary in grub_efi_init() after
the guard gets initialized. The problem I ran into is that gcc outputs
instructions that use the base pointer register as a general purpose
register. I couldn't come up with another method of generically figuring
out where the start of the frame was so that I could set the canary. The
rsp register is offset by the functions local variables which are variable
as code is inlined or otherwise changes. Knowing the offset of the stack
pointer to the frame start, I could properly set the canary. But again,
if local variables are added or subtracted, including in inlined functions
called, that offset will need to be regenerated. And its fragile because
different verions of GCC or optimization levels may change this offset also.

If there was a robust way to set the canary, I think that would be preferable
so that we can get stack smashing support on the function. Any one have ideas?

Another thing that has troubled me is why has no one else seen this behavior
in the last several years that this code has been active? Do distros already
have a patch for this? Is the RNG protocol supported by virtual no bare
metal firmwares? I don't hit this issue on test machine, so I'm lead to
believe that it must not support the RNG protocol (but I've yet to check).
Are there other explanations for this that I'm missing?

Glenn
---
 grub-core/kern/efi/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
index 6fe1ff8c868e..e759cc315b85 100644
--- a/grub-core/kern/efi/init.c
+++ b/grub-core/kern/efi/init.c
@@ -102,7 +102,7 @@ stack_protector_init (void)
 
 grub_addr_t grub_modbase;
 
-void
+__attribute__ ((__optimize__ ("-fno-stack-protector"))) void
 grub_efi_init (void)
 {
   grub_modbase = grub_efi_section_addr ("mods");
-- 
2.34.1