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.han...@linux.intel.com> > 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.han...@linux.intel.com> > Cc: Ning Sun <ning....@intel.com> > Cc: Thomas Gleixner <t...@linutronix.de> > Cc: Ingo Molnar <mi...@redhat.com> > Cc: "H. Peter Anvin" <h...@zytor.com> > Cc: x...@kernel.org > Cc: tboot-de...@lists.sourceforge.net > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Andrea Arcangeli <aarca...@redhat.com> > --- > 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; > } > >
