linux-kernel - Re: [PATCH v3 00/17] x86: head

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: <CAMj1kXGnGz+V3tmonitY+3TiWuWJFWj-mQLUbRo+xp8UwZ_SpQ@mail.gmail.com>
Date:   Wed, 23 Nov 2022 11:52:32 +0100
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Tom Lendacky <thomas.lendacky@....com>, linux-efi@...r.kernel.org,
        linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Michael Roth <michael.roth@....com>
Subject: Re: [PATCH v3 00/17] x86: head_64.S spring cleaning

On Wed, 23 Nov 2022 at 11:49, Borislav Petkov <bp@...en8.de> 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