Re: Avoid speculative indirect calls in kernel

[Thomas Gleixner]

On Mon, 8 Jan 2018, Andrea Arcangeli wrote:

> On Mon, Jan 08, 2018 at 09:53:02PM +0100, Thomas Gleixner wrote:
> > Thanks for resending it.
> 
> Thanks to you for the PTI improvements!
> 
> Did my best to do the cleanest patch for tip, but I now figured Dave's
> original comment was spot on: a _PAGE_NX clear then becomes necessary
> also after pud_alloc not only after p4d_alloc.
> 
> pmd_alloc would run into the same with x86 32bit non-PAE too.
> 
> So there are two choices, either going back to one single _PAGE_NX
> clear from the original Dave's original patch as below, or to add
> multiple clear after each level which was my objective and is more
> robust, but it may be overkill in this case. As long as it was one
> line it looked a clear improvement.
> 
> Considering the caller in both cases is going to abort I guess we can
> use the one liner approach as Dave and Jiri did originally.

Dave ?

> 
> It's up to you, doing it at each level would be more resilent in case
> the caller is changed.
> 
> For the efi_64 same issue, the current tip patch will work better, but
> it can still be cleaned up with pgd_efi instead of pgd_offset_k().
> 
> I got partly fooled because it worked great with 4levels, but it
> wasn't ok anyway for 32bit non-PAE. Sometime it's the simpler stuff
> that gets more subtle.
> 
> Andrea
> 
> >From 391517951e904cdd231dda9943c36a25a7bf01b9 Mon Sep 17 00:00:00 2001
> From: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Date: Sat, 6 Jan 2018 18:41:14 +0100
> Subject: [PATCH 1/1] x86/kaiser/efi: unbreak tboot
> 
> This is another case similar to what EFI does: create a new set of
> page tables, map some code at a low address, and jump to it.  PTI
> mistakes this low address for userspace and mistakenly marks it
> non-executable in an effort to make it unusable for userspace.  Undo
> the poison to allow execution.
> 
> Signed-off-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Cc: Ning Sun <ning.sun@xxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> Cc: x86@xxxxxxxxxx
> Cc: tboot-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Andrea Arcangeli <aarcange@xxxxxxxxxx>
> ---
>  arch/x86/kernel/tboot.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
> index a4eb27918ceb..a2486f444073 100644
> --- a/arch/x86/kernel/tboot.c
> +++ b/arch/x86/kernel/tboot.c
> @@ -138,6 +138,17 @@ static int map_tboot_page(unsigned long vaddr, unsigned long pfn,
>  		return -1;
>  	set_pte_at(&tboot_mm, vaddr, pte, pfn_pte(pfn, prot));
>  	pte_unmap(pte);
> +
> +	/*
> +	 * PTI poisons low addresses in the kernel page tables in the
> +	 * name of making them unusable for userspace.  To execute
> +	 * code at such a low address, the poison must be cleared.
> +	 *
> +	 * Note: 'pgd' actually gets set in p4d_alloc() _or_
> +	 * pud_alloc() depending on 4/5-level paging.
> +	 */
> +	pgd->pgd &= ~_PAGE_NX;
> +
>  	return 0;
>  }
>  
>