Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1551446 > unrolled thread

[RFC] x86/mm/KASLR: Remap GDTs at fixed location

Back to article view | Back to linux.kernel

Contents

  [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-04 23:20 +0100
    Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Ingo Molnar <mingo@kernel.org> - 2017-01-05 09:30 +0100
      Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Arjan van de Ven <arjan@linux.intel.com> - 2017-01-05 16:10 +0100
        Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-05 17:50 +0100
          Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Arjan van de Ven <arjan@linux.intel.com> - 2017-01-05 20:10 +0100
            Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-05 20:10 +0100
          Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Borislav Petkov <bp@alien8.de> - 2017-01-06 18:50 +0100
            Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-06 19:10 +0100
      Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-05 17:40 +0100
        Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Ingo Molnar <mingo@kernel.org> - 2017-01-06 07:40 +0100
    Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-05 19:20 +0100
      Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Andy Lutomirski <luto@amacapital.net> - 2017-01-05 19:40 +0100
        Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-05 19:40 +0100
      Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Arjan van de Ven <arjan@linux.intel.com> - 2017-01-05 20:10 +0100
        Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-05 20:20 +0100
          Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Andy Lutomirski <luto@kernel.org> - 2017-01-05 21:20 +0100
            Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-05 22:10 +0100
              Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Andy Lutomirski <luto@amacapital.net> - 2017-01-05 22:30 +0100
                Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-05 23:00 +0100
                  Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Ingo Molnar <mingo@kernel.org> - 2017-01-06 08:00 +0100
                    Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-06 19:10 +0100
                      Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Andy Lutomirski <luto@kernel.org> - 2017-01-06 23:30 +0100
                        Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-07 00:00 +0100
                          Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Andy Lutomirski <luto@kernel.org> - 2017-01-07 00:40 +0100
                            Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Ingo Molnar <mingo@kernel.org> - 2017-01-07 08:50 +0100
                              Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Andy Lutomirski <luto@amacapital.net> - 2017-01-07 17:00 +0100
                          Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Ingo Molnar <mingo@kernel.org> - 2017-01-07 08:40 +0100
                            Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Andy Lutomirski <luto@amacapital.net> - 2017-01-07 17:10 +0100
                            Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-09 23:40 +0100
                              Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Ingo Molnar <mingo@kernel.org> - 2017-01-10 11:30 +0100
                                Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-10 18:20 +0100
            Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Linus Torvalds <torvalds@linux-foundation.org> - 2017-01-06 00:10 +0100
              Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-06 00:30 +0100
              Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Andy Lutomirski <luto@amacapital.net> - 2017-01-06 03:40 +0100
                Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Thomas Garnier <thgarnie@google.com> - 2017-01-06 19:10 +0100
                  Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Andy Lutomirski <luto@kernel.org> - 2017-01-06 23:00 +0100
                    Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Ingo Molnar <mingo@kernel.org> - 2017-01-07 08:50 +0100
        Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Ingo Molnar <mingo@kernel.org> - 2017-01-06 07:50 +0100
    Re: [RFC] x86/mm/KASLR: Remap GDTs at fixed location Andy Lutomirski <luto@amacapital.net> - 2017-01-05 19:30 +0100

Page 1 of 2  [1] 2  Next page →

#1551446 — [RFC] x86/mm/KASLR: Remap GDTs at fixed location

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).

In this change, a space is reserved at the end of the memory range
available for KASLR memory randomization. The space is big enough to hold
the maximum number of CPUs (as defined by setup_max_cpus). Each GDT is
mapped at specific offset based on the target CPU. Note that if there is
not enough space available, the GDTs are not remapped.

The document was changed to mention GDT remapping for KASLR. This patch
also include dump page tables support.

This patch was tested on multiple hardware configurations and for
hibernation support.

Signed-off-by: Thomas Garnier <thgarnie@google.com>
---
Based on next-20170104
---
 arch/x86/include/asm/kaslr.h  |   4 ++
 arch/x86/kernel/cpu/common.c  |   7 ++-
 arch/x86/mm/dump_pagetables.c |  10 ++++
 arch/x86/mm/kaslr.c           | 107 +++++++++++++++++++++++++++++++++++++++++-
 kernel/cpu.c                  |   3 ++
 kernel/smp.c                  |   1 +
 6 files changed, 130 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kaslr.h b/arch/x86/include/asm/kaslr.h
