[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJcbSZHiL3OmswxAJ9gVkC9a4O01qdiy7yjsb5dvofhOn8s+8Q@mail.gmail.com>
Date: Mon, 6 Feb 2017 14:10:36 -0800
From: Thomas Garnier <thgarnie@...gle.com>
To: Andy Lutomirski <luto@...nel.org>
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>, 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 Wed, Feb 1, 2017 at 9:14 PM, Andy Lutomirski <luto@...nel.org> wrote:
> 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.
>
I tried but I can't get it working properly because the gdt field is
an array, not a pointer. For example with this_cpu_read(gdt_page.gdt),
I get:
./arch/x86/include/asm/desc.h: In function ‘get_current_gdt_rw’:
./include/linux/percpu-defs.h:308:21: error: incompatible types when
assigning to type ‘struct desc_struct[16]’ from type ‘struct
desc_struct *’
case 1: pscr_ret__ = stem##1(variable); break; \
^
I tried different variants without success. What do you think?
>> +/*
>> + * 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.
--
Thomas
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.
