From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751752AbdKYBSF (ORCPT ); Fri, 24 Nov 2017 20:18:05 -0500 Received: from smtp-fw-33001.amazon.com ([207.171.190.10]:61505 "EHLO smtp-fw-33001.amazon.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751080AbdKYBSD (ORCPT ); Fri, 24 Nov 2017 20:18:03 -0500 X-IronPort-AV: E=Sophos;i="5.44,450,1505779200"; d="scan'208";a="704720236" Date: Fri, 24 Nov 2017 17:17:55 -0800 From: Eduardo Valentin To: Dave Hansen CC: , , , , , , , , , , , Subject: Re: [PATCH 21/23] x86, kaiser: un-poison PGDs at runtime Message-ID: <20171125011755.GC2017@u40b0340c692b58f6553c.ant.amazon.com> References: <20171123003438.48A0EEDE@viggo.jf.intel.com> <20171123003521.A90AC3AF@viggo.jf.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20171123003521.A90AC3AF@viggo.jf.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 22, 2017 at 04:35:21PM -0800, Dave Hansen wrote: > > From: Dave Hansen > > With KAISER Kernel PGDs that map userspace are "poisoned" with > the NX bit. This ensures that if a kernel->user CR3 switch is > missed, userspace crashes instead of running in an unhardened > state. > > This code will be needed in a moment when KAISER is turned > on and off at runtime. > > Note that an __ASSEMBLY__ #ifdef is now required since kaiser.h > is indirectly included into assembly. > > Signed-off-by: Dave Hansen > Cc: Moritz Lipp > Cc: Daniel Gruss > Cc: Michael Schwarz > Cc: Richard Fellner > Cc: Andy Lutomirski > Cc: Linus Torvalds > Cc: Kees Cook > Cc: Hugh Dickins > Cc: x86@kernel.org > --- > > b/arch/x86/include/asm/pgtable_64.h | 16 ++++++++++++++- > b/arch/x86/mm/kaiser.c | 38 ++++++++++++++++++++++++++++++++++++ > b/include/linux/kaiser.h | 3 +- > 3 files changed, 55 insertions(+), 2 deletions(-) > > diff -puN arch/x86/include/asm/pgtable_64.h~kaiser-dynamic-unpoison-pgd arch/x86/include/asm/pgtable_64.h > --- a/arch/x86/include/asm/pgtable_64.h~kaiser-dynamic-unpoison-pgd 2017-11-22 15:45:55.818619722 -0800 > +++ b/arch/x86/include/asm/pgtable_64.h 2017-11-22 15:45:55.824619722 -0800 > @@ -3,6 +3,7 @@ > #define _ASM_X86_PGTABLE_64_H > > #include > +#include > #include > > #ifndef __ASSEMBLY__ > @@ -199,6 +200,18 @@ static inline bool pgd_userspace_access( > return pgd.pgd & _PAGE_USER; > } > > +static inline void kaiser_poison_pgd(pgd_t *pgd) > +{ > + if (pgd->pgd & _PAGE_PRESENT) > + pgd->pgd |= _PAGE_NX; > +} > + > +static inline void kaiser_unpoison_pgd(pgd_t *pgd) > +{ > + if (pgd->pgd & _PAGE_PRESENT) > + pgd->pgd &= ~_PAGE_NX; > +} > + > /* > * Take a PGD location (pgdp) and a pgd value that needs > * to be set there. Populates the shadow and returns > @@ -222,7 +235,8 @@ static inline pgd_t kaiser_set_shadow_pg > * wrong CR3 value, userspace will crash > * instead of running. > */ > - pgd.pgd |= _PAGE_NX; > + if (kaiser_active()) > + kaiser_poison_pgd(&pgd); > } > } else if (pgd_userspace_access(*pgdp)) { > /* > diff -puN arch/x86/mm/kaiser.c~kaiser-dynamic-unpoison-pgd arch/x86/mm/kaiser.c > --- a/arch/x86/mm/kaiser.c~kaiser-dynamic-unpoison-pgd 2017-11-22 15:45:55.819619722 -0800 > +++ b/arch/x86/mm/kaiser.c 2017-11-22 15:45:55.825619722 -0800 > @@ -501,6 +501,9 @@ static ssize_t kaiser_enabled_write_file > if (enable > 1) > return -EINVAL; > > + if (kaiser_enabled == enable) > + return count; > + > WRITE_ONCE(kaiser_enabled, enable); > return count; > } Shouldn't the above hunk be part of the patch that adds the debugfs entry? > @@ -518,3 +521,38 @@ static int __init create_kaiser_enabled( > return 0; > } > late_initcall(create_kaiser_enabled); > + > +enum poison { > + KAISER_POISON, > + KAISER_UNPOISON > +}; > +void kaiser_poison_pgd_page(pgd_t *pgd_page, enum poison do_poison) > +{ > + int i = 0; > + > + for (i = 0; i < PTRS_PER_PGD; i++) { > + pgd_t *pgd = &pgd_page[i]; > + > + /* Stop once we hit kernel addresses: */ > + if (!pgdp_maps_userspace(pgd)) > + break; > + > + if (do_poison == KAISER_POISON) > + kaiser_poison_pgd(pgd); > + else > + kaiser_unpoison_pgd(pgd); > + } > + > +} > + > +void kaiser_poison_pgds(enum poison do_poison) > +{ > + struct page *page; > + > + spin_lock(&pgd_lock); > + list_for_each_entry(page, &pgd_list, lru) { > + pgd_t *pgd = (pgd_t *)page_address(page); > + kaiser_poison_pgd_page(pgd, do_poison); > + } > + spin_unlock(&pgd_lock); > +} > diff -puN include/linux/kaiser.h~kaiser-dynamic-unpoison-pgd include/linux/kaiser.h > --- a/include/linux/kaiser.h~kaiser-dynamic-unpoison-pgd 2017-11-22 15:45:55.821619722 -0800 > +++ b/include/linux/kaiser.h 2017-11-22 15:45:55.826619722 -0800 > @@ -4,7 +4,7 @@ > #ifdef CONFIG_KAISER > #include > #else > - > +#ifndef __ASSEMBLY__ > /* > * These stubs are used whenever CONFIG_KAISER is off, which > * includes architectures that support KAISER, but have it > @@ -33,5 +33,6 @@ static inline bool kaiser_active(void) > { > return 0; > } > +#endif /* __ASSEMBLY__ */ > #endif /* !CONFIG_KAISER */ > #endif /* _INCLUDE_KAISER_H */ > _ > -- All the best, Eduardo Valentin