linux-kernel - Re: [PATCH 2/3] efi/libstub: Fix page table access in 5-level to 4-level paging transition

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: <lbkjggisugaccdowww6zlux6w6jpf2izm54myfrla2lgvcomb7@52kd6ferku5r>
Date: Thu, 23 Oct 2025 15:28:19 +0100
From: Kiryl Shutsemau <kas@...nel.org>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: Usama Arif <usamaarif642@...il.com>, dwmw@...zon.co.uk, 
	tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com, 
	hpa@...or.com, x86@...nel.org, apopple@...dia.com, thuth@...hat.com, 
	nik.borisov@...e.com, linux-kernel@...r.kernel.org, linux-efi@...r.kernel.org, 
	kernel-team@...a.com, Michael van der Westhuizen <rmikey@...a.com>, 
	Tobias Fleig <tfleig@...a.com>
Subject: Re: [PATCH 2/3] efi/libstub: Fix page table access in 5-level to
 4-level paging transition

On Thu, Oct 23, 2025 at 04:13:26PM +0200, Ard Biesheuvel wrote:
> On Thu, 23 Oct 2025 at 00:08, Usama Arif <usamaarif642@...il.com> wrote:
> >
> > When transitioning from 5-level to 4-level paging, the existing code
> > incorrectly accesses page table entries by directly dereferencing CR3
> > and applying PAGE_MASK. This approach has several issues:
> >
> > - __native_read_cr3() returns the raw CR3 register value, which on
> >   x86_64 includes not just the physical address but also flags Bits
> >   above the physical address width of the system (i.e. above
> >   __PHYSICAL_MASK_SHIFT) are also not masked.
> > - The pgd value is masked by PAGE_SIZE which doesn't take into account
> >   the higher bits such as _PAGE_BIT_NOPTISHADOW.
> >
> > Replace this with proper accessor functions:
> > - read_cr3_pa(): Uses CR3_ADDR_MASK properly clearing SME encryption bit
> >   and extracting only the physical address portion.
> > - mask pgd value with PTE_PFN_MASK instead of PAGE_MASK, accounting for
> >   flags above physical address (_PAGE_BIT_NOPTISHADOW in particular).
> >
> > Fixes: cb1c9e02b0c1 ("x86/efistub: Perform 4/5 level paging switch from the stub")
> > Co-developed-by: Kiryl Shutsemau <kas@...nel.org>
> > Signed-off-by: Kiryl Shutsemau <kas@...nel.org>
> > Signed-off-by: Usama Arif <usamaarif642@...il.com>
> > Reported-by: Michael van der Westhuizen <rmikey@...a.com>
> > Reported-by: Tobias Fleig <tfleig@...a.com>
> > ---
> >  drivers/firmware/efi/libstub/x86-5lvl.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/x86-5lvl.c b/drivers/firmware/efi/libstub/x86-5lvl.c
> > index f1c5fb45d5f7c..34b72da457487 100644
> > --- a/drivers/firmware/efi/libstub/x86-5lvl.c
> > +++ b/drivers/firmware/efi/libstub/x86-5lvl.c
> > @@ -81,8 +81,11 @@ void efi_5level_switch(void)
> >                 new_cr3 = memset(pgt, 0, PAGE_SIZE);
> >                 new_cr3[0] = (u64)cr3 | _PAGE_TABLE_NOENC;
> >         } else {
> > +               pgd_t *pgdp;
> > +
> > +               pgdp = (pgd_t *)read_cr3_pa();
> 
> Shouldn't this be using native_read_cr3_pa()?

Perhaps. But I don't think it makes a difference.

We don't have paravirt in stub/decompressor, do we?

> And is there any reason
> to re-read CR3 here, rather than update the code that populates the
> cr3 variable? The preceding other branch of the if() should probably
> use the same sanitised value of CR3, no?

Good point.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov