From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752877AbdLEQBs (ORCPT ); Tue, 5 Dec 2017 11:01:48 -0500 Received: from mx2.suse.de ([195.135.220.15]:47149 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751937AbdLEQBq (ORCPT ); Tue, 5 Dec 2017 11:01:46 -0500 Date: Tue, 5 Dec 2017 17:01:34 +0100 From: Borislav Petkov To: Thomas Gleixner Cc: LKML , x86@kernel.org, Linus Torvalds , Andy Lutomirsky , Peter Zijlstra , Dave Hansen , Greg KH , keescook@google.com, hughd@google.com, Brian Gerst , Josh Poimboeuf , Denys Vlasenko , Rik van Riel , Boris Ostrovsky , Juergen Gross , David Laight , Eduardo Valentin , aliguori@amazon.com, Will Deacon , daniel.gruss@iaik.tugraz.at, Dave Hansen Subject: Re: [patch 31/60] x86/mm/kpti: Add mapping helper functions Message-ID: <20171205160134.dhrc5oata2hqln3a@pd.tnic> References: <20171204140706.296109558@linutronix.de> <20171204150607.391576490@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20171204150607.391576490@linutronix.de> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Dec 04, 2017 at 03:07:37PM +0100, Thomas Gleixner wrote: > From: Dave Hansen > > Add the pagetable helper functions do manage the separate user space page > tables. > > [ tglx: Split out from the big combo kaiser patch ] > > Signed-off-by: Dave Hansen > Signed-off-by: Thomas Gleixner > > --- > arch/x86/include/asm/pgtable_64.h | 139 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 139 insertions(+) ... > +/* > + * Take a PGD location (pgdp) and a pgd value that needs to be set there. > + * Populates the user and returns the resulting PGD that must be set in > + * the kernel copy of the page tables. > + */ > +static inline pgd_t kpti_set_user_pgd(pgd_t *pgdp, pgd_t pgd) > +{ Btw, do we want to inline a relatively big function like that? I see at least 20-ish callsites of set_pgd() only. > +#ifdef CONFIG_KERNEL_PAGE_TABLE_ISOLATION > + if (!static_cpu_has_bug(X86_BUG_CPU_SECURE_MODE_KPTI)) > + return pgd; > + > + if (pgd_userspace_access(pgd)) { > + if (pgdp_maps_userspace(pgdp)) { > + /* > + * The user page tables get the full PGD, > + * accessible from userspace: > + */ > + kernel_to_user_pgdp(pgdp)->pgd = pgd.pgd; > + /* > + * For the copy of the pgd that the kernel uses, > + * make it unusable to userspace. This ensures on > + * in case that a return to userspace with the > + * kernel CR3 value, userspace will crash instead > + * of running. > + * > + * Note: NX might be not available or disabled. > + */ > + if (__supported_pte_mask & _PAGE_NX) > + pgd.pgd |= _PAGE_NX; > + } > + } else if (pgd_userspace_access(*pgdp)) { > + /* > + * We are clearing a _PAGE_USER PGD for which we presumably > + * populated the user PGD. We must now clear the user PGD > + * entry. > + */ > + if (pgdp_maps_userspace(pgdp)) { > + kernel_to_user_pgdp(pgdp)->pgd = pgd.pgd; > + } else { > + /* > + * Attempted to clear a _PAGE_USER PGD which is in > + * the kernel porttion of the address space. PGDs "portion" > + * are pre-populated and we never clear them. > + */ > + WARN_ON_ONCE(1); > + } > + } else { > + /* > + * _PAGE_USER was not set in either the PGD being set or > + * cleared. All kernel PGDs should be pre-populated so > + * this should never happen after boot. > + */ > + WARN_ON_ONCE(system_state == SYSTEM_RUNNING); > + } Btw, we could keep the warning and have a separate path kernel users like kernel_ident_mapping_init() (i.e., kexec, hibernation, et al) call to bypass the warning vs all the remaining users which call the default set_pgd(). > +#endif > + /* return the copy of the PGD we want the kernel to use: */ > + return pgd; > +} > + > + > static inline void native_set_p4d(p4d_t *p4dp, p4d_t p4d) > { > +#if defined(CONFIG_KERNEL_PAGE_TABLE_ISOLATION) && !defined(CONFIG_X86_5LEVEL) > + p4dp->pgd = kpti_set_user_pgd(&p4dp->pgd, p4d.pgd); > +#else > *p4dp = p4d; > +#endif > } > > static inline void native_p4d_clear(p4d_t *p4d) > @@ -147,7 +282,11 @@ static inline void native_p4d_clear(p4d_ > > static inline void native_set_pgd(pgd_t *pgdp, pgd_t pgd) > { > +#ifdef CONFIG_KERNEL_PAGE_TABLE_ISOLATION > + *pgdp = kpti_set_user_pgd(pgdp, pgd); > +#else > *pgdp = pgd; > +#endif I guess that ifdef is not needed as kpti_set_user_pgd() already has it so we can do simply: static inline void native_set_pgd(pgd_t *pgdp, pgd_t pgd) { *pgdp = kpti_set_user_pgd(pgdp, pgd); } AFAICT. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --