Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754459AbeAJApO (ORCPT + 1 other); Tue, 9 Jan 2018 19:45:14 -0500 Received: from Galois.linutronix.de ([146.0.238.70]:53838 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751981AbeAJApN (ORCPT ); Tue, 9 Jan 2018 19:45:13 -0500 Date: Wed, 10 Jan 2018 01:45:05 +0100 (CET) From: Thomas Gleixner To: Andrea Arcangeli cc: Jon Masters , "Woodhouse, David" , Paolo Bonzini , Alan Cox , Linus Torvalds , Andi Kleen , Greg Kroah-Hartman , Tim Chen , Linux Kernel Mailing List , Dave Hansen , Jeff Law , Nick Clifton , Andy Lutomirski , Peter Zijlstra Subject: Re: Avoid speculative indirect calls in kernel In-Reply-To: <20180108213223.GF4703@redhat.com> Message-ID: References: <20180104015920.1ad7b9d3@alans-desktop> <1515054014.12987.75.camel@amazon.co.uk> <403e65be-cfd1-fd08-0401-2e26470b63d4@redhat.com> <4dde456c-fd15-e768-8876-5844c8b7c455@redhat.com> <9976a670-a023-ea1f-3f13-ee5253092533@redhat.com> <20180108102805.GK25546@redhat.com> <20180108213223.GF4703@redhat.com> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: 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 > 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 > Cc: Ning Sun > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: "H. Peter Anvin" > Cc: x86@kernel.org > Cc: tboot-devel@lists.sourceforge.net > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Andrea Arcangeli > --- > 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; > } > >