Re: [PATCH v3 00/17] x86: head_64.S spring cleaning

[Ard Biesheuvel]

On Wed, 23 Nov 2022 at 11:49, Borislav Petkov <bp@xxxxxxxxx> wrote:
>
> On Tue, Nov 22, 2022 at 03:49:29PM -0600, Tom Lendacky wrote:
> > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> > index cb5f0befee57..a0bfd31358ba 100644
> > --- a/drivers/firmware/efi/libstub/x86-stub.c
> > +++ b/drivers/firmware/efi/libstub/x86-stub.c
> > @@ -23,7 +23,7 @@
> >  const efi_system_table_t *efi_system_table;
> >  const efi_dxe_services_table_t *efi_dxe_table;
> > -u32 image_offset;
> > +u32 image_offset __section(".data");
> >  static efi_loaded_image_t *image = NULL;
> >  static efi_status_t
> >
> > I assume it has to do with being in .data vs .bss and not being explicitly
> > cleared with the encryption bit set. With the change to put image_offset in
> > the .data section, it is read as zero, where as when it was in the .bss
> > section it was reading "ciphertext".
>
> Hmm, two points about this:
>
> 1. Can we do
>
> u32 image_offset __bss_decrypted;
>
> here instead? We have this special section just for that fun and it
> self-documents this way.
>

The patch moves it from .data to .bss inadvertently, and I am not
convinced Tom's analysis is entirely accurate: we may simply have
garbage in image_offset if we access it before .bss gets cleared.

> 2. Also, why does my SEV-ES guest boot just fine without that change?
>

Indeed, so it needs to be in .data


> [    0.000000] Linux version 6.1.0-rc6+ (root@ml) (gcc (Debian 11.3.0-1) 11.3.0, GNU ld (GNU Binutils for Debian) 2.38) #1 SMP PREEMPT_DYNAMIC Wed Nov 23 11:27:17 CET 2022
> ...
> [    0.336132] Memory Encryption Features active: AMD SEV SEV-ES
>
> Thx.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette