Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 1 Feb 2017 21:14:26 -0800
From: Andy Lutomirski <luto@...nel.org>
To: Thomas Garnier <thgarnie@...gle.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, 
	"H . Peter Anvin" <hpa@...or.com>, Andrey Ryabinin <aryabinin@...tuozzo.com>, 
	Alexander Potapenko <glider@...gle.com>, Dmitry Vyukov <dvyukov@...gle.com>, 
	Kees Cook <keescook@...omium.org>, Andy Lutomirski <luto@...nel.org>, 
	Arjan van de Ven <arjan@...ux.intel.com>, Paul Gortmaker <paul.gortmaker@...driver.com>, 
	Borislav Petkov <bp@...e.de>, "Rafael J . Wysocki" <rjw@...ysocki.net>, Len Brown <len.brown@...el.com>, 
	Pavel Machek <pavel@....cz>, Jiri Kosina <jikos@...nel.org>, Matt Fleming <matt@...eblueprint.co.uk>, 
	Ard Biesheuvel <ard.biesheuvel@...aro.org>, Boris Ostrovsky <boris.ostrovsky@...cle.com>, 
	Juergen Gross <jgross@...e.com>, Rusty Russell <rusty@...tcorp.com.au>, 
	Christian Borntraeger <borntraeger@...ibm.com>, Fenghua Yu <fenghua.yu@...el.com>, 
	He Chen <he.chen@...ux.intel.com>, Brian Gerst <brgerst@...il.com>, 
	"Luis R . Rodriguez" <mcgrof@...nel.org>, Adam Buchbinder <adam.buchbinder@...il.com>, 
	Stanislaw Gruszka <sgruszka@...hat.com>, Arnd Bergmann <arnd@...db.de>, Dave Hansen <dave.hansen@...el.com>, 
	Chen Yucong <slaoub@...il.com>, Vitaly Kuznetsov <vkuznets@...hat.com>, 
	David Vrabel <david.vrabel@...rix.com>, Josh Poimboeuf <jpoimboe@...hat.com>, 
	Tim Chen <tim.c.chen@...ux.intel.com>, Rik van Riel <riel@...hat.com>, 
	Andi Kleen <ak@...ux.intel.com>, Jiri Olsa <jolsa@...hat.com>, 
	Prarit Bhargava <prarit@...hat.com>, Michael Ellerman <mpe@...erman.id.au>, Joerg Roedel <joro@...tes.org>, 
	Paolo Bonzini <pbonzini@...hat.com>, Radim Krčmář <rkrcmar@...hat.com>, 
	X86 ML <x86@...nel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, kasan-dev <kasan-dev@...glegroups.com>, 
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>, 
	"linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>, 
	"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>, lguest@...ts.ozlabs.org, 
	kvm list <kvm@...r.kernel.org>, 
	"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH v2 3/3] x86: Make the GDT remapping read-only on 64 bit

On Thu, Jan 26, 2017 at 8:59 AM, Thomas Garnier <thgarnie@...gle.com> wrote:
> This patch makes the GDT remapped pages read-only to prevent corruption.
> This change is done only on 64 bit.
>
> The native_load_tr_desc function was adapted to correctly handle a
> read-only GDT. The LTR instruction always writes to the GDT TSS entry.
> This generates a page fault if the GDT is read-only. This change checks
> if the current GDT is a remap and swap GDTs as needed. This function was
> tested by booting multiple machines and checking hibernation works
> properly.
>
> KVM SVM and VMX were adapted to use the writeable GDT. On VMX, the
> per-cpu variable was removed for functions to fetch the original GDT.
> Instead of reloading the previous GDT, VMX will reload the fixmap GDT as
> expected. For testing, VMs were started and restored on multiple
> configurations.
>
> Signed-off-by: Thomas Garnier <thgarnie@...gle.com>
> ---
> Based on next-20170125
> ---
>  arch/x86/include/asm/desc.h      | 46 +++++++++++++++++++++++++++++++++++-----
>  arch/x86/include/asm/processor.h |  1 +
>  arch/x86/kernel/cpu/common.c     | 28 ++++++++++++++++++------
>  arch/x86/kvm/svm.c               |  4 +---
>  arch/x86/kvm/vmx.c               | 15 +++++--------
>  5 files changed, 70 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
> index 4cc176f57b78..ca7b2224fcb4 100644
> --- a/arch/x86/include/asm/desc.h
> +++ b/arch/x86/include/asm/desc.h
> @@ -52,6 +52,12 @@ static inline struct desc_struct *get_cpu_direct_gdt(unsigned int cpu)
>         return per_cpu(gdt_page, cpu).gdt;
>  }
>
> +/* Provide the current original GDT */
> +static inline struct desc_struct *get_current_direct_gdt(void)
> +{
> +       return this_cpu_ptr(&gdt_page)->gdt;
> +}

I'm assuming that the reason that this isn't part of patch 2 and used
instead of the version that takes cpu as a parameter is that TLS
doesn't work until the GDT is set up.  If so, perhaps that's worthy of
a comment in patch 2.

But give this_cpu_read(gdt_page.gdt) a try, please.

> +/*
> + * The LTR instruction marks the TSS GDT entry as busy. In 64bit, the GDT is
> + * a read-only remapping. To prevent a page fault, the GDT is switched to the
> + * original writeable version when needed.
> + */
> +#ifdef CONFIG_X86_64
> +static inline void native_load_tr_desc(void)
> +{
> +       struct desc_ptr gdt;
> +       int cpu = raw_smp_processor_id();
> +       bool restore = false;
> +       struct desc_struct *fixmap_gdt;
> +
> +       native_store_gdt(&gdt);

Off the top of my head, this is something like 10 cycles.  IMO that's
fast enough not to worry about the regression this will cause to KVM
exits.  In any event, we'll get that back and *much* more when we do
the optimizations that this series enables.

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.