linux-kernel - Re: [PATCH v4 2/3] x86/efistub: Obtain SEV CC blob address from the stub

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMj1kXEn1cJ7+K+sr1520S3z=7uB6_=xjA5+JF2Q50y_V6zRoA@mail.gmail.com>
Date: Thu, 11 Sep 2025 23:49:26 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Tom Lendacky <thomas.lendacky@....com>
Cc: Ard Biesheuvel <ardb+git@...gle.com>, linux-efi@...r.kernel.org, 
	linux-kernel@...r.kernel.org, x86@...nel.org, Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH v4 2/3] x86/efistub: Obtain SEV CC blob address from the stub

On Thu, 11 Sept 2025 at 23:27, Tom Lendacky <thomas.lendacky@....com> wrote:
>
> On 9/9/25 03:06, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@...nel.org>
> >
> > The x86 EFI stub no longer boots the core kernel via the traditional
> > decompressor but jumps straight to it, avoiding all the page fault
> > handling and other complexity that is entirely unnecessary when booting
> > via EFI, which guarantees that all system memory is mapped 1:1.
> >
> > The SEV startup code in the core kernel expects the address of the CC
> > blob configuration table in boot_params, so store it there when booting
> > from EFI with SEV-SNP enabled. This removes the need to call
> > sev_enable() from the EFI stub.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> > ---
> >  drivers/firmware/efi/libstub/x86-stub.c | 21 +++++++++++++++-----
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> > index 0d05eac7c72b..c4ef645762ec 100644
> > --- a/drivers/firmware/efi/libstub/x86-stub.c
> > +++ b/drivers/firmware/efi/libstub/x86-stub.c
> > @@ -681,17 +681,28 @@ static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
> >       return EFI_SUCCESS;
> >  }
> >
> > -static bool have_unsupported_snp_features(void)
> > +static bool check_snp_features(struct boot_params *bp)
> >  {
> > +     u64 status = sev_get_status();
> >       u64 unsupported;
> >
> > -     unsupported = snp_get_unsupported_features(sev_get_status());
> > +     unsupported = snp_get_unsupported_features(status);
> >       if (unsupported) {
> >               efi_err("Unsupported SEV-SNP features detected: 0x%llx\n",
> >                       unsupported);
> > -             return true;
> > +             return false;
> >       }
> > -     return false;
> > +
> > +     if (status & MSR_AMD64_SEV_SNP_ENABLED) {
> > +             void *tbl = get_efi_config_table(EFI_CC_BLOB_GUID);
> > +
> > +             if (!tbl) {
> > +                     efi_err("SEV-SNP is enabled but CC blob not found\n");
> > +                     return false;
> > +             }
> > +             bp->cc_blob_address = (u32)(unsigned long)tbl;
>
> I think we ran into bugs where the cc_blob_address was random data from a
> boot loader when SNP wasn't active and that's why we always initialize it
> to 0:
>
> 4b1c74240757 ("x86/boot: Don't propagate uninitialized boot_params->cc_blob_address")
>
> So we probably need the same statement that is at the beginning of the
> decompressor sev_enable() at the very beginning of this function to ensure
> cc_blob_address is set to zero:
>
>         /*
>          * bp->cc_blob_address should only be set by boot/compressed kernel.
>          * Initialize it to 0 to ensure that uninitialized values from
>          * buggy bootloaders aren't propagated.
>          */
>         if (bp)
>                 bp->cc_blob_address = 0;
>

AIUI this was needed for bootloaders like SYSLINUX, which boot in
pseudo-EFI mode, i.e., not via the EFI stub but with the EFI fields in
struct boot_params populated. This means zeroing the field in the stub
is not going to make a difference.

It doesn't hurt either, so I can find a place to stick this, but I'm
not convinced we still need this here. Note that GRUB no longer boots
via the EFI handover protocol (and mainline GRUB never did), and so
struct boot_params is typically allocated (and wiped) by the EFI stub
and not taken from the bootloader.