linux-kernel - Re: [PATCH v3 3/4] x86/mm: Flush global TLB when switching to trampoline page-table

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]

Date:   Thu, 2 Dec 2021 13:58:51 +0100
From:   Joerg Roedel <jroedel@...e.de>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Joerg Roedel <joro@...tes.org>, x86@...nel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, hpa@...or.com,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Mike Rapoport <rppt@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Brijesh Singh <brijesh.singh@....com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/4] x86/mm: Flush global TLB when switching to
 trampoline page-table

On Wed, Oct 27, 2021 at 11:58:45AM +0200, Borislav Petkov wrote:
> On Fri, Oct 01, 2021 at 05:48:16PM +0200, Joerg Roedel wrote:
> > +void load_trampoline_pgtable(void)
> > +{
> > +#ifdef CONFIG_X86_32
> > +	load_cr3(initial_page_table);
> > +#else
> > +	/* Exiting long mode will fail if CR4.PCIDE is set. */
> 
> So this comment is not valid anymore if this is a separate function - it
> is valid only when that function is called in reboot.c so I guess you
> should leave that comment there.

Okay, but in the caller it is not visible the CR4.PCID is disabled in
this function. I'd rather update the comment to tell that the function
is called before transitioning to real mode?

> 
> > +	if (boot_cpu_has(X86_FEATURE_PCID))
> > +		cr4_clear_bits(X86_CR4_PCIDE);
> > +
> > +	write_cr3(real_mode_header->trampoline_pgd);
> 
> Is there any significance to the reordering of those calls here? The
> commit message doesn't say...

Yes, the call to cr4_clear_bits() is not safe anymore on the trampoline
page-table, because the per-cpu areas are not fully mapped anymore.

This changes with the next patch, but its nevertheless more robust to
minimize the code running on the trampoline page-table.

I will add that to the commit message.

Regards,

-- 
Jörg Rödel
jroedel@...e.de

SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
 
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev