[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJcbSZHQdWF36cgbgjcq=2rEDTc54PizFtjBvMxE9hUJsBaf3g@mail.gmail.com>
Date: Fri, 20 Jan 2017 17:06:54 -0800
From: Thomas Garnier <thgarnie@...gle.com>
To: Andy Lutomirski <luto@...capital.net>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
"H . Peter Anvin" <hpa@...or.com>, Kees Cook <keescook@...omium.org>,
"Rafael J . Wysocki" <rjw@...ysocki.net>, Pavel Machek <pavel@....cz>, Andy Lutomirski <luto@...nel.org>,
Borislav Petkov <bp@...e.de>, Christian Borntraeger <borntraeger@...ibm.com>, Brian Gerst <brgerst@...il.com>,
He Chen <he.chen@...ux.intel.com>, Dave Hansen <dave.hansen@...el.com>,
Chen Yucong <slaoub@...il.com>, Baoquan He <bhe@...hat.com>,
Paul Gortmaker <paul.gortmaker@...driver.com>, Joerg Roedel <joro@...tes.org>,
Paolo Bonzini <pbonzini@...hat.com>, Radim Krčmář <rkrcmar@...hat.com>,
Fenghua Yu <fenghua.yu@...el.com>, X86 ML <x86@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>, kvm list <kvm@...r.kernel.org>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH v1 2/3] x86: Remap GDT tables in the Fixmap section
On Fri, Jan 20, 2017 at 4:57 PM, Andy Lutomirski <luto@...capital.net> wrote:
> On Fri, Jan 20, 2017 at 8:41 AM, Thomas Garnier <thgarnie@...gle.com> wrote:
>> Each processor holds a GDT in its per-cpu structure. The sgdt
>> instruction gives the base address of the current GDT. This address can
>> be used to bypass KASLR memory randomization. With another bug, an
>> attacker could target other per-cpu structures or deduce the base of
>> the main memory section (PAGE_OFFSET).
>>
>> This patch relocates the GDT table for each processor inside the
>> Fixmap section. The space is reserved based on number of supported
>> cpus.
>>
>> For consistency, the remapping is done by default on 32 and 64 bit.
>>
>> Each processor switches to its remapped GDT at the end of
>> initialization. For hibernation, the main processor returns with the
>> original GDT and switches back to the remapping at completion.
>>
>> On 32 bit, the maximum number of processors is now 256. The Fixmap
>> section cannot handle the original 512. Additional asserts ensure that
>> the Fixmap section cannot grow beyond the space available.
>>
>> This patch was tested on both architectures. Hibernation and KVM were
>> both tested specially for their usage of the GDT.
>>
>> Signed-off-by: Thomas Garnier <thgarnie@...gle.com>
>> ---
>> Based on next-20170119
>> ---
>> arch/x86/Kconfig | 1 +
>> arch/x86/include/asm/fixmap.h | 4 ++++
>> arch/x86/include/asm/processor.h | 1 +
>> arch/x86/kernel/cpu/common.c | 18 +++++++++++++++++-
>> arch/x86/mm/init_32.c | 2 ++
>> arch/x86/power/cpu.c | 3 +++
>> 6 files changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index f1d4e8f2131f..b4ed35db10a8 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -912,6 +912,7 @@ config MAXSMP
>> config NR_CPUS
>> int "Maximum number of CPUs" if SMP && !MAXSMP
>> range 2 8 if SMP && X86_32 && !X86_BIGSMP
>> + range 2 256 if SMP && X86_32 && X86_BIGSMP
>> range 2 512 if SMP && !MAXSMP && !CPUMASK_OFFSTACK
>> range 2 8192 if SMP && !MAXSMP && CPUMASK_OFFSTACK && X86_64
>> default "1" if !SMP
>> diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
>> index c46289799b02..8b913b5e9383 100644
>> --- a/arch/x86/include/asm/fixmap.h
>> +++ b/arch/x86/include/asm/fixmap.h
>> @@ -100,6 +100,10 @@ enum fixed_addresses {
>> #ifdef CONFIG_X86_INTEL_MID
>> FIX_LNW_VRTC,
>> #endif
>> + /* Fixmap entries to remap the GDTs, one per processor. */
>> + FIX_GDT_REMAP_BEGIN,
>> + FIX_GDT_REMAP_END = FIX_GDT_REMAP_BEGIN + NR_CPUS - 1,
>> +
>> __end_of_permanent_fixed_addresses,
>>
>> /*
>> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
>> index 1be64da0384e..280211ad8be9 100644
>> --- a/arch/x86/include/asm/processor.h
>> +++ b/arch/x86/include/asm/processor.h
>> @@ -705,6 +705,7 @@ extern struct desc_ptr early_gdt_descr;
>>
>> extern void cpu_set_gdt(int);
>> extern void switch_to_new_gdt(int);
>> +extern void load_remapped_gdt(int);
>> extern void load_percpu_segment(int);
>> extern void cpu_init(void);
>>
>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>> index e97ffc8d29f4..7d940b0e805a 100644
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -443,6 +443,19 @@ void load_percpu_segment(int cpu)
>> load_stack_canary_segment();
>> }
>>
>> +/* Load a fixmap remapping of the per-cpu GDT */
>> +void load_remapped_gdt(int cpu)
>> +{
>> + struct desc_ptr gdt_descr;
>> + unsigned long idx = FIX_GDT_REMAP_BEGIN + cpu;
>> +
>> + __set_fixmap(idx, __pa(get_cpu_gdt_table(cpu)), PAGE_KERNEL);
>> +
>> + gdt_descr.address = (long)__fix_to_virt(idx);
>> + gdt_descr.size = GDT_SIZE - 1;
>> + load_gdt(&gdt_descr);
>> +}
>
> IMO this should be split into two functions, one to set up the fixmap
> entry and one to load the GDT.
>
That make sense.
> Also, would it be possible to rename the various gdt helpers so that
> their functionality is more obvious? For example, get_cpu_gdt_table()
> could be get_cpu_direct_gdt_table() and the function to load the gdt
> could be load_fixmap_gdt().
>
Sure no problem.
> --Andy
--
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.