index 1052a797d71d..babc32803182 100644
--- a/arch/x86/include/asm/kaslr.h
+++ b/arch/x86/include/asm/kaslr.h
@@ -9,8 +9,12 @@ extern unsigned long vmalloc_base;
 extern unsigned long vmemmap_base;
 
 void kernel_randomize_memory(void);
+void kernel_randomize_smp(void);
+void* kaslr_get_gdt_remap(int cpu);
 #else
 static inline void kernel_randomize_memory(void) { }
+static inline void kernel_randomize_smp(void) { }
+static inline void *kaslr_get_gdt_remap(int cpu) { return NULL; }
 #endif /* CONFIG_RANDOMIZE_MEMORY */
 
 #endif
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index dc1697ca5191..2c8a7b4718ea 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -450,8 +450,13 @@ void load_percpu_segment(int cpu)
 void switch_to_new_gdt(int cpu)
 {
 	struct desc_ptr gdt_descr;
+	struct desc_struct *gdt;
 
-	gdt_descr.address = (long)get_cpu_gdt_table(cpu);
+	gdt = kaslr_get_gdt_remap(cpu);
+	if (!gdt)
+		gdt = get_cpu_gdt_table(cpu);
+
+	gdt_descr.address = (long)gdt;
 	gdt_descr.size = GDT_SIZE - 1;
 	load_gdt(&gdt_descr);
 	/* Reload the per-cpu base */
diff --git a/arch/x86/mm/dump_pagetables.c b/arch/x86/mm/dump_pagetables.c
index ea9c49adaa1f..213fe01f28dc 100644
--- a/arch/x86/mm/dump_pagetables.c
+++ b/arch/x86/mm/dump_pagetables.c
@@ -50,6 +50,9 @@ enum address_markers_idx {
 	LOW_KERNEL_NR,
 	VMALLOC_START_NR,
 	VMEMMAP_START_NR,
+# ifdef CONFIG_RANDOMIZE_MEMORY
+	GDT_REMAP_NR,
+# endif
 # ifdef CONFIG_X86_ESPFIX64
 	ESPFIX_START_NR,
 # endif
@@ -75,6 +78,9 @@ static struct addr_marker address_markers[] = {
 	{ 0/* PAGE_OFFSET */,   "Low Kernel Mapping" },
 	{ 0/* VMALLOC_START */, "vmalloc() Area" },
 	{ 0/* VMEMMAP_START */, "Vmemmap" },
+# ifdef CONFIG_RANDOMIZE_MEMORY
+	{ 0, "GDT remapping" },
+# endif
 # ifdef CONFIG_X86_ESPFIX64
 	{ ESPFIX_BASE_ADDR,	"ESPfix Area", 16 },
 # endif
@@ -442,6 +448,10 @@ static int __init pt_dump_init(void)
 	address_markers[LOW_KERNEL_NR].start_address = PAGE_OFFSET;
 	address_markers[VMALLOC_START_NR].start_address = VMALLOC_START;
 	address_markers[VMEMMAP_START_NR].start_address = VMEMMAP_START;
+#ifdef CONFIG_RANDOMIZE_MEMORY
+	address_markers[GDT_REMAP_NR].start_address =
+		(unsigned long) kaslr_get_gdt_remap(0);
+#endif
 #endif
 #ifdef CONFIG_X86_32
 	address_markers[VMALLOC_START_NR].start_address = VMALLOC_START;
diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
index 887e57182716..db1bdb75f8af 100644
--- a/arch/x86/mm/kaslr.c
+++ b/arch/x86/mm/kaslr.c
@@ -22,11 +22,13 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/random.h>
+#include <linux/slab.h>
 
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>
 #include <asm/setup.h>
 #include <asm/kaslr.h>
+#include <asm/desc.h>
 
 #include "mm_internal.h"
 
@@ -60,6 +62,7 @@ unsigned long vmalloc_base = __VMALLOC_BASE;
 EXPORT_SYMBOL(vmalloc_base);
 unsigned long vmemmap_base = __VMEMMAP_BASE;
 EXPORT_SYMBOL(vmemmap_base);
+unsigned long gdt_tables_base = 0;
 
 /*
  * Memory regions randomized by KASLR (except modules that use a separate logic
@@ -97,7 +100,7 @@ void __init kernel_randomize_memory(void)
 	unsigned long vaddr = vaddr_start;
 	unsigned long rand, memory_tb;
 	struct rnd_state rand_state;
-	unsigned long remain_entropy;
+	unsigned long remain_entropy, gdt_reserved;
 
 	/*
 	 * All these BUILD_BUG_ON checks ensures the memory layout is
@@ -131,6 +134,13 @@ void __init kernel_randomize_memory(void)
 	for (i = 0; i < ARRAY_SIZE(kaslr_regions); i++)
 		remain_entropy -= get_padding(&kaslr_regions[i]);
 
+	/* Reserve space for fixed GDTs, if we have enough available */
+	gdt_reserved = sizeof(struct gdt_page) * max(setup_max_cpus, 1U);
+	if (gdt_reserved < remain_entropy) {
+		gdt_tables_base = vaddr_end - gdt_reserved;
+		remain_entropy -= gdt_reserved;
+	}
+
 	prandom_seed_state(&rand_state, kaslr_get_random_long("Memory"));
 
 	for (i = 0; i < ARRAY_SIZE(kaslr_regions); i++) {
@@ -192,3 +202,98 @@ void __meminit init_trampoline(void)
 	set_pgd(&trampoline_pgd_entry,
 		__pgd(_KERNPG_TABLE | __pa(pud_page_tramp)));
 }
+
+/* Hold the remapping of the gdt page for each cpu */
+DEFINE_PER_CPU_PAGE_ALIGNED(struct desc_struct *, gdt_remap);
+
+/* Return the address where the GDT is remapped for this CPU */
+static unsigned long gdt_remap_address(int cpu)
+{
+	return gdt_tables_base + cpu * sizeof(struct gdt_page);
+}
+
+/* Remap the specified gdt table */
+static struct desc_struct *remap_gdt(int cpu)
+{
+	pgd_t *pgd;
+	pud_t *pud;
+	pmd_t *pmd;
+	pte_t *pte;
+	struct desc_struct *gdt;
+	unsigned long addr;
+
+	/* GDT table should be only one page */
+	BUILD_BUG_ON(sizeof(struct gdt_page) != PAGE_SIZE);
+
+	/* Keep the original GDT before the allocator is available */
+	if (!slab_is_available())
+		return NULL;
+
+	gdt = get_cpu_gdt_table(cpu);
+	addr = gdt_remap_address(cpu);
+
+	pgd = pgd_offset_k(addr);
+	pud = pud_alloc(&init_mm, pgd, addr);
+	if (WARN_ON(!pud))
+		return NULL;
+	pmd = pmd_alloc(&init_mm, pud, addr);
+	if (WARN_ON(!pmd))
+		return NULL;
+	pte = pte_alloc_kernel(pmd, addr);
+	if (WARN_ON(!pte))
+		return NULL;
+
+	/* If the PTE is already set, something is wrong with the VA ranges */
+	BUG_ON(!pte_none(*pte));
+
+	/* Remap the target GDT and return it */
+	set_pte_at(&init_mm, addr, pte,
+		   pfn_pte(PFN_DOWN(__pa(gdt)), PAGE_KERNEL));
+	gdt = (struct desc_struct *)addr;
+	per_cpu(gdt_remap, cpu) = gdt;
+	return gdt;
+}
+
+/* Check if GDT remapping is enabled */
+static bool kaslr_gdt_remap_enabled(void)
+{
+	return kaslr_memory_enabled() && gdt_tables_base != 0;
+}
+
+/*
+ * The GDT table address is available to user-mode through the sgdt
+ * instruction. This function will return a fixed remapping to load so you
+ * cannot leak the per-cpu structure address.
+ */
+void* kaslr_get_gdt_remap(int cpu)
+{
+	struct desc_struct *gdt_remapping;
+
+	if (!kaslr_gdt_remap_enabled())
+		return NULL;
+
+	gdt_remapping = per_cpu(gdt_remap, cpu);
+	if (!gdt_remapping)
+		gdt_remapping = remap_gdt(cpu);
+
+	return gdt_remapping;
+}
+
+/*
+ * Switch the first processor GDT to the remapping. The GDT is loaded too early
+ * to generate the remapping correctly. This step is done later at boot or
+ * before other processors come back from hibernation.
+ */
+void kernel_randomize_smp(void)
+{
+	struct desc_ptr gdt_descr;
+	struct desc_struct *gdt;
+
+	gdt = kaslr_get_gdt_remap(raw_smp_processor_id());
+	if (WARN_ON(!gdt))
+		return;
+
+	gdt_descr.address = (long)gdt;
+	gdt_descr.size = GDT_SIZE - 1;
+	load_gdt(&gdt_descr);
+}
diff --git a/kernel/cpu.c b/kernel/cpu.c
index f75c4d031eeb..4d6979299b9a 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1040,6 +1040,9 @@ void enable_nonboot_cpus(void)
 {
 	int cpu, error;
 
+	/* Redo KASLR steps for main processor */
+	kernel_randomize_smp();
+
 	/* Allow everyone to use the CPU hotplug again */
 	cpu_maps_update_begin();
 	__cpu_hotplug_enable();
diff --git a/kernel/smp.c b/kernel/smp.c
index 77fcdb9f2775..e1ef8d05e179 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -554,6 +554,7 @@ void __init smp_init(void)
 
 	idle_threads_init();
 	cpuhp_threads_init();
+	kernel_randomize_smp();
 
 	pr_info("Bringing up secondary CPUs ...\n");
 
-- 
2.11.0.390.gc69c2f50cf-goog

[toc] | [next] | [standalone]

#1551742

* Thomas Garnier <thgarnie@google.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).
> 
> In this change, a space is reserved at the end of the memory range
> available for KASLR memory randomization. The space is big enough to hold
> the maximum number of CPUs (as defined by setup_max_cpus). Each GDT is
> mapped at specific offset based on the target CPU. Note that if there is
> not enough space available, the GDTs are not remapped.
> 
> The document was changed to mention GDT remapping for KASLR. This patch
> also include dump page tables support.
> 
> This patch was tested on multiple hardware configurations and for
> hibernation support.

>  void kernel_randomize_memory(void);
> +void kernel_randomize_smp(void);
> +void* kaslr_get_gdt_remap(int cpu);

Yeah, no fundamental objections from me to the principle, but I get some bad vibes 
from the naming here: seeing that kernel_randomize_smp() actually makes things 
less random.

Also, don't we want to do this unconditionally and not allow remapping failures? 

The GDT is fairly small, plus making the SGDT instruction expose fewer kernel 
internals would be (marginally) useful on non-randomized kernels as well.

It also makes the code more common, more predictable, more debuggable and less 
complex overall - which is pretty valuable in terms of long term security as well.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1552038

On 1/5/2017 12:11 AM, Ingo Molnar wrote:
>
> * Thomas Garnier <thgarnie@google.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).
>>
>> In this change, a space is reserved at the end of the memory range
>> available for KASLR memory randomization. The space is big enough to hold
>> the maximum number of CPUs (as defined by setup_max_cpus). Each GDT is
>> mapped at specific offset based on the target CPU. Note that if there is
>> not enough space available, the GDTs are not remapped.
>>
>> The document was changed to mention GDT remapping for KASLR. This patch
>> also include dump page tables support.
>>
>> This patch was tested on multiple hardware configurations and for
>> hibernation support.
>
>>  void kernel_randomize_memory(void);
>> +void kernel_randomize_smp(void);
>> +void* kaslr_get_gdt_remap(int cpu);
>
> Yeah, no fundamental objections from me to the principle, but I get some bad vibes
> from the naming here: seeing that kernel_randomize_smp() actually makes things
> less random.
>

kernel_unrandomize_smp() ...

one request.. can we make sure this unrandomization is optional?

[toc] | [prev] | [next] | [standalone]

#1552108

On Thu, Jan 5, 2017 at 7:08 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On 1/5/2017 12:11 AM, Ingo Molnar wrote:
>>
>>
>> * Thomas Garnier <thgarnie@google.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).
>>>
>>> In this change, a space is reserved at the end of the memory range
>>> available for KASLR memory randomization. The space is big enough to hold
>>> the maximum number of CPUs (as defined by setup_max_cpus). Each GDT is
>>> mapped at specific offset based on the target CPU. Note that if there is
>>> not enough space available, the GDTs are not remapped.
>>>
>>> The document was changed to mention GDT remapping for KASLR. This patch
>>> also include dump page tables support.
>>>
>>> This patch was tested on multiple hardware configurations and for
>>> hibernation support.
>>
>>
>>>  void kernel_randomize_memory(void);
>>> +void kernel_randomize_smp(void);
>>> +void* kaslr_get_gdt_remap(int cpu);
>>
>>
>> Yeah, no fundamental objections from me to the principle, but I get some
>> bad vibes
>> from the naming here: seeing that kernel_randomize_smp() actually makes
>> things
>> less random.
>>
>
> kernel_unrandomize_smp() ...
>

That seems like a better name.

> one request.. can we make sure this unrandomization is optional?
>

Well, it happens only when KASLR memory randomization is enabled. Do
you think it should have a separate config option?

-- 
Thomas

[toc] | [prev] | [next] | [standalone]

#1552238

On 1/5/2017 8:40 AM, Thomas Garnier wrote:
> Well, it happens only when KASLR memory randomization is enabled. Do
> you think it should have a separate config option?

no I would want it a runtime option.... "sgdt from ring 3" is going away
with UMIP (and is already possibly gone in virtual machines, see
https://lwn.net/Articles/694385/) and for those cases it would be a shame
to lose the randomization

[toc] | [prev] | [next] | [standalone]

#1552239

On Thu, Jan 5, 2017 at 10:56 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On 1/5/2017 8:40 AM, Thomas Garnier wrote:
>>
>> Well, it happens only when KASLR memory randomization is enabled. Do
>> you think it should have a separate config option?
>
>
> no I would want it a runtime option.... "sgdt from ring 3" is going away
> with UMIP (and is already possibly gone in virtual machines, see
> https://lwn.net/Articles/694385/) and for those cases it would be a shame
> to lose the randomization
>

That's correct. When UMIP is enabled, we should disable fixed location
for both GDT and IDT. Glad to do that when UMIP support is added.

-- 
Thomas

[toc] | [prev] | [next] | [standalone]

#1552989

On Thu, Jan 05, 2017 at 08:40:29AM -0800, Thomas Garnier wrote:
> > kernel_unrandomize_smp() ...
> >
> 
> That seems like a better name.

Hardly... I'd call it something like kaslr_load_gdt() to actually say
what this function is doing.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1553003

On Fri, Jan 6, 2017 at 9:44 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Thu, Jan 05, 2017 at 08:40:29AM -0800, Thomas Garnier wrote:
>> > kernel_unrandomize_smp() ...
>> >
>>
>> That seems like a better name.
>
> Hardly... I'd call it something like kaslr_load_gdt() to actually say
> what this function is doing.
>

True, it loads it only for the main processor though. I will try a
variant like kaslr_load_main_gdt().

> --
> Regards/Gruss,
>     Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.



-- 
Thomas

[toc] | [prev] | [next] | [standalone]

#1552099

On Thu, Jan 5, 2017 at 12:11 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Thomas Garnier <thgarnie@google.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).
>>
>> In this change, a space is reserved at the end of the memory range
>> available for KASLR memory randomization. The space is big enough to hold
>> the maximum number of CPUs (as defined by setup_max_cpus). Each GDT is
>> mapped at specific offset based on the target CPU. Note that if there is
>> not enough space available, the GDTs are not remapped.
>>
>> The document was changed to mention GDT remapping for KASLR. This patch
>> also include dump page tables support.
>>
>> This patch was tested on multiple hardware configurations and for
>> hibernation support.
>
>>  void kernel_randomize_memory(void);
>> +void kernel_randomize_smp(void);
>> +void* kaslr_get_gdt_remap(int cpu);
>
> Yeah, no fundamental objections from me to the principle, but I get some bad vibes
> from the naming here: seeing that kernel_randomize_smp() actually makes things
> less random.
>

I agree, I went back and forth on the name. I will change it to
something better.

> Also, don't we want to do this unconditionally and not allow remapping failures?
>
> The GDT is fairly small, plus making the SGDT instruction expose fewer kernel
> internals would be (marginally) useful on non-randomized kernels as well.
>
> It also makes the code more common, more predictable, more debuggable and less
> complex overall - which is pretty valuable in terms of long term security as well.
>

Okay, I will add BUG_ON on failures to remap.

> Thanks,
>
>         Ingo

Ingo: I saw the 5-level page table support being sent through. Do you
want me to wait for it to be -next? (Given it will need to be changed
too).

-- 
Thomas

[toc] | [prev] | [next] | [standalone]

#1552554

* Thomas Garnier <thgarnie@google.com> wrote:

> > Thanks,
> >
> >         Ingo
> 
> Ingo: I saw the 5-level page table support being sent through. Do you
> want me to wait for it to be -next? (Given it will need to be changed
> too).

Please just base your bits on Linus's latest tree - we'll sort out any
conflicts as/when they happen.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1552195

On Thu, Jan 5, 2017 at 9:51 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, Jan 4, 2017 at 2:16 PM, Thomas Garnier <thgarnie@google.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).
>>
>> In this change, a space is reserved at the end of the memory range
>> available for KASLR memory randomization. The space is big enough to hold
>> the maximum number of CPUs (as defined by setup_max_cpus). Each GDT is
>> mapped at specific offset based on the target CPU. Note that if there is
>> not enough space available, the GDTs are not remapped.
>
> Can we remap it read-only?  I.e. use PAGE_KERNEL_RO instead of
> PAGE_KERNEL.  After all, the ability to modify the GDT is instant
> root.

That's my goal too. I started by doing a RO remap and got couple
problems with hibernation. I can try again for the next iteration or
delay it for another patch. I also need to look at KVM GDT usage, I am
not familiar with it yet.

-- 
Thomas

[toc] | [prev] | [next] | [standalone]

#1552206

On Thu, Jan 5, 2017 at 9:54 AM, Thomas Garnier <thgarnie@google.com> wrote:
> On Thu, Jan 5, 2017 at 9:51 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Wed, Jan 4, 2017 at 2:16 PM, Thomas Garnier <thgarnie@google.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).
>>>
>>> In this change, a space is reserved at the end of the memory range
>>> available for KASLR memory randomization. The space is big enough to hold
>>> the maximum number of CPUs (as defined by setup_max_cpus). Each GDT is
>>> mapped at specific offset based on the target CPU. Note that if there is
>>> not enough space available, the GDTs are not remapped.
>>
>> Can we remap it read-only?  I.e. use PAGE_KERNEL_RO instead of
>> PAGE_KERNEL.  After all, the ability to modify the GDT is instant
>> root.
>
> That's my goal too. I started by doing a RO remap and got couple
> problems with hibernation. I can try again for the next iteration or
> delay it for another patch. I also need to look at KVM GDT usage, I am
> not familiar with it yet.

If you want a small adventure, I think a significant KVM-related
performance improvement is available.  Specifically, on VMX exits, the
GDT limit is hardwired to 0xffff (IIRC -- I could be remembering the
actual vaue wrong).  KVM does LGDT to fix it.

If we actually made the GDT have limit 0xffff (presumably by mapping
the zero page a few times to pad it out without wasting memory), then
we would avoid the LGDT.  LGDT is incredibly slow, so this would be a
big win. Want to see if you can make this work with your patch set?

[toc] | [prev] | [next] | [standalone]

#1552209

On Thu, Jan 5, 2017 at 10:01 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Jan 5, 2017 at 9:54 AM, Thomas Garnier <thgarnie@google.com> wrote:
>> On Thu, Jan 5, 2017 at 9:51 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Wed, Jan 4, 2017 at 2:16 PM, Thomas Garnier <thgarnie@google.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).
>>>>
>>>> In this change, a space is reserved at the end of the memory range
>>>> available for KASLR memory randomization. The space is big enough to hold
>>>> the maximum number of CPUs (as defined by setup_max_cpus). Each GDT is
>>>> mapped at specific offset based on the target CPU. Note that if there is
>>>> not enough space available, the GDTs are not remapped.
>>>
>>> Can we remap it read-only?  I.e. use PAGE_KERNEL_RO instead of
>>> PAGE_KERNEL.  After all, the ability to modify the GDT is instant
>>> root.
>>
>> That's my goal too. I started by doing a RO remap and got couple
>> problems with hibernation. I can try again for the next iteration or
>> delay it for another patch. I also need to look at KVM GDT usage, I am
>> not familiar with it yet.
>
> If you want a small adventure, I think a significant KVM-related
> performance improvement is available.  Specifically, on VMX exits, the
> GDT limit is hardwired to 0xffff (IIRC -- I could be remembering the
> actual vaue wrong).  KVM does LGDT to fix it.
>
> If we actually made the GDT have limit 0xffff (presumably by mapping
> the zero page a few times to pad it out without wasting memory), then
> we would avoid the LGDT.  LGDT is incredibly slow, so this would be a
> big win. Want to see if you can make this work with your patch set?

I can always take a look. If you have any prototype or more details,
feel free to send it to me on a separate thread.

-- 
Thomas

[toc] | [prev] | [next] | [standalone]

#1552252

On 1/5/2017 9:54 AM, Thomas Garnier wrote:

>
> That's my goal too. I started by doing a RO remap and got couple
> problems with hibernation. I can try again for the next iteration or
> delay it for another patch. I also need to look at KVM GDT usage, I am
> not familiar with it yet.

don't we write to the GDT as part of the TLS segment stuff for glibc ?

[toc] | [prev] | [next] | [standalone]

#1552259

On Thu, Jan 5, 2017 at 10:58 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On 1/5/2017 9:54 AM, Thomas Garnier wrote:
>
>>
>> That's my goal too. I started by doing a RO remap and got couple
>> problems with hibernation. I can try again for the next iteration or
>> delay it for another patch. I also need to look at KVM GDT usage, I am
>> not familiar with it yet.
>
>
> don't we write to the GDT as part of the TLS segment stuff for glibc ?
>

Not sure which glibc feature it is.

In this design, you can write to the GDT per-cpu variable that will
remain read-write. You just need to make the remapping writeable when
we load task registers (ltr) then the processor use the current GDT
address. At least that the case I know, I might find more through
testing.

-- 
Thomas

[toc] | [prev] | [next] | [standalone]

#1552303

On Thu, Jan 5, 2017 at 11:03 AM, Thomas Garnier <thgarnie@google.com> wrote:
> On Thu, Jan 5, 2017 at 10:58 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
>> On 1/5/2017 9:54 AM, Thomas Garnier wrote:
>>
>>>
>>> That's my goal too. I started by doing a RO remap and got couple
>>> problems with hibernation. I can try again for the next iteration or
>>> delay it for another patch. I also need to look at KVM GDT usage, I am
>>> not familiar with it yet.
>>
>>
>> don't we write to the GDT as part of the TLS segment stuff for glibc ?
>>
>
> Not sure which glibc feature it is.
>
> In this design, you can write to the GDT per-cpu variable that will
> remain read-write. You just need to make the remapping writeable when
> we load task registers (ltr) then the processor use the current GDT
> address. At least that the case I know, I might find more through
> testing.

Hmm.  I bet that if we preset the accessed bits in all the segments
then we don't need it to be writable in general.  But your point about
set_thread_area (TLS) is well taken.  However, I strongly suspect that
we could make set_thread_area unconditionally set the accessed bit and
no one would ever notice.

[toc] | [prev] | [next] | [standalone]

#1552333

On Thu, Jan 5, 2017 at 12:18 PM, Andy Lutomirski <luto@kernel.org> wrote:
> On Thu, Jan 5, 2017 at 11:03 AM, Thomas Garnier <thgarnie@google.com> wrote:
>> On Thu, Jan 5, 2017 at 10:58 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
>>> On 1/5/2017 9:54 AM, Thomas Garnier wrote:
>>>
>>>>
>>>> That's my goal too. I started by doing a RO remap and got couple
>>>> problems with hibernation. I can try again for the next iteration or
>>>> delay it for another patch. I also need to look at KVM GDT usage, I am
>>>> not familiar with it yet.
>>>
>>>
>>> don't we write to the GDT as part of the TLS segment stuff for glibc ?
>>>
>>
>> Not sure which glibc feature it is.
>>
>> In this design, you can write to the GDT per-cpu variable that will
>> remain read-write. You just need to make the remapping writeable when
>> we load task registers (ltr) then the processor use the current GDT
>> address. At least that the case I know, I might find more through
>> testing.
>
> Hmm.  I bet that if we preset the accessed bits in all the segments
> then we don't need it to be writable in general.  But your point about
> set_thread_area (TLS) is well taken.  However, I strongly suspect that
> we could make set_thread_area unconditionally set the accessed bit and
> no one would ever notice.

Not sure I fully understood and I don't want to miss an important
point. Do you mean making GDT (remapping and per-cpu) read-only and
switch the writeable flag only when we write to the per-cpu entry?

-- 
Thomas

[toc] | [prev] | [next] | [standalone]

#1552342

On Thu, Jan 5, 2017 at 1:08 PM, Thomas Garnier <thgarnie@google.com> wrote:
> On Thu, Jan 5, 2017 at 12:18 PM, Andy Lutomirski <luto@kernel.org> wrote:
>> On Thu, Jan 5, 2017 at 11:03 AM, Thomas Garnier <thgarnie@google.com> wrote:
>>> On Thu, Jan 5, 2017 at 10:58 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
>>>> On 1/5/2017 9:54 AM, Thomas Garnier wrote:
>>>>
>>>>>
>>>>> That's my goal too. I started by doing a RO remap and got couple
>>>>> problems with hibernation. I can try again for the next iteration or
>>>>> delay it for another patch. I also need to look at KVM GDT usage, I am
>>>>> not familiar with it yet.
>>>>
>>>>
>>>> don't we write to the GDT as part of the TLS segment stuff for glibc ?
>>>>
>>>
>>> Not sure which glibc feature it is.
>>>
>>> In this design, you can write to the GDT per-cpu variable that will
>>> remain read-write. You just need to make the remapping writeable when
>>> we load task registers (ltr) then the processor use the current GDT
>>> address. At least that the case I know, I might find more through
>>> testing.
>>
>> Hmm.  I bet that if we preset the accessed bits in all the segments
>> then we don't need it to be writable in general.  But your point about
>> set_thread_area (TLS) is well taken.  However, I strongly suspect that
>> we could make set_thread_area unconditionally set the accessed bit and
>> no one would ever notice.
>
> Not sure I fully understood and I don't want to miss an important
> point. Do you mean making GDT (remapping and per-cpu) read-only and
> switch the writeable flag only when we write to the per-cpu entry?
>

What I mean is: write to the GDT through normal percpu access (or
whatever the normal mapping is) but load a read-only alias into the
GDT register.  As long as nothing ever tries to write through the GDTR
alias, no page faults will be generated.  So we just need to make sure
that nothing ever writes to it through GDTR.  AFAIK the only reason
the CPU ever writes to the address in GDTR is to set an accessed bit.

--Andy

[toc] | [prev] | [next] | [standalone]

#1552356

On Thu, Jan 5, 2017 at 1:19 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Jan 5, 2017 at 1:08 PM, Thomas Garnier <thgarnie@google.com> wrote:
>> On Thu, Jan 5, 2017 at 12:18 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>> On Thu, Jan 5, 2017 at 11:03 AM, Thomas Garnier <thgarnie@google.com> wrote:
>>>> On Thu, Jan 5, 2017 at 10:58 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
>>>>> On 1/5/2017 9:54 AM, Thomas Garnier wrote:
>>>>>
>>>>>>
>>>>>> That's my goal too. I started by doing a RO remap and got couple
>>>>>> problems with hibernation. I can try again for the next iteration or
>>>>>> delay it for another patch. I also need to look at KVM GDT usage, I am
>>>>>> not familiar with it yet.
>>>>>
>>>>>
>>>>> don't we write to the GDT as part of the TLS segment stuff for glibc ?
>>>>>
>>>>
>>>> Not sure which glibc feature it is.
>>>>
>>>> In this design, you can write to the GDT per-cpu variable that will
>>>> remain read-write. You just need to make the remapping writeable when
>>>> we load task registers (ltr) then the processor use the current GDT
>>>> address. At least that the case I know, I might find more through
>>>> testing.
>>>
>>> Hmm.  I bet that if we preset the accessed bits in all the segments
>>> then we don't need it to be writable in general.  But your point about
>>> set_thread_area (TLS) is well taken.  However, I strongly suspect that
>>> we could make set_thread_area unconditionally set the accessed bit and
>>> no one would ever notice.
>>
>> Not sure I fully understood and I don't want to miss an important
>> point. Do you mean making GDT (remapping and per-cpu) read-only and
>> switch the writeable flag only when we write to the per-cpu entry?
>>
>
> What I mean is: write to the GDT through normal percpu access (or
> whatever the normal mapping is) but load a read-only alias into the
> GDT register.  As long as nothing ever tries to write through the GDTR
> alias, no page faults will be generated.  So we just need to make sure
> that nothing ever writes to it through GDTR.  AFAIK the only reason
> the CPU ever writes to the address in GDTR is to set an accessed bit.
>

A write is made when we use load_TR_desc (ltr). I didn't see any other yet.

> --Andy



-- 
Thomas

[toc] | [prev] | [next] | [standalone]

#1552556

* Thomas Garnier <thgarnie@google.com> wrote:

> >> Not sure I fully understood and I don't want to miss an important point. Do 
> >> you mean making GDT (remapping and per-cpu) read-only and switch the 
> >> writeable flag only when we write to the per-cpu entry?
> >
> > What I mean is: write to the GDT through normal percpu access (or whatever the 
> > normal mapping is) but load a read-only alias into the GDT register.  As long 
> > as nothing ever tries to write through the GDTR alias, no page faults will be 
> > generated.  So we just need to make sure that nothing ever writes to it 
> > through GDTR.  AFAIK the only reason the CPU ever writes to the address in 
> > GDTR is to set an accessed bit.
> 
> A write is made when we use load_TR_desc (ltr). I didn't see any other yet.

Is this write to the GDT, generated by the LTR instruction, done unconditionally 
by the hardware?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web