On Mon, 13 Nov 2023 17:18:50 +0100
Daniel Kiper <dkiper@net-space.pl> wrote:
> On Sun, Nov 12, 2023 at 08:22:42AM +0100, Heinrich Schuchardt wrote:
> > On 11/12/23 04:23, Glenn Washburn wrote:
> > > The canary, __stack_chk_guard, is in the BSS and so will get initialized to
> > > zero if it is not explicitly initialized. If the UEFI firmware does not
> > > support the RNG protocol, then the canary will not be randomized and will
> > > be used as zero. This seems like a possibly easier value to write by an
> > > attacker. Initialize canary to static random bytes, so that it is still
> > > random when there is not RNG protocol.
Ugh. s/not/no/
> > >
> > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> >
> > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
>
> > > ---
> > > 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 0e28bea17a76..b85d98ca47fd 100644
> > > --- a/grub-core/kern/efi/init.c
> > > +++ b/grub-core/kern/efi/init.c
> > > @@ -41,7 +41,7 @@ static grub_guid_t rng_protocol_guid = GRUB_EFI_RNG_PROTOCOL_GUID;
> > >
> > > static grub_efi_uint8_t stack_chk_guard_buf[32];
> > >
> > > -grub_addr_t __stack_chk_guard;
> > > +grub_addr_t __stack_chk_guard = (grub_addr_t) 0x92f2b7e2f193b25c;
Just now reflecting on this I had the idea that perhaps even better is
to have this randomly generated at build time and inserted as an
autoconf variable. Then every build would have a different canary.
I think I'll send a patch for this soon.
But please insure it is reproducible (as in reproducible builds initiative).
But also, one already has time and date macros that are unique and reproducible in most distros. Can you use TIME macro as either the random value or the seed to make a suitable random number?
>
> I would add last sentence from the commit message before this line.
> I can do it for you before push...
Sure. And please remove the 't' mentioned above in the commit message.
Glenn
>
> > >
> > > void __attribute__ ((noreturn))
> > > __stack_chk_fail (void)
>
> Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel