linux-kernel - Re: [PATCH v2 4/4] x86/64/mm: Map all kernel memory into trampoline

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 for Android: free password hash cracker in your pocket

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

Message-ID: <7461fee2-6e9e-8e5c-2dc3-e18cdf63bf72@intel.com>
Date:   Wed, 29 Sep 2021 08:22:28 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Joerg Roedel <joro@...tes.org>, x86@...nel.org
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        hpa@...or.com, Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Joerg Roedel <jroedel@...e.de>,
        Mike Rapoport <rppt@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Brijesh Singh <brijesh.singh@....com>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2 4/4] x86/64/mm: Map all kernel memory into
 trampoline_pgd

On 9/29/21 7:55 AM, Joerg Roedel wrote:
...
> The reason this does not crash always is only that kernel mappings are
> global and the CR3 switch does not flush those mappings. But if theses
> mappings are not in the TLB already, the above code will crash before it
> can jump to the real-mode stub.

This would have been nice to have in the cover letter.  The whole
purpose for this series wasn't totally apparent until I read this.

> diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c
> index 0cfe1046cec9..792cb9ca9b29 100644
> --- a/arch/x86/realmode/init.c
> +++ b/arch/x86/realmode/init.c
> @@ -91,6 +91,7 @@ static void __init setup_real_mode(void)
>  #ifdef CONFIG_X86_64
>  	u64 *trampoline_pgd;
>  	u64 efer;
> +	int i;
>  #endif
>  
>  	base = (unsigned char *)real_mode_header;
> @@ -147,8 +148,17 @@ static void __init setup_real_mode(void)
>  	trampoline_header->flags = 0;
>  
>  	trampoline_pgd = (u64 *) __va(real_mode_header->trampoline_pgd);
> +
> +	/*
> +	 * Map all of kernel memory into the trampoline PGD so that it includes
> +	 * the direct mapping and vmalloc space. This is needed to keep the
> +	 * stack and real_mode_header mapped when switching to this page table.
> +	 */

This comment's mention of the direct map and vmalloc() makes a lot of
sense in the context of this patch where you're adding them.  But, it
doesn't mention the pgd[511] stuff.

Maybe just make it more generic:

	Include the entirety of the kernel mapping into the trampoline
	PGD.  This way, all mappings present in the normal kernel page
	tables are usable while running on trampoline_pgd.


> +	for (i = pgd_index(__PAGE_OFFSET); i < PTRS_PER_PGD; i++)
> +		trampoline_pgd[i] = init_top_pgt[i].pgd;
> +
> +	/* Map the real mode stub as virtual == physical */
>  	trampoline_pgd[0] = trampoline_pgd_entry.pgd;
> -	trampoline_pgd[511] = init_top_pgt[511].pgd;
>  #endif

Nit: can we preserve the order, please?

	/* Map the real mode stub as virtual == physical */
  	trampoline_pgd[0] = trampoline_pgd_entry.pgd;

	for (i = pgd_index(__PAGE_OFFSET); i < PTRS_PER_PGD; i++)
		trampoline_pgd[i] = init_top_pgt[i].pgd;