x86: modify_ldt improvement, test, and config option
Subsystems (7)
v6 cover x86: modify_ldt improvement, test, and config option
This is intended for x86/urgent. Sorry for taking so long, but it seemed nice to avoid breaking Xen.
This fixes the "dazed and confused" issue which was exposed by the CVE-2015-5157 fix. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.
Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.
Xen people, can you test patch 1? It works for me on my evil 32-bit Xen virtio setup.
Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.
Changes from v5:
- Improve Xen bits, this time with comments that seem plausible.
- Fix debug_locks warning on exec.
- Add a test for exec preserving the LDT (!)
Changes from v4:
- Fix Xen even better (patch 1 is new).
- Reorder the patches to make a little more sense.
Changes from v3:
- Hopefully fixed Xen.
- Fixed 32-bit test case on 32-bit native kernel.
- Fix bogus vumnap for some LDT sizes.
- Strengthen test case to check all LDT sizes (catches bogus vunmap).
- Lots of cleanups, mostly from Borislav.
- Simplify IPI code using on_each_cpu_mask.
Changes from v2:
- Allocate ldt_struct and the LDT entries separately. This should fix Xen.
- Stop using write_ldt_entry, since I'm pretty sure it's unnecessary now that we no longer mutate an in-use LDT. (Xen people, can you check?)
Changes from v1:
- The config option is new.
- The test case is new.
- Fixed a missing allocation failure check.
- Fixed a use-after-free on fork().
Andy Lutomirski (4):
x86/xen: Probe target addresses in set_aliased_prot before the
hypercall
x86/ldt: Make modify_ldt synchronous
selftests/x86, x86/ldt: Add a selftest for modify_ldt x86/ldt: Make modify_ldt optional
arch/x86/Kconfig | 17 + arch/x86/include/asm/desc.h | 15 - arch/x86/include/asm/mmu.h | 5 +- arch/x86/include/asm/mmu_context.h | 68 +++- arch/x86/kernel/Makefile | 3 +- arch/x86/kernel/cpu/common.c | 4 +- arch/x86/kernel/cpu/perf_event.c | 16 +- arch/x86/kernel/ldt.c | 262 +++++++++------- arch/x86/kernel/process_64.c | 6 +- arch/x86/kernel/step.c | 8 +- arch/x86/power/cpu.c | 3 +- arch/x86/xen/enlighten.c | 40 +++ kernel/sys_ni.c | 1 + tools/testing/selftests/x86/Makefile | 2 +- tools/testing/selftests/x86/ldt_gdt.c | 576 ++++++++++++++++++++++++++++++++++ 15 files changed, 871 insertions(+), 155 deletions(-) create mode 100644 tools/testing/selftests/x86/ldt_gdt.c
Signature · 1 line
2.4.3
PATCH v6 1/4 x86/xen: Probe target addresses in set_aliased_prot before the hypercall
The update_va_mapping hypercall can fail if the VA isn't present in the guest's page tables. Under certain loads, this can result in an OOPS when the target address is in unpopulated vmap space.
While we're at it, add comments to help explain what's going on.
This isn't a great long-term fix. This code should probably be changed to use something like set_memory_ro.
Cc Andrew Cooper <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc David Vrabel <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/xen/enlighten.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
arch/x86/xen/enlighten.c +40 -0
@@ -483,6 +483,7 @@ static void set_aliased_prot(void *v, pgprot_t prot)pte_t pte;unsigned long pfn;struct page *page; +unsigned char dummy; ptep =lookup_address ((unsigned long )v, &level);BUG_ON (ptep ==NULL );
@@ -492,6 +493,32 @@ static void set_aliased_prot(void *v, pgprot_t prot) pte =pfn_pte (pfn, prot); +/* +* Careful: update_va_mapping will fail if the virtual address +* we're poking isn't populated in the page tables. We don't +* need to worry about the direct map (that's always in the page +* tables), but we need to be careful about vmap space. In +* particular, the top level page table can lazily propagate +* entries between processes, so if we've switched mms since we +* vmapped the target in the first place, we might not have the +* top-level page table entry populated. +* +* We disable preemption because we want the same mm active when +* we probe the target and when we issue the hypercall. We'll +* have the same nominal mm, but if we're a kernel thread, lazy +* mm dropping could change our pgd. +* +* Out of an abundance of caution, this uses __get_user to fault +* in the target address just in case there's some obscure case +* in which the target address isn't readable. +*/ + +preempt_disable (); + +pagefault_disable ();/* Avoid warnings due to being atomic. */ +__get_user (dummy, (unsigned char __user __force *)v); +pagefault_enable (); +if (HYPERVISOR_update_va_mapping ((unsigned long )v, pte,0 ))BUG ();
@@ -503,6 +530,8 @@ static void set_aliased_prot(void *v, pgprot_t prot)BUG (); }else kmap_flush_unused (); + +preempt_enable (); }static void xen_alloc_ldt (struct desc_struct *ldt,unsigned entries)
@@ -510,6 +539,17 @@ static void xen_alloc_ldt(struct desc_struct *ldt, unsigned entries)const unsigned entries_per_page =PAGE_SIZE /LDT_ENTRY_SIZE ;int i; +/* +* We need to mark the all aliases of the LDT pages RO. We +* don't need to call vm_flush_aliases(), though, since that's +* only responsible for flushing aliases out the TLBs, not the +* page tables, and Xen will flush the TLB for us if needed. +* +* To avoid confusing future readers: none of this is necessary +* to load the LDT. The hypervisor only checks this when the +* LDT is faulted in due to subsequent descriptor access. +*/ +for (i =0 ; i < entries; i += entries_per_page)set_aliased_prot (ldt + i,PAGE_KERNEL_RO ); }
Signature · 1 line
2.4.3
PATCH v6 1/4 x86/xen: Probe target addresses in set_aliased_prot before the hypercall
In reply to Andy LutomirskiThe update_va_mapping hypercall can fail if the VA isn't present in the guest's page tables. Under certain loads, this can result in an OOPS when the target address is in unpopulated vmap space.
While we're at it, add comments to help explain what's going on.
This isn't a great long-term fix. This code should probably be changed to use something like set_memory_ro.
Cc Andrew Cooper <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc David Vrabel <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/xen/enlighten.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
arch/x86/xen/enlighten.c +40 -0
@@ -483,6 +483,7 @@ static void set_aliased_prot(void *v, pgprot_t prot)pte_t pte;unsigned long pfn;struct page *page; +unsigned char dummy; ptep =lookup_address ((unsigned long )v, &level);BUG_ON (ptep ==NULL );
@@ -492,6 +493,32 @@ static void set_aliased_prot(void *v, pgprot_t prot) pte =pfn_pte (pfn, prot); +/* +* Careful: update_va_mapping will fail if the virtual address +* we're poking isn't populated in the page tables. We don't +* need to worry about the direct map (that's always in the page +* tables), but we need to be careful about vmap space. In +* particular, the top level page table can lazily propagate +* entries between processes, so if we've switched mms since we +* vmapped the target in the first place, we might not have the +* top-level page table entry populated. +* +* We disable preemption because we want the same mm active when +* we probe the target and when we issue the hypercall. We'll +* have the same nominal mm, but if we're a kernel thread, lazy +* mm dropping could change our pgd. +* +* Out of an abundance of caution, this uses __get_user to fault +* in the target address just in case there's some obscure case +* in which the target address isn't readable. +*/ + +preempt_disable (); + +pagefault_disable ();/* Avoid warnings due to being atomic. */ +__get_user (dummy, (unsigned char __user __force *)v); +pagefault_enable (); +if (HYPERVISOR_update_va_mapping ((unsigned long )v, pte,0 ))BUG ();
@@ -503,6 +530,8 @@ static void set_aliased_prot(void *v, pgprot_t prot)BUG (); }else kmap_flush_unused (); + +preempt_enable (); }static void xen_alloc_ldt (struct desc_struct *ldt,unsigned entries)
@@ -510,6 +539,17 @@ static void xen_alloc_ldt(struct desc_struct *ldt, unsigned entries)const unsigned entries_per_page =PAGE_SIZE /LDT_ENTRY_SIZE ;int i; +/* +* We need to mark the all aliases of the LDT pages RO. We +* don't need to call vm_flush_aliases(), though, since that's +* only responsible for flushing aliases out the TLBs, not the +* page tables, and Xen will flush the TLB for us if needed. +* +* To avoid confusing future readers: none of this is necessary +* to load the LDT. The hypervisor only checks this when the +* LDT is faulted in due to subsequent descriptor access. +*/ +for (i =0 ; i < entries; i += entries_per_page)set_aliased_prot (ldt + i,PAGE_KERNEL_RO ); }
Signature · 1 line
2.4.3
Re: [PATCH v6 1/4] x86/xen: Probe target addresses in set_aliased_prot before the hypercall
Quoted 8 lines · Andy Lutomirski
The
update_va_mappinghypercall can fail if the VA isn't present in the guest's page tables. Under certain loads, this can result in an OOPS when the target address is in unpopulated vmap space.While we're at it, add comments to help explain what's going on.
This isn't a great long-term fix. This code should probably be changed to use something like
set_memory_ro.
Reviewed-by David Vrabel <[email protected]>
Thanks.
David
Re: [Xen-devel] [PATCH v6 1/4] x86/xen: Probe target addresses in set_aliased_prot before the hypercall
In reply to Andy LutomirskiQuoted 8 lines · Andy Lutomirski
The
update_va_mappinghypercall can fail if the VA isn't present in the guest's page tables. Under certain loads, this can result in an OOPS when the target address is in unpopulated vmap space.While we're at it, add comments to help explain what's going on.
This isn't a great long-term fix. This code should probably be changed to use something like
set_memory_ro.
Reviewed-by David Vrabel <[email protected]>
Thanks.
David
tip-bot for Andy Lutomirski
merge bot[tip:x86/asm] x86/xen: Probe target addresses in set_aliased_prot () before the hypercall
In reply to Andy Lutomirskix86/xen: Probe target addresses in set_aliased_prot() before the hypercall
Commit-ID: aa1acff356bbedfd03b544051f5b371746735d89 Gitweb: http://git.kernel.org/tip/aa1acff356bbedfd03b544051f5b371746735d89 Author: Andy Lutomirski <[email protected]> AuthorDate: Thu, 30 Jul 2015 14:31:31 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 31 Jul 2015 10:23:22 +0200
x86/xen: Probe target addresses in set_aliased_prot() before the hypercall
The update_va_mapping hypercall can fail if the VA isn't present in the guest's page tables. Under certain loads, this can result in an OOPS when the target address is in unpopulated vmap space.
While we're at it, add comments to help explain what's going on.
This isn't a great long-term fix. This code should probably be changed to use something like set_memory_ro.
Signed-off-by Andy Lutomirski <[email protected]>
Cc Andrew Cooper <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc David Vrabel <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Jan Beulich <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Sasha Levin <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc <[email protected]>
Cc xen-devel <[email protected]>
Link http://lkml.kernel.org/r/0b0e55b995cda11e7829f140b833ef932fcabe3a.1438291540.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/xen/enlighten.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
arch/x86/xen/enlighten.c +40 -0
@@ -483,6 +483,7 @@ static void set_aliased_prot(void *v, pgprot_t prot)pte_t pte;unsigned long pfn;struct page *page; +unsigned char dummy; ptep =lookup_address ((unsigned long )v, &level);BUG_ON (ptep ==NULL );
@@ -492,6 +493,32 @@ static void set_aliased_prot(void *v, pgprot_t prot) pte =pfn_pte (pfn, prot); +/* +* Careful: update_va_mapping() will fail if the virtual address +* we're poking isn't populated in the page tables. We don't +* need to worry about the direct map (that's always in the page +* tables), but we need to be careful about vmap space. In +* particular, the top level page table can lazily propagate +* entries between processes, so if we've switched mms since we +* vmapped the target in the first place, we might not have the +* top-level page table entry populated. +* +* We disable preemption because we want the same mm active when +* we probe the target and when we issue the hypercall. We'll +* have the same nominal mm, but if we're a kernel thread, lazy +* mm dropping could change our pgd. +* +* Out of an abundance of caution, this uses __get_user() to fault +* in the target address just in case there's some obscure case +* in which the target address isn't readable. +*/ + +preempt_disable (); + +pagefault_disable ();/* Avoid warnings due to being atomic. */ +__get_user (dummy, (unsigned char __user __force *)v); +pagefault_enable (); +if (HYPERVISOR_update_va_mapping ((unsigned long )v, pte,0 ))BUG ();
@@ -503,6 +530,8 @@ static void set_aliased_prot(void *v, pgprot_t prot)BUG (); }else kmap_flush_unused (); + +preempt_enable (); }static void xen_alloc_ldt (struct desc_struct *ldt,unsigned entries)
@@ -510,6 +539,17 @@ static void xen_alloc_ldt(struct desc_struct *ldt, unsigned entries)const unsigned entries_per_page =PAGE_SIZE /LDT_ENTRY_SIZE ;int i; +/* +* We need to mark the all aliases of the LDT pages RO. We +* don't need to call vm_flush_aliases(), though, since that's +* only responsible for flushing aliases out the TLBs, not the +* page tables, and Xen will flush the TLB for us if needed. +* +* To avoid confusing future readers: none of this is necessary +* to load the LDT. The hypervisor only checks this when the +* LDT is faulted in due to subsequent descriptor access. +*/ +for (i =0 ; i < entries; i += entries_per_page)set_aliased_prot (ldt + i,PAGE_KERNEL_RO ); }
tip-bot for Andy Lutomirski
merge bot[tip:x86/asm] x86/xen: Probe target addresses in set_aliased_prot () before the hypercall
In reply to Andy Lutomirskix86/xen: Probe target addresses in set_aliased_prot() before the hypercall
Commit-ID: aa1acff356bbedfd03b544051f5b371746735d89 Gitweb: http://git.kernel.org/tip/aa1acff356bbedfd03b544051f5b371746735d89 Author: Andy Lutomirski <[email protected]> AuthorDate: Thu, 30 Jul 2015 14:31:31 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 31 Jul 2015 10:23:22 +0200
x86/xen: Probe target addresses in set_aliased_prot() before the hypercall
The update_va_mapping hypercall can fail if the VA isn't present in the guest's page tables. Under certain loads, this can result in an OOPS when the target address is in unpopulated vmap space.
While we're at it, add comments to help explain what's going on.
This isn't a great long-term fix. This code should probably be changed to use something like set_memory_ro.
Signed-off-by Andy Lutomirski <[email protected]>
Cc Andrew Cooper <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc David Vrabel <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Jan Beulich <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Sasha Levin <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc <[email protected]>
Cc xen-devel <[email protected]>
Link http://lkml.kernel.org/r/0b0e55b995cda11e7829f140b833ef932fcabe3a.1438291540.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/xen/enlighten.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
arch/x86/xen/enlighten.c +40 -0
@@ -483,6 +483,7 @@ static void set_aliased_prot(void *v, pgprot_t prot)pte_t pte;unsigned long pfn;struct page *page; +unsigned char dummy; ptep =lookup_address ((unsigned long )v, &level);BUG_ON (ptep ==NULL );
@@ -492,6 +493,32 @@ static void set_aliased_prot(void *v, pgprot_t prot) pte =pfn_pte (pfn, prot); +/* +* Careful: update_va_mapping() will fail if the virtual address +* we're poking isn't populated in the page tables. We don't +* need to worry about the direct map (that's always in the page +* tables), but we need to be careful about vmap space. In +* particular, the top level page table can lazily propagate +* entries between processes, so if we've switched mms since we +* vmapped the target in the first place, we might not have the +* top-level page table entry populated. +* +* We disable preemption because we want the same mm active when +* we probe the target and when we issue the hypercall. We'll +* have the same nominal mm, but if we're a kernel thread, lazy +* mm dropping could change our pgd. +* +* Out of an abundance of caution, this uses __get_user() to fault +* in the target address just in case there's some obscure case +* in which the target address isn't readable. +*/ + +preempt_disable (); + +pagefault_disable ();/* Avoid warnings due to being atomic. */ +__get_user (dummy, (unsigned char __user __force *)v); +pagefault_enable (); +if (HYPERVISOR_update_va_mapping ((unsigned long )v, pte,0 ))BUG ();
@@ -503,6 +530,8 @@ static void set_aliased_prot(void *v, pgprot_t prot)BUG (); }else kmap_flush_unused (); + +preempt_enable (); }static void xen_alloc_ldt (struct desc_struct *ldt,unsigned entries)
@@ -510,6 +539,17 @@ static void xen_alloc_ldt(struct desc_struct *ldt, unsigned entries)const unsigned entries_per_page =PAGE_SIZE /LDT_ENTRY_SIZE ;int i; +/* +* We need to mark the all aliases of the LDT pages RO. We +* don't need to call vm_flush_aliases(), though, since that's +* only responsible for flushing aliases out the TLBs, not the +* page tables, and Xen will flush the TLB for us if needed. +* +* To avoid confusing future readers: none of this is necessary +* to load the LDT. The hypervisor only checks this when the +* LDT is faulted in due to subsequent descriptor access. +*/ +for (i =0 ; i < entries; i += entries_per_page)set_aliased_prot (ldt + i,PAGE_KERNEL_RO ); }
PATCH v6 2/4 x86/ldt: Make modify_ldt synchronous
In reply to Andy Lutomirskimodify_ldt has questionable locking and does not synchronize threads. Improve it: redesign the locking and synchronize all threads' LDTs using an IPI on all modifications.
This will dramatically slow down modify_ldt in multithreaded programs, but there shouldn't be any multithreaded programs that care about modify_ldt's performance in the first place.
This fixes some fallout from the CVE-2015-5157 fixes.
Note to -stable maintainers: by itself, this patch makes a pre-existing Xen bug much easier to trigger; on a 32-bit Xen guest, the new ldt_gdt selftest is likely to OOPS. Even without this patch, the test can OOPS, but it's much less likely to happen. The Xen maintainers should have a fix for that issue shortly.
Reviewed-by Borislav Petkov <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/include/asm/desc.h | 15 --- arch/x86/include/asm/mmu.h | 3 +- arch/x86/include/asm/mmu_context.h | 54 +++++++- arch/x86/kernel/cpu/common.c | 4 +- arch/x86/kernel/cpu/perf_event.c | 12 +- arch/x86/kernel/ldt.c | 262 ++++++++++++++++++++----------------- arch/x86/kernel/process_64.c | 4 +- arch/x86/kernel/step.c | 6 +- arch/x86/power/cpu.c | 3 +- 9 files changed, 210 insertions(+), 153 deletions(-)
arch/x86/include/asm/desc.h +0 -15
@@ -280,21 +280,6 @@ static inline void clear_LDT(void)set_ldt (NULL ,0 ); } -/* -* load one particular LDT into the current CPU -*/ -static inline void load_LDT_nolock (mm_context_t *pc) -{ -set_ldt (pc->ldt, pc->size); -} - -static inline void load_LDT (mm_context_t *pc) -{ -preempt_disable (); -load_LDT_nolock (pc); -preempt_enable (); -} -static inline unsigned long get_desc_base (const struct desc_struct *desc) {return (unsigned )(desc->base0 | ((desc->base1) <<16 ) | ((desc->base2) <<24 ));
arch/x86/include/asm/mmu.h +1 -2
@@ -9,8 +9,7 @@* we put the segment information here. */ typedef struct { - void *ldt; - int size; + struct ldt_struct *ldt; #ifdef CONFIG_X86_64 /* True if mm supports a task running in 32 bit compatibility mode. */
arch/x86/include/asm/mmu_context.h +49 -5
74 lines not on this page · raw message
arch/x86/kernel/cpu/common.c +2 -2
@@ -1410,7 +1410,7 @@ void cpu_init(void)load_sp0 (t, ¤t->thread);set_tss_desc (cpu, t);load_TR_desc (); -load_LDT (&init_mm.context); +load_mm_ldt (&init_mm);clear_all_debug_regs ();dbg_restore_debug_regs ();
@@ -1459,7 +1459,7 @@ void cpu_init(void)load_sp0 (t, thread);set_tss_desc (cpu, t);load_TR_desc (); -load_LDT (&init_mm.context); +load_mm_ldt (&init_mm); t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap);
arch/x86/kernel/cpu/perf_event.c +8 -4
@@ -2179,21 +2179,25 @@ static unsigned long get_segment_base(unsigned int segment)int idx = segment >>3 ;if ((segment &SEGMENT_TI_MASK ) ==SEGMENT_LDT ) { +struct ldt_struct *ldt; +if (idx >LDT_ENTRIES )return 0 ; -if (idx > current->active_mm->context.size) +/* IRQs are off, so this synchronizes with smp_store_release */ + ldt =lockless_dereference (current->active_mm->context.ldt); +if (!ldt || idx > ldt->size)return 0 ; - desc = current->active_mm->context.ldt; + desc = &ldt->entries[idx]; }else {if (idx >GDT_ENTRIES )return 0 ; - desc =raw_cpu_ptr (gdt_page.gdt); + desc =raw_cpu_ptr (gdt_page.gdt) + idx; } -return get_desc_base (desc + idx); +return get_desc_base (desc); }#ifdef CONFIG_COMPAT
arch/x86/kernel/ldt.c +142 -120
340 lines not on this page · raw message
arch/x86/kernel/process_64.c +2 -2
@@ -121,11 +121,11 @@ void __show_regs(struct pt_regs *regs, int all)void release_thread (struct task_struct *dead_task) {if (dead_task->mm) { -if (dead_task->mm->context.size) { +if (dead_task->mm->context.ldt) {pr_warn ("WARNING: dead process %s still has LDT? <%p/%d>\n" , dead_task->comm, dead_task->mm->context.ldt, - dead_task->mm->context.size); + dead_task->mm->context.ldt->size);BUG (); } }
arch/x86/kernel/step.c +4 -2
@@ -5,6 +5,7 @@#include <linux/mm.h> #include <linux/ptrace.h> #include <asm/desc.h> +#include <asm/mmu_context.h> unsigned long convert_ip_to_linear (struct task_struct *child,struct pt_regs *regs) {
@@ -30,10 +31,11 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re seg &= ~7UL ;mutex_lock (&child->mm->context.lock); -if (unlikely ((seg >>3 ) >= child->mm->context.size)) +if (unlikely (!child->mm->context.ldt || + (seg >>3 ) >= child->mm->context.ldt->size)) addr =-1L ;/* bogus selector, access would fault */ else { - desc = child->mm->context.ldt + seg; + desc = &child->mm->context.ldt->entries[seg]; base =get_desc_base (desc);/* 16-bit code segment? */
arch/x86/power/cpu.c +2 -1
@@ -22,6 +22,7 @@#include <asm/fpu/internal.h> #include <asm/debugreg.h> #include <asm/cpu.h> +#include <asm/mmu_context.h> #ifdef CONFIG_X86_32 __visible unsigned long saved_context_ebx;
@@ -153,7 +154,7 @@ static void fix_processor_context(void)syscall_init ();/* This sets MSR_*STAR and related */ #endif load_TR_desc ();/* This does ltr */ -load_LDT (¤t->active_mm->context);/* This does lldt */ +load_mm_ldt (current->active_mm);/* This does lldt */ fpu__resume_cpu (); }
Signature · 1 line
2.4.3
PATCH v6 2/4 x86/ldt: Make modify_ldt synchronous
In reply to Andy Lutomirskimodify_ldt has questionable locking and does not synchronize threads. Improve it: redesign the locking and synchronize all threads' LDTs using an IPI on all modifications.
This will dramatically slow down modify_ldt in multithreaded programs, but there shouldn't be any multithreaded programs that care about modify_ldt's performance in the first place.
This fixes some fallout from the CVE-2015-5157 fixes.
Note to -stable maintainers: by itself, this patch makes a pre-existing Xen bug much easier to trigger; on a 32-bit Xen guest, the new ldt_gdt selftest is likely to OOPS. Even without this patch, the test can OOPS, but it's much less likely to happen. The Xen maintainers should have a fix for that issue shortly.
Reviewed-by Borislav Petkov <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/include/asm/desc.h | 15 --- arch/x86/include/asm/mmu.h | 3 +- arch/x86/include/asm/mmu_context.h | 54 +++++++- arch/x86/kernel/cpu/common.c | 4 +- arch/x86/kernel/cpu/perf_event.c | 12 +- arch/x86/kernel/ldt.c | 262 ++++++++++++++++++++----------------- arch/x86/kernel/process_64.c | 4 +- arch/x86/kernel/step.c | 6 +- arch/x86/power/cpu.c | 3 +- 9 files changed, 210 insertions(+), 153 deletions(-)
arch/x86/include/asm/desc.h +0 -15
@@ -280,21 +280,6 @@ static inline void clear_LDT(void)set_ldt (NULL ,0 ); } -/* -* load one particular LDT into the current CPU -*/ -static inline void load_LDT_nolock (mm_context_t *pc) -{ -set_ldt (pc->ldt, pc->size); -} - -static inline void load_LDT (mm_context_t *pc) -{ -preempt_disable (); -load_LDT_nolock (pc); -preempt_enable (); -} -static inline unsigned long get_desc_base (const struct desc_struct *desc) {return (unsigned )(desc->base0 | ((desc->base1) <<16 ) | ((desc->base2) <<24 ));
arch/x86/include/asm/mmu.h +1 -2
@@ -9,8 +9,7 @@* we put the segment information here. */ typedef struct { - void *ldt; - int size; + struct ldt_struct *ldt; #ifdef CONFIG_X86_64 /* True if mm supports a task running in 32 bit compatibility mode. */
arch/x86/include/asm/mmu_context.h +49 -5
74 lines not on this page · raw message
arch/x86/kernel/cpu/common.c +2 -2
@@ -1410,7 +1410,7 @@ void cpu_init(void)load_sp0 (t, ¤t->thread);set_tss_desc (cpu, t);load_TR_desc (); -load_LDT (&init_mm.context); +load_mm_ldt (&init_mm);clear_all_debug_regs ();dbg_restore_debug_regs ();
@@ -1459,7 +1459,7 @@ void cpu_init(void)load_sp0 (t, thread);set_tss_desc (cpu, t);load_TR_desc (); -load_LDT (&init_mm.context); +load_mm_ldt (&init_mm); t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap);
arch/x86/kernel/cpu/perf_event.c +8 -4
@@ -2179,21 +2179,25 @@ static unsigned long get_segment_base(unsigned int segment)int idx = segment >>3 ;if ((segment &SEGMENT_TI_MASK ) ==SEGMENT_LDT ) { +struct ldt_struct *ldt; +if (idx >LDT_ENTRIES )return 0 ; -if (idx > current->active_mm->context.size) +/* IRQs are off, so this synchronizes with smp_store_release */ + ldt =lockless_dereference (current->active_mm->context.ldt); +if (!ldt || idx > ldt->size)return 0 ; - desc = current->active_mm->context.ldt; + desc = &ldt->entries[idx]; }else {if (idx >GDT_ENTRIES )return 0 ; - desc =raw_cpu_ptr (gdt_page.gdt); + desc =raw_cpu_ptr (gdt_page.gdt) + idx; } -return get_desc_base (desc + idx); +return get_desc_base (desc); }#ifdef CONFIG_COMPAT
arch/x86/kernel/ldt.c +142 -120
340 lines not on this page · raw message
arch/x86/kernel/process_64.c +2 -2
@@ -121,11 +121,11 @@ void __show_regs(struct pt_regs *regs, int all)void release_thread (struct task_struct *dead_task) {if (dead_task->mm) { -if (dead_task->mm->context.size) { +if (dead_task->mm->context.ldt) {pr_warn ("WARNING: dead process %s still has LDT? <%p/%d>\n" , dead_task->comm, dead_task->mm->context.ldt, - dead_task->mm->context.size); + dead_task->mm->context.ldt->size);BUG (); } }
arch/x86/kernel/step.c +4 -2
@@ -5,6 +5,7 @@#include <linux/mm.h> #include <linux/ptrace.h> #include <asm/desc.h> +#include <asm/mmu_context.h> unsigned long convert_ip_to_linear (struct task_struct *child,struct pt_regs *regs) {
@@ -30,10 +31,11 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re seg &= ~7UL ;mutex_lock (&child->mm->context.lock); -if (unlikely ((seg >>3 ) >= child->mm->context.size)) +if (unlikely (!child->mm->context.ldt || + (seg >>3 ) >= child->mm->context.ldt->size)) addr =-1L ;/* bogus selector, access would fault */ else { - desc = child->mm->context.ldt + seg; + desc = &child->mm->context.ldt->entries[seg]; base =get_desc_base (desc);/* 16-bit code segment? */
arch/x86/power/cpu.c +2 -1
@@ -22,6 +22,7 @@#include <asm/fpu/internal.h> #include <asm/debugreg.h> #include <asm/cpu.h> +#include <asm/mmu_context.h> #ifdef CONFIG_X86_32 __visible unsigned long saved_context_ebx;
@@ -153,7 +154,7 @@ static void fix_processor_context(void)syscall_init ();/* This sets MSR_*STAR and related */ #endif load_TR_desc ();/* This does ltr */ -load_LDT (¤t->active_mm->context);/* This does lldt */ +load_mm_ldt (current->active_mm);/* This does lldt */ fpu__resume_cpu (); }
Signature · 1 line
2.4.3
Re: [PATCH v6 2/4] x86/ldt: Make modify_ldt synchronous
Quoted 5 lines · Andy Lutomirski
Note to -stable maintainers: by itself, this patch makes a pre-existing Xen bug much easier to trigger; on a 32-bit Xen guest, the new
ldt_gdtselftest is likely to OOPS. Even without this patch, the test can OOPS, but it's much less likely to happen. The Xen maintainers should have a fix for that issue shortly.
All Xen issues should be fixed by patch 1 now.
~Andrew
Re: [PATCH v6 2/4] x86/ldt: Make modify_ldt synchronous
In reply to Andy LutomirskiQuoted 5 lines · Andy Lutomirski
Note to -stable maintainers: by itself, this patch makes a pre-existing Xen bug much easier to trigger; on a 32-bit Xen guest, the new
ldt_gdtselftest is likely to OOPS. Even without this patch, the test can OOPS, but it's much less likely to happen. The Xen maintainers should have a fix for that issue shortly.
All Xen issues should be fixed by patch 1 now.
~Andrew
Re: [PATCH v6 2/4] x86/ldt: Make modify_ldt synchronous
Quoted 9 lines · Andrew Cooper
Quoted 5 lines · Andy Lutomirski
Note to -stable maintainers: by itself, this patch makes a pre-existing Xen bug much easier to trigger; on a 32-bit Xen guest, the new
ldt_gdtselftest is likely to OOPS. Even without this patch, the test can OOPS, but it's much less likely to happen. The Xen maintainers should have a fix for that issue shortly.All Xen issues should be fixed by patch 1 now.
Whoops, right. That paragraph in the changelog can be deleted.
Quoted 1 line
~Andrew
Signature · 2 lines
Andy Lutomirski AMA Capital Management, LLC
Re: [PATCH v6 2/4] x86/ldt: Make modify_ldt synchronous
In reply to Andrew CooperQuoted 9 lines · Andrew Cooper
Quoted 5 lines · Andy Lutomirski
Note to -stable maintainers: by itself, this patch makes a pre-existing Xen bug much easier to trigger; on a 32-bit Xen guest, the new
ldt_gdtselftest is likely to OOPS. Even without this patch, the test can OOPS, but it's much less likely to happen. The Xen maintainers should have a fix for that issue shortly.All Xen issues should be fixed by patch 1 now.
Whoops, right. That paragraph in the changelog can be deleted.
Quoted 1 line
~Andrew
Signature · 2 lines
Andy Lutomirski AMA Capital Management, LLC
tip-bot for Andy Lutomirski
merge bot[tip:x86/asm] x86/ldt: Make modify_ldt synchronous
In reply to Andy Lutomirskix86/ldt: Make modify_ldt synchronous
Commit-ID: 37868fe113ff2ba814b3b4eb12df214df555f8dc Gitweb: http://git.kernel.org/tip/37868fe113ff2ba814b3b4eb12df214df555f8dc Author: Andy Lutomirski <[email protected]> AuthorDate: Thu, 30 Jul 2015 14:31:32 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 31 Jul 2015 10:23:23 +0200
x86/ldt: Make modify_ldt synchronous
modify_ldt() has questionable locking and does not synchronize threads. Improve it: redesign the locking and synchronize all threads' LDTs using an IPI on all modifications.
This will dramatically slow down modify_ldt in multithreaded programs, but there shouldn't be any multithreaded programs that care about modify_ldt's performance in the first place.
This fixes some fallout from the CVE-2015-5157 fixes.
Signed-off-by Andy Lutomirski <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Andrew Cooper <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Jan Beulich <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Sasha Levin <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc <[email protected]>
Cc xen-devel <[email protected]>
Link http://lkml.kernel.org/r/4c6978476782160600471bd865b318db34c7b628.1438291540.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/include/asm/desc.h | 15 --- arch/x86/include/asm/mmu.h | 3 +- arch/x86/include/asm/mmu_context.h | 54 +++++++- arch/x86/kernel/cpu/common.c | 4 +- arch/x86/kernel/cpu/perf_event.c | 12 +- arch/x86/kernel/ldt.c | 262 ++++++++++++++++++++----------------- arch/x86/kernel/process_64.c | 4 +- arch/x86/kernel/step.c | 6 +- arch/x86/power/cpu.c | 3 +- 9 files changed, 210 insertions(+), 153 deletions(-)
arch/x86/include/asm/desc.h +0 -15
@@ -280,21 +280,6 @@ static inline void clear_LDT(void)set_ldt (NULL ,0 ); } -/* -* load one particular LDT into the current CPU -*/ -static inline void load_LDT_nolock (mm_context_t *pc) -{ -set_ldt (pc->ldt, pc->size); -} - -static inline void load_LDT (mm_context_t *pc) -{ -preempt_disable (); -load_LDT_nolock (pc); -preempt_enable (); -} -static inline unsigned long get_desc_base (const struct desc_struct *desc) {return (unsigned )(desc->base0 | ((desc->base1) <<16 ) | ((desc->base2) <<24 ));
arch/x86/include/asm/mmu.h +1 -2
@@ -9,8 +9,7 @@* we put the segment information here. */ typedef struct { - void *ldt; - int size; + struct ldt_struct *ldt; #ifdef CONFIG_X86_64 /* True if mm supports a task running in 32 bit compatibility mode. */
arch/x86/include/asm/mmu_context.h +49 -5
74 lines not on this page · raw message
arch/x86/kernel/cpu/common.c +2 -2
@@ -1410,7 +1410,7 @@ void cpu_init(void)load_sp0 (t, ¤t->thread);set_tss_desc (cpu, t);load_TR_desc (); -load_LDT (&init_mm.context); +load_mm_ldt (&init_mm);clear_all_debug_regs ();dbg_restore_debug_regs ();
@@ -1459,7 +1459,7 @@ void cpu_init(void)load_sp0 (t, thread);set_tss_desc (cpu, t);load_TR_desc (); -load_LDT (&init_mm.context); +load_mm_ldt (&init_mm); t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap);
arch/x86/kernel/cpu/perf_event.c +8 -4
@@ -2179,21 +2179,25 @@ static unsigned long get_segment_base(unsigned int segment)int idx = segment >>3 ;if ((segment &SEGMENT_TI_MASK ) ==SEGMENT_LDT ) { +struct ldt_struct *ldt; +if (idx >LDT_ENTRIES )return 0 ; -if (idx > current->active_mm->context.size) +/* IRQs are off, so this synchronizes with smp_store_release */ + ldt =lockless_dereference (current->active_mm->context.ldt); +if (!ldt || idx > ldt->size)return 0 ; - desc = current->active_mm->context.ldt; + desc = &ldt->entries[idx]; }else {if (idx >GDT_ENTRIES )return 0 ; - desc =raw_cpu_ptr (gdt_page.gdt); + desc =raw_cpu_ptr (gdt_page.gdt) + idx; } -return get_desc_base (desc + idx); +return get_desc_base (desc); }#ifdef CONFIG_COMPAT
arch/x86/kernel/ldt.c +142 -120
340 lines not on this page · raw message
arch/x86/kernel/process_64.c +2 -2
@@ -121,11 +121,11 @@ void __show_regs(struct pt_regs *regs, int all)void release_thread (struct task_struct *dead_task) {if (dead_task->mm) { -if (dead_task->mm->context.size) { +if (dead_task->mm->context.ldt) {pr_warn ("WARNING: dead process %s still has LDT? <%p/%d>\n" , dead_task->comm, dead_task->mm->context.ldt, - dead_task->mm->context.size); + dead_task->mm->context.ldt->size);BUG (); } }
arch/x86/kernel/step.c +4 -2
@@ -5,6 +5,7 @@#include <linux/mm.h> #include <linux/ptrace.h> #include <asm/desc.h> +#include <asm/mmu_context.h> unsigned long convert_ip_to_linear (struct task_struct *child,struct pt_regs *regs) {
@@ -30,10 +31,11 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re seg &= ~7UL ;mutex_lock (&child->mm->context.lock); -if (unlikely ((seg >>3 ) >= child->mm->context.size)) +if (unlikely (!child->mm->context.ldt || + (seg >>3 ) >= child->mm->context.ldt->size)) addr =-1L ;/* bogus selector, access would fault */ else { - desc = child->mm->context.ldt + seg; + desc = &child->mm->context.ldt->entries[seg]; base =get_desc_base (desc);/* 16-bit code segment? */
arch/x86/power/cpu.c +2 -1
@@ -22,6 +22,7 @@#include <asm/fpu/internal.h> #include <asm/debugreg.h> #include <asm/cpu.h> +#include <asm/mmu_context.h> #ifdef CONFIG_X86_32 __visible unsigned long saved_context_ebx;
@@ -153,7 +154,7 @@ static void fix_processor_context(void)syscall_init ();/* This sets MSR_*STAR and related */ #endif load_TR_desc ();/* This does ltr */ -load_LDT (¤t->active_mm->context);/* This does lldt */ +load_mm_ldt (current->active_mm);/* This does lldt */ fpu__resume_cpu (); }
tip-bot for Andy Lutomirski
merge bot[tip:x86/asm] x86/ldt: Make modify_ldt synchronous
In reply to Andy Lutomirskix86/ldt: Make modify_ldt synchronous
Commit-ID: 37868fe113ff2ba814b3b4eb12df214df555f8dc Gitweb: http://git.kernel.org/tip/37868fe113ff2ba814b3b4eb12df214df555f8dc Author: Andy Lutomirski <[email protected]> AuthorDate: Thu, 30 Jul 2015 14:31:32 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 31 Jul 2015 10:23:23 +0200
x86/ldt: Make modify_ldt synchronous
modify_ldt() has questionable locking and does not synchronize threads. Improve it: redesign the locking and synchronize all threads' LDTs using an IPI on all modifications.
This will dramatically slow down modify_ldt in multithreaded programs, but there shouldn't be any multithreaded programs that care about modify_ldt's performance in the first place.
This fixes some fallout from the CVE-2015-5157 fixes.
Signed-off-by Andy Lutomirski <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Andrew Cooper <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Jan Beulich <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Sasha Levin <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc <[email protected]>
Cc xen-devel <[email protected]>
Link http://lkml.kernel.org/r/4c6978476782160600471bd865b318db34c7b628.1438291540.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/include/asm/desc.h | 15 --- arch/x86/include/asm/mmu.h | 3 +- arch/x86/include/asm/mmu_context.h | 54 +++++++- arch/x86/kernel/cpu/common.c | 4 +- arch/x86/kernel/cpu/perf_event.c | 12 +- arch/x86/kernel/ldt.c | 262 ++++++++++++++++++++----------------- arch/x86/kernel/process_64.c | 4 +- arch/x86/kernel/step.c | 6 +- arch/x86/power/cpu.c | 3 +- 9 files changed, 210 insertions(+), 153 deletions(-)
arch/x86/include/asm/desc.h +0 -15
@@ -280,21 +280,6 @@ static inline void clear_LDT(void)set_ldt (NULL ,0 ); } -/* -* load one particular LDT into the current CPU -*/ -static inline void load_LDT_nolock (mm_context_t *pc) -{ -set_ldt (pc->ldt, pc->size); -} - -static inline void load_LDT (mm_context_t *pc) -{ -preempt_disable (); -load_LDT_nolock (pc); -preempt_enable (); -} -static inline unsigned long get_desc_base (const struct desc_struct *desc) {return (unsigned )(desc->base0 | ((desc->base1) <<16 ) | ((desc->base2) <<24 ));
arch/x86/include/asm/mmu.h +1 -2
@@ -9,8 +9,7 @@* we put the segment information here. */ typedef struct { - void *ldt; - int size; + struct ldt_struct *ldt; #ifdef CONFIG_X86_64 /* True if mm supports a task running in 32 bit compatibility mode. */
arch/x86/include/asm/mmu_context.h +49 -5
74 lines not on this page · raw message
arch/x86/kernel/cpu/common.c +2 -2
@@ -1410,7 +1410,7 @@ void cpu_init(void)load_sp0 (t, ¤t->thread);set_tss_desc (cpu, t);load_TR_desc (); -load_LDT (&init_mm.context); +load_mm_ldt (&init_mm);clear_all_debug_regs ();dbg_restore_debug_regs ();
@@ -1459,7 +1459,7 @@ void cpu_init(void)load_sp0 (t, thread);set_tss_desc (cpu, t);load_TR_desc (); -load_LDT (&init_mm.context); +load_mm_ldt (&init_mm); t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap);
arch/x86/kernel/cpu/perf_event.c +8 -4
@@ -2179,21 +2179,25 @@ static unsigned long get_segment_base(unsigned int segment)int idx = segment >>3 ;if ((segment &SEGMENT_TI_MASK ) ==SEGMENT_LDT ) { +struct ldt_struct *ldt; +if (idx >LDT_ENTRIES )return 0 ; -if (idx > current->active_mm->context.size) +/* IRQs are off, so this synchronizes with smp_store_release */ + ldt =lockless_dereference (current->active_mm->context.ldt); +if (!ldt || idx > ldt->size)return 0 ; - desc = current->active_mm->context.ldt; + desc = &ldt->entries[idx]; }else {if (idx >GDT_ENTRIES )return 0 ; - desc =raw_cpu_ptr (gdt_page.gdt); + desc =raw_cpu_ptr (gdt_page.gdt) + idx; } -return get_desc_base (desc + idx); +return get_desc_base (desc); }#ifdef CONFIG_COMPAT
arch/x86/kernel/ldt.c +142 -120
340 lines not on this page · raw message
arch/x86/kernel/process_64.c +2 -2
@@ -121,11 +121,11 @@ void __show_regs(struct pt_regs *regs, int all)void release_thread (struct task_struct *dead_task) {if (dead_task->mm) { -if (dead_task->mm->context.size) { +if (dead_task->mm->context.ldt) {pr_warn ("WARNING: dead process %s still has LDT? <%p/%d>\n" , dead_task->comm, dead_task->mm->context.ldt, - dead_task->mm->context.size); + dead_task->mm->context.ldt->size);BUG (); } }
arch/x86/kernel/step.c +4 -2
@@ -5,6 +5,7 @@#include <linux/mm.h> #include <linux/ptrace.h> #include <asm/desc.h> +#include <asm/mmu_context.h> unsigned long convert_ip_to_linear (struct task_struct *child,struct pt_regs *regs) {
@@ -30,10 +31,11 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re seg &= ~7UL ;mutex_lock (&child->mm->context.lock); -if (unlikely ((seg >>3 ) >= child->mm->context.size)) +if (unlikely (!child->mm->context.ldt || + (seg >>3 ) >= child->mm->context.ldt->size)) addr =-1L ;/* bogus selector, access would fault */ else { - desc = child->mm->context.ldt + seg; + desc = &child->mm->context.ldt->entries[seg]; base =get_desc_base (desc);/* 16-bit code segment? */
arch/x86/power/cpu.c +2 -1
@@ -22,6 +22,7 @@#include <asm/fpu/internal.h> #include <asm/debugreg.h> #include <asm/cpu.h> +#include <asm/mmu_context.h> #ifdef CONFIG_X86_32 __visible unsigned long saved_context_ebx;
@@ -153,7 +154,7 @@ static void fix_processor_context(void)syscall_init ();/* This sets MSR_*STAR and related */ #endif load_TR_desc ();/* This does ltr */ -load_LDT (¤t->active_mm->context);/* This does lldt */ +load_mm_ldt (current->active_mm);/* This does lldt */ fpu__resume_cpu (); }
PATCH v6 3/4 selftests/x86, x86/ldt: Add a selftest for modify_ldt
In reply to Andy LutomirskiThis tests general modify_ldt behavior (only writes, so far) as well as synchronous updates via IPI. It fails on old kernels.
I called this ldt_gdt because I'll add set_thread_area tests to it at some point.
Reviewed-by Kees Cook <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
--- tools/testing/selftests/x86/Makefile | 2 +- tools/testing/selftests/x86/ldt_gdt.c | 576 ++++++++++++++++++++++++++++++++++ 2 files changed, 577 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/x86/ldt_gdt.c
tools/testing/selftests/x86/Makefile +1 -1
@@ -4,7 +4,7 @@ include ../lib.mk .PHONY: all all_32 all_64 warn_32bit_failure clean -TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs +TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs ldt_gdt TARGETS_C_32BIT_ONLY := entry_from_vm86 TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
tools/testing/selftests/x86/ldt_gdt.c new file +576 -0
576 lines not on this page · raw message
Signature · 1 line
2.4.3
PATCH v6 3/4 selftests/x86, x86/ldt: Add a selftest for modify_ldt
In reply to Andy LutomirskiThis tests general modify_ldt behavior (only writes, so far) as well as synchronous updates via IPI. It fails on old kernels.
I called this ldt_gdt because I'll add set_thread_area tests to it at some point.
Reviewed-by Kees Cook <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
--- tools/testing/selftests/x86/Makefile | 2 +- tools/testing/selftests/x86/ldt_gdt.c | 576 ++++++++++++++++++++++++++++++++++ 2 files changed, 577 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/x86/ldt_gdt.c
tools/testing/selftests/x86/Makefile +1 -1
@@ -4,7 +4,7 @@ include ../lib.mk .PHONY: all all_32 all_64 warn_32bit_failure clean -TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs +TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs ldt_gdt TARGETS_C_32BIT_ONLY := entry_from_vm86 TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
tools/testing/selftests/x86/ldt_gdt.c new file +576 -0
576 lines not on this page · raw message
Signature · 1 line
2.4.3
tip-bot for Andy Lutomirski
merge bot[tip:x86/asm] selftests/x86, x86/ldt: Add a selftest for modify_ldt()
selftests/x86, x86/ldt: Add a selftest for modify_ldt()
Commit-ID: 014dc90b66c8d0b5f5a9400440727c134ee5e5a3 Gitweb: http://git.kernel.org/tip/014dc90b66c8d0b5f5a9400440727c134ee5e5a3 Author: Andy Lutomirski <[email protected]> AuthorDate: Thu, 30 Jul 2015 14:31:33 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 31 Jul 2015 10:24:37 +0200
selftests/x86, x86/ldt: Add a selftest for modify_ldt()
This tests general modify_ldt() behavior (only writes, so far) as well as synchronous updates via IPI. It fails on old kernels.
I called this ldt_gdt because I'll add set_thread_area() tests to it at some point.
Signed-off-by Andy Lutomirski <[email protected]>
Reviewed-by Kees Cook <[email protected]>
Cc Andrew Cooper <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Jan Beulich <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Sasha Levin <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc xen-devel <[email protected]>
Link http://lkml.kernel.org/r/dcfda65dad07ff5a3ea97a9172b5963bf8031b2e.1438291540.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- tools/testing/selftests/x86/Makefile | 2 +- tools/testing/selftests/x86/ldt_gdt.c | 576 ++++++++++++++++++++++++++++++++++ 2 files changed, 577 insertions(+), 1 deletion(-)
tools/testing/selftests/x86/Makefile +1 -1
@@ -4,7 +4,7 @@ include ../lib.mk .PHONY: all all_32 all_64 warn_32bit_failure clean -TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs +TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs ldt_gdt TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
tools/testing/selftests/x86/ldt_gdt.c new file +576 -0
576 lines not on this page · raw message
tip-bot for Andy Lutomirski
merge bot[tip:x86/asm] selftests/x86, x86/ldt: Add a selftest for modify_ldt()
In reply to Andy Lutomirskiselftests/x86, x86/ldt: Add a selftest for modify_ldt()
Commit-ID: 014dc90b66c8d0b5f5a9400440727c134ee5e5a3 Gitweb: http://git.kernel.org/tip/014dc90b66c8d0b5f5a9400440727c134ee5e5a3 Author: Andy Lutomirski <[email protected]> AuthorDate: Thu, 30 Jul 2015 14:31:33 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 31 Jul 2015 10:24:37 +0200
selftests/x86, x86/ldt: Add a selftest for modify_ldt()
This tests general modify_ldt() behavior (only writes, so far) as well as synchronous updates via IPI. It fails on old kernels.
I called this ldt_gdt because I'll add set_thread_area() tests to it at some point.
Signed-off-by Andy Lutomirski <[email protected]>
Reviewed-by Kees Cook <[email protected]>
Cc Andrew Cooper <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Jan Beulich <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Sasha Levin <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc xen-devel <[email protected]>
Link http://lkml.kernel.org/r/dcfda65dad07ff5a3ea97a9172b5963bf8031b2e.1438291540.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- tools/testing/selftests/x86/Makefile | 2 +- tools/testing/selftests/x86/ldt_gdt.c | 576 ++++++++++++++++++++++++++++++++++ 2 files changed, 577 insertions(+), 1 deletion(-)
tools/testing/selftests/x86/Makefile +1 -1
@@ -4,7 +4,7 @@ include ../lib.mk .PHONY: all all_32 all_64 warn_32bit_failure clean -TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs +TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs ldt_gdt TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
tools/testing/selftests/x86/ldt_gdt.c new file +576 -0
576 lines not on this page · raw message
PATCH v6 4/4 x86/ldt: Make modify_ldt optional
In reply to Andy LutomirskiThe modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.
Reviewed-by Kees Cook <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/Kconfig | 17 +++++++++++++++++ arch/x86/include/asm/mmu.h | 2 ++ arch/x86/include/asm/mmu_context.h | 28 +++++++++++++++++++++------- arch/x86/kernel/Makefile | 3 ++- arch/x86/kernel/cpu/perf_event.c | 4 ++++ arch/x86/kernel/process_64.c | 2 ++ arch/x86/kernel/step.c | 2 ++ kernel/sys_ni.c | 1 + 8 files changed, 51 insertions(+), 8 deletions(-)
arch/x86/Kconfig +17 -0
@@ -1015,6 +1015,7 @@ config VM86 config X86_16BIT bool "Enable support for 16-bit segments" if EXPERT default y + depends on MODIFY_LDT_SYSCALL ---help--- This option is required by programs like Wine to run 16-bit protected mode legacy code on x86 processors. Disabling
@@ -2053,6 +2054,22 @@ config CMDLINE_OVERRIDE This is used to work around broken boot loaders. This should be set to 'N' under normal conditions. +config MODIFY_LDT_SYSCALL + bool "Enable the LDT (local descriptor table)" if EXPERT + default y + ---help--- + Linux can allow user programs to install a per-process x86 + Local Descriptor Table (LDT) using the modify_ldt(2) system + call. This is required to run 16-bit or segmented code such as + DOSEMU or some Wine programs. It is also used by some very old + threading libraries. + + Enabling this feature adds a small amount of overhead to + context switches and increases the low-level kernel attack + surface. Disabling it removes the modify_ldt(2) system call. + + Saying 'N' here may make sense for embedded or server kernels. + source "kernel/livepatch/Kconfig" endmenu
arch/x86/include/asm/mmu.h +2 -0
@@ -9,7 +9,9 @@* we put the segment information here. */ typedef struct { +#ifdef CONFIG_MODIFY_LDT_SYSCALL struct ldt_struct *ldt; +#endif #ifdef CONFIG_X86_64 /* True if mm supports a task running in 32 bit compatibility mode. */
arch/x86/include/asm/mmu_context.h +21 -7
@@ -33,6 +33,7 @@ static inline void load_mm_cr4(struct mm_struct *mm)static inline void load_mm_cr4 (struct mm_struct *mm) {}#endif +#ifdef CONFIG_MODIFY_LDT_SYSCALL /* * ldt_structs can be allocated, used, and freed, but they are never * modified while live.
@@ -48,8 +49,23 @@ struct ldt_struct { int size; }; +/* +* Used for LDT copy/destruction. +*/ +int init_new_context (struct task_struct *tsk,struct mm_struct *mm); +void destroy_context (struct mm_struct *mm); +#else /* CONFIG_MODIFY_LDT_SYSCALL */ +static inline int init_new_context (struct task_struct *tsk, +struct mm_struct *mm) +{ +return 0 ; +} +static inline void destroy_context (struct mm_struct *mm) {} +#endif +static inline void load_mm_ldt (struct mm_struct *mm) { +#ifdef CONFIG_MODIFY_LDT_SYSCALL struct ldt_struct *ldt;/* lockless_dereference synchronizes with smp_store_release */
@@ -73,17 +89,13 @@ static inline void load_mm_ldt(struct mm_struct *mm)set_ldt (ldt->entries, ldt->size); elseclear_LDT (); +#else +clear_LDT (); +#endif DEBUG_LOCKS_WARN_ON (preemptible ()); } -/* -* Used for LDT copy/destruction. -*/ -int init_new_context (struct task_struct *tsk,struct mm_struct *mm); -void destroy_context (struct mm_struct *mm); - -static inline void enter_lazy_tlb (struct mm_struct *mm,struct task_struct *tsk) {#ifdef CONFIG_SMP
@@ -114,6 +126,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,/* Load per-mm CR4 state */ load_mm_cr4 (next); +#ifdef CONFIG_MODIFY_LDT_SYSCALL /* * Load the LDT, if the LDT is different. *
@@ -128,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,*/ if (unlikely (prev->context.ldt != next->context.ldt))load_mm_ldt (next); +#endif }#ifdef CONFIG_SMP else {
arch/x86/kernel/Makefile +2 -1
@@ -24,7 +24,8 @@ CFLAGS_irq.o := -I$(src)/../include/asm/trace obj-y := process_$(BITS).o signal.o obj-y += traps.o irq.o irq_$(BITS).o dumpstack_$(BITS).o -obj-y += time.o ioport.o ldt.o dumpstack.o nmi.o +obj-y += time.o ioport.o dumpstack.o nmi.o +obj-$(CONFIG_MODIFY_LDT_SYSCALL) += ldt.o obj-y += setup.o x86_init.o i8259.o irqinit.o jump_label.o obj-$(CONFIG_IRQ_WORK) += irq_work.o obj-y += probe_roms.o
arch/x86/kernel/cpu/perf_event.c +4 -0
@@ -2179,6 +2179,7 @@ static unsigned long get_segment_base(unsigned int segment)int idx = segment >>3 ;if ((segment &SEGMENT_TI_MASK ) ==SEGMENT_LDT ) { +#ifdef CONFIG_MODIFY_LDT_SYSCALL struct ldt_struct *ldt;if (idx >LDT_ENTRIES )
@@ -2190,6 +2191,9 @@ static unsigned long get_segment_base(unsigned int segment)return 0 ; desc = &ldt->entries[idx]; +#else +return 0 ; +#endif }else {if (idx >GDT_ENTRIES )return 0 ;
arch/x86/kernel/process_64.c +2 -0
@@ -121,6 +121,7 @@ void __show_regs(struct pt_regs *regs, int all) void release_thread(struct task_struct *dead_task) { if (dead_task->mm) { +#ifdef CONFIG_MODIFY_LDT_SYSCALL if (dead_task->mm->context.ldt) { pr_warn("WARNING: dead process %s still has LDT? <%p/%d>\n", dead_task->comm,
@@ -128,6 +129,7 @@ void release_thread(struct task_struct *dead_task) dead_task->mm->context.ldt->size); BUG(); } +#endif } }
arch/x86/kernel/step.c +2 -0
@@ -18,6 +18,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *rereturn addr; } +#ifdef CONFIG_MODIFY_LDT_SYSCALL /* * We'll assume that the code segments in the GDT * are all zero-based. That is largely true: the
@@ -45,6 +46,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re }mutex_unlock (&child->mm->context.lock); } +#endif return addr; }
kernel/sys_ni.c +1 -0
@@ -140,6 +140,7 @@ cond_syscall(sys_sgetmask);cond_syscall (sys_ssetmask);cond_syscall (sys_vm86old);cond_syscall (sys_vm86); +cond_syscall (sys_modify_ldt);cond_syscall (sys_ipc);cond_syscall (compat_sys_ipc);cond_syscall (compat_sys_sysctl);
Signature · 1 line
2.4.3
PATCH v6 4/4 x86/ldt: Make modify_ldt optional
In reply to Andy LutomirskiThe modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.
Reviewed-by Kees Cook <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/Kconfig | 17 +++++++++++++++++ arch/x86/include/asm/mmu.h | 2 ++ arch/x86/include/asm/mmu_context.h | 28 +++++++++++++++++++++------- arch/x86/kernel/Makefile | 3 ++- arch/x86/kernel/cpu/perf_event.c | 4 ++++ arch/x86/kernel/process_64.c | 2 ++ arch/x86/kernel/step.c | 2 ++ kernel/sys_ni.c | 1 + 8 files changed, 51 insertions(+), 8 deletions(-)
arch/x86/Kconfig +17 -0
@@ -1015,6 +1015,7 @@ config VM86 config X86_16BIT bool "Enable support for 16-bit segments" if EXPERT default y + depends on MODIFY_LDT_SYSCALL ---help--- This option is required by programs like Wine to run 16-bit protected mode legacy code on x86 processors. Disabling
@@ -2053,6 +2054,22 @@ config CMDLINE_OVERRIDE This is used to work around broken boot loaders. This should be set to 'N' under normal conditions. +config MODIFY_LDT_SYSCALL + bool "Enable the LDT (local descriptor table)" if EXPERT + default y + ---help--- + Linux can allow user programs to install a per-process x86 + Local Descriptor Table (LDT) using the modify_ldt(2) system + call. This is required to run 16-bit or segmented code such as + DOSEMU or some Wine programs. It is also used by some very old + threading libraries. + + Enabling this feature adds a small amount of overhead to + context switches and increases the low-level kernel attack + surface. Disabling it removes the modify_ldt(2) system call. + + Saying 'N' here may make sense for embedded or server kernels. + source "kernel/livepatch/Kconfig" endmenu
arch/x86/include/asm/mmu.h +2 -0
@@ -9,7 +9,9 @@* we put the segment information here. */ typedef struct { +#ifdef CONFIG_MODIFY_LDT_SYSCALL struct ldt_struct *ldt; +#endif #ifdef CONFIG_X86_64 /* True if mm supports a task running in 32 bit compatibility mode. */
arch/x86/include/asm/mmu_context.h +21 -7
@@ -33,6 +33,7 @@ static inline void load_mm_cr4(struct mm_struct *mm)static inline void load_mm_cr4 (struct mm_struct *mm) {}#endif +#ifdef CONFIG_MODIFY_LDT_SYSCALL /* * ldt_structs can be allocated, used, and freed, but they are never * modified while live.
@@ -48,8 +49,23 @@ struct ldt_struct { int size; }; +/* +* Used for LDT copy/destruction. +*/ +int init_new_context (struct task_struct *tsk,struct mm_struct *mm); +void destroy_context (struct mm_struct *mm); +#else /* CONFIG_MODIFY_LDT_SYSCALL */ +static inline int init_new_context (struct task_struct *tsk, +struct mm_struct *mm) +{ +return 0 ; +} +static inline void destroy_context (struct mm_struct *mm) {} +#endif +static inline void load_mm_ldt (struct mm_struct *mm) { +#ifdef CONFIG_MODIFY_LDT_SYSCALL struct ldt_struct *ldt;/* lockless_dereference synchronizes with smp_store_release */
@@ -73,17 +89,13 @@ static inline void load_mm_ldt(struct mm_struct *mm)set_ldt (ldt->entries, ldt->size); elseclear_LDT (); +#else +clear_LDT (); +#endif DEBUG_LOCKS_WARN_ON (preemptible ()); } -/* -* Used for LDT copy/destruction. -*/ -int init_new_context (struct task_struct *tsk,struct mm_struct *mm); -void destroy_context (struct mm_struct *mm); - -static inline void enter_lazy_tlb (struct mm_struct *mm,struct task_struct *tsk) {#ifdef CONFIG_SMP
@@ -114,6 +126,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,/* Load per-mm CR4 state */ load_mm_cr4 (next); +#ifdef CONFIG_MODIFY_LDT_SYSCALL /* * Load the LDT, if the LDT is different. *
@@ -128,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,*/ if (unlikely (prev->context.ldt != next->context.ldt))load_mm_ldt (next); +#endif }#ifdef CONFIG_SMP else {
arch/x86/kernel/Makefile +2 -1
@@ -24,7 +24,8 @@ CFLAGS_irq.o := -I$(src)/../include/asm/trace obj-y := process_$(BITS).o signal.o obj-y += traps.o irq.o irq_$(BITS).o dumpstack_$(BITS).o -obj-y += time.o ioport.o ldt.o dumpstack.o nmi.o +obj-y += time.o ioport.o dumpstack.o nmi.o +obj-$(CONFIG_MODIFY_LDT_SYSCALL) += ldt.o obj-y += setup.o x86_init.o i8259.o irqinit.o jump_label.o obj-$(CONFIG_IRQ_WORK) += irq_work.o obj-y += probe_roms.o
arch/x86/kernel/cpu/perf_event.c +4 -0
@@ -2179,6 +2179,7 @@ static unsigned long get_segment_base(unsigned int segment)int idx = segment >>3 ;if ((segment &SEGMENT_TI_MASK ) ==SEGMENT_LDT ) { +#ifdef CONFIG_MODIFY_LDT_SYSCALL struct ldt_struct *ldt;if (idx >LDT_ENTRIES )
@@ -2190,6 +2191,9 @@ static unsigned long get_segment_base(unsigned int segment)return 0 ; desc = &ldt->entries[idx]; +#else +return 0 ; +#endif }else {if (idx >GDT_ENTRIES )return 0 ;
arch/x86/kernel/process_64.c +2 -0
@@ -121,6 +121,7 @@ void __show_regs(struct pt_regs *regs, int all) void release_thread(struct task_struct *dead_task) { if (dead_task->mm) { +#ifdef CONFIG_MODIFY_LDT_SYSCALL if (dead_task->mm->context.ldt) { pr_warn("WARNING: dead process %s still has LDT? <%p/%d>\n", dead_task->comm,
@@ -128,6 +129,7 @@ void release_thread(struct task_struct *dead_task) dead_task->mm->context.ldt->size); BUG(); } +#endif } }
arch/x86/kernel/step.c +2 -0
@@ -18,6 +18,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *rereturn addr; } +#ifdef CONFIG_MODIFY_LDT_SYSCALL /* * We'll assume that the code segments in the GDT * are all zero-based. That is largely true: the
@@ -45,6 +46,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re }mutex_unlock (&child->mm->context.lock); } +#endif return addr; }
kernel/sys_ni.c +1 -0
@@ -140,6 +140,7 @@ cond_syscall(sys_sgetmask);cond_syscall (sys_ssetmask);cond_syscall (sys_vm86old);cond_syscall (sys_vm86); +cond_syscall (sys_modify_ldt);cond_syscall (sys_ipc);cond_syscall (compat_sys_ipc);cond_syscall (compat_sys_sysctl);
Signature · 1 line
2.4.3
Re: [PATCH v6 4/4] x86/ldt: Make modify_ldt optional
Quoted 5 lines · Andy Lutomirski
The
modify_ldtsyscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.Reviewed-by Kees Cook <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>
Quoted 10 lines
--- arch/x86/Kconfig | 17 +++++++++++++++++ arch/x86/include/asm/mmu.h | 2 ++ arch/x86/include/asm/mmu_context.h | 28 +++++++++++++++++++++------- arch/x86/kernel/Makefile | 3 ++- arch/x86/kernel/cpu/perf_event.c | 4 ++++ arch/x86/kernel/process_64.c | 2 ++ arch/x86/kernel/step.c | 2 ++ kernel/sys_ni.c | 1 + 8 files changed, 51 insertions(+), 8 deletions(-)
btw., I fixed a (rare) build failure on MATH_EMULATION=y && !MODIFY_LDT_SYSCALL:
arch/x86/math-emu/fpu_system.h:21:71: error: ‘mm_context_t’ has no member named ‘ldt’
I took the easy fix: made MATH_EMULATION depend on MODIFY_LDT_SYSCALL for now.
Thanks,
Ingo
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Re: [PATCH v6 4/4] x86/ldt: Make modify_ldt optional
In reply to Andy LutomirskiQuoted 5 lines · Andy Lutomirski
The
modify_ldtsyscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.Reviewed-by Kees Cook <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>
Quoted 10 lines
--- arch/x86/Kconfig | 17 +++++++++++++++++ arch/x86/include/asm/mmu.h | 2 ++ arch/x86/include/asm/mmu_context.h | 28 +++++++++++++++++++++------- arch/x86/kernel/Makefile | 3 ++- arch/x86/kernel/cpu/perf_event.c | 4 ++++ arch/x86/kernel/process_64.c | 2 ++ arch/x86/kernel/step.c | 2 ++ kernel/sys_ni.c | 1 + 8 files changed, 51 insertions(+), 8 deletions(-)
btw., I fixed a (rare) build failure on MATH_EMULATION=y && !MODIFY_LDT_SYSCALL:
arch/x86/math-emu/fpu_system.h:21:71: error: ‘mm_context_t’ has no member named ‘ldt’
I took the easy fix: made MATH_EMULATION depend on MODIFY_LDT_SYSCALL for now.
Thanks,
Ingo
tip-bot for Andy Lutomirski
merge bot[tip:x86/asm] x86/ldt: Make modify_ldt() optional
In reply to Andy Lutomirskix86/ldt: Make modify_ldt() optional
Commit-ID: a5b9e5a2f14f25a8dae987494d50ad3aac7366b6 Gitweb: http://git.kernel.org/tip/a5b9e5a2f14f25a8dae987494d50ad3aac7366b6 Author: Andy Lutomirski <[email protected]> AuthorDate: Thu, 30 Jul 2015 14:31:34 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 31 Jul 2015 13:30:45 +0200
x86/ldt: Make modify_ldt() optional
The modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.
Signed-off-by Andy Lutomirski <[email protected]>
Reviewed-by Kees Cook <[email protected]>
Cc Andrew Cooper <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Jan Beulich <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Sasha Levin <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc xen-devel <[email protected]>
Link http://lkml.kernel.org/r/a605166a771c343fd64802dece77a903507333bd.1438291540.git.luto@kernel.org open here
[ Made MATH_EMULATION dependent on MODIFY_LDT_SYSCALL. ]
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/Kconfig | 18 ++++++++++++++++++ arch/x86/include/asm/mmu.h | 2 ++ arch/x86/include/asm/mmu_context.h | 28 +++++++++++++++++++++------- arch/x86/kernel/Makefile | 3 ++- arch/x86/kernel/cpu/perf_event.c | 4 ++++ arch/x86/kernel/process_64.c | 2 ++ arch/x86/kernel/step.c | 2 ++ kernel/sys_ni.c | 1 + 8 files changed, 52 insertions(+), 8 deletions(-)
arch/x86/Kconfig +18 -0
@@ -1036,6 +1036,7 @@ config VM86 config X86_16BIT bool "Enable support for 16-bit segments" if EXPERT default y + depends on MODIFY_LDT_SYSCALL ---help--- This option is required by programs like Wine to run 16-bit protected mode legacy code on x86 processors. Disabling
@@ -1530,6 +1531,7 @@ config X86_RESERVE_LOW config MATH_EMULATION bool + depends on MODIFY_LDT_SYSCALL prompt "Math emulation" if X86_32 ---help--- Linux can emulate a math coprocessor (used for floating point
@@ -2074,6 +2076,22 @@ config CMDLINE_OVERRIDE This is used to work around broken boot loaders. This should be set to 'N' under normal conditions. +config MODIFY_LDT_SYSCALL + bool "Enable the LDT (local descriptor table)" if EXPERT + default y + ---help--- + Linux can allow user programs to install a per-process x86 + Local Descriptor Table (LDT) using the modify_ldt(2) system + call. This is required to run 16-bit or segmented code such as + DOSEMU or some Wine programs. It is also used by some very old + threading libraries. + + Enabling this feature adds a small amount of overhead to + context switches and increases the low-level kernel attack + surface. Disabling it removes the modify_ldt(2) system call. + + Saying 'N' here may make sense for embedded or server kernels. + source "kernel/livepatch/Kconfig" endmenu
arch/x86/include/asm/mmu.h +2 -0
@@ -9,7 +9,9 @@* we put the segment information here. */ typedef struct { +#ifdef CONFIG_MODIFY_LDT_SYSCALL struct ldt_struct *ldt; +#endif #ifdef CONFIG_X86_64 /* True if mm supports a task running in 32 bit compatibility mode. */
arch/x86/include/asm/mmu_context.h +21 -7
@@ -33,6 +33,7 @@ static inline void load_mm_cr4(struct mm_struct *mm)static inline void load_mm_cr4 (struct mm_struct *mm) {}#endif +#ifdef CONFIG_MODIFY_LDT_SYSCALL /* * ldt_structs can be allocated, used, and freed, but they are never * modified while live.
@@ -48,8 +49,23 @@ struct ldt_struct { int size; }; +/* +* Used for LDT copy/destruction. +*/ +int init_new_context (struct task_struct *tsk,struct mm_struct *mm); +void destroy_context (struct mm_struct *mm); +#else /* CONFIG_MODIFY_LDT_SYSCALL */ +static inline int init_new_context (struct task_struct *tsk, +struct mm_struct *mm) +{ +return 0 ; +} +static inline void destroy_context (struct mm_struct *mm) {} +#endif +static inline void load_mm_ldt (struct mm_struct *mm) { +#ifdef CONFIG_MODIFY_LDT_SYSCALL struct ldt_struct *ldt;/* lockless_dereference synchronizes with smp_store_release */
@@ -73,17 +89,13 @@ static inline void load_mm_ldt(struct mm_struct *mm)set_ldt (ldt->entries, ldt->size); elseclear_LDT (); +#else +clear_LDT (); +#endif DEBUG_LOCKS_WARN_ON (preemptible ()); } -/* -* Used for LDT copy/destruction. -*/ -int init_new_context (struct task_struct *tsk,struct mm_struct *mm); -void destroy_context (struct mm_struct *mm); - -static inline void enter_lazy_tlb (struct mm_struct *mm,struct task_struct *tsk) {#ifdef CONFIG_SMP
@@ -114,6 +126,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,/* Load per-mm CR4 state */ load_mm_cr4 (next); +#ifdef CONFIG_MODIFY_LDT_SYSCALL /* * Load the LDT, if the LDT is different. *
@@ -128,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,*/ if (unlikely (prev->context.ldt != next->context.ldt))load_mm_ldt (next); +#endif }#ifdef CONFIG_SMP else {
arch/x86/kernel/Makefile +2 -1
@@ -25,7 +25,8 @@ CFLAGS_irq.o := -I$(src)/../include/asm/trace obj-y := process_$(BITS).o signal.o obj-$(CONFIG_COMPAT) += signal_compat.o obj-y += traps.o irq.o irq_$(BITS).o dumpstack_$(BITS).o -obj-y += time.o ioport.o ldt.o dumpstack.o nmi.o +obj-y += time.o ioport.o dumpstack.o nmi.o +obj-$(CONFIG_MODIFY_LDT_SYSCALL) += ldt.o obj-y += setup.o x86_init.o i8259.o irqinit.o jump_label.o obj-$(CONFIG_IRQ_WORK) += irq_work.o obj-y += probe_roms.o
arch/x86/kernel/cpu/perf_event.c +4 -0
@@ -2179,6 +2179,7 @@ static unsigned long get_segment_base(unsigned int segment)int idx = segment >>3 ;if ((segment &SEGMENT_TI_MASK ) ==SEGMENT_LDT ) { +#ifdef CONFIG_MODIFY_LDT_SYSCALL struct ldt_struct *ldt;if (idx >LDT_ENTRIES )
@@ -2190,6 +2191,9 @@ static unsigned long get_segment_base(unsigned int segment)return 0 ; desc = &ldt->entries[idx]; +#else +return 0 ; +#endif }else {if (idx >GDT_ENTRIES )return 0 ;
arch/x86/kernel/process_64.c +2 -0
@@ -121,6 +121,7 @@ void __show_regs(struct pt_regs *regs, int all) void release_thread(struct task_struct *dead_task) { if (dead_task->mm) { +#ifdef CONFIG_MODIFY_LDT_SYSCALL if (dead_task->mm->context.ldt) { pr_warn("WARNING: dead process %s still has LDT? <%p/%d>\n", dead_task->comm,
@@ -128,6 +129,7 @@ void release_thread(struct task_struct *dead_task) dead_task->mm->context.ldt->size); BUG(); } +#endif } }
arch/x86/kernel/step.c +2 -0
@@ -18,6 +18,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *rereturn addr; } +#ifdef CONFIG_MODIFY_LDT_SYSCALL /* * We'll assume that the code segments in the GDT * are all zero-based. That is largely true: the
@@ -45,6 +46,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re }mutex_unlock (&child->mm->context.lock); } +#endif return addr; }
kernel/sys_ni.c +1 -0
@@ -140,6 +140,7 @@ cond_syscall(sys_sgetmask);cond_syscall (sys_ssetmask);cond_syscall (sys_vm86old);cond_syscall (sys_vm86); +cond_syscall (sys_modify_ldt);cond_syscall (sys_ipc);cond_syscall (compat_sys_ipc);cond_syscall (compat_sys_sysctl);
tip-bot for Andy Lutomirski
merge bot[tip:x86/asm] x86/ldt: Make modify_ldt() optional
In reply to Andy Lutomirskix86/ldt: Make modify_ldt() optional
Commit-ID: a5b9e5a2f14f25a8dae987494d50ad3aac7366b6 Gitweb: http://git.kernel.org/tip/a5b9e5a2f14f25a8dae987494d50ad3aac7366b6 Author: Andy Lutomirski <[email protected]> AuthorDate: Thu, 30 Jul 2015 14:31:34 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 31 Jul 2015 13:30:45 +0200
x86/ldt: Make modify_ldt() optional
The modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.
Signed-off-by Andy Lutomirski <[email protected]>
Reviewed-by Kees Cook <[email protected]>
Cc Andrew Cooper <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Jan Beulich <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Sasha Levin <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc xen-devel <[email protected]>
Link http://lkml.kernel.org/r/a605166a771c343fd64802dece77a903507333bd.1438291540.git.luto@kernel.org open here
[ Made MATH_EMULATION dependent on MODIFY_LDT_SYSCALL. ]
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/Kconfig | 18 ++++++++++++++++++ arch/x86/include/asm/mmu.h | 2 ++ arch/x86/include/asm/mmu_context.h | 28 +++++++++++++++++++++------- arch/x86/kernel/Makefile | 3 ++- arch/x86/kernel/cpu/perf_event.c | 4 ++++ arch/x86/kernel/process_64.c | 2 ++ arch/x86/kernel/step.c | 2 ++ kernel/sys_ni.c | 1 + 8 files changed, 52 insertions(+), 8 deletions(-)
arch/x86/Kconfig +18 -0
@@ -1036,6 +1036,7 @@ config VM86 config X86_16BIT bool "Enable support for 16-bit segments" if EXPERT default y + depends on MODIFY_LDT_SYSCALL ---help--- This option is required by programs like Wine to run 16-bit protected mode legacy code on x86 processors. Disabling
@@ -1530,6 +1531,7 @@ config X86_RESERVE_LOW config MATH_EMULATION bool + depends on MODIFY_LDT_SYSCALL prompt "Math emulation" if X86_32 ---help--- Linux can emulate a math coprocessor (used for floating point
@@ -2074,6 +2076,22 @@ config CMDLINE_OVERRIDE This is used to work around broken boot loaders. This should be set to 'N' under normal conditions. +config MODIFY_LDT_SYSCALL + bool "Enable the LDT (local descriptor table)" if EXPERT + default y + ---help--- + Linux can allow user programs to install a per-process x86 + Local Descriptor Table (LDT) using the modify_ldt(2) system + call. This is required to run 16-bit or segmented code such as + DOSEMU or some Wine programs. It is also used by some very old + threading libraries. + + Enabling this feature adds a small amount of overhead to + context switches and increases the low-level kernel attack + surface. Disabling it removes the modify_ldt(2) system call. + + Saying 'N' here may make sense for embedded or server kernels. + source "kernel/livepatch/Kconfig" endmenu
arch/x86/include/asm/mmu.h +2 -0
@@ -9,7 +9,9 @@* we put the segment information here. */ typedef struct { +#ifdef CONFIG_MODIFY_LDT_SYSCALL struct ldt_struct *ldt; +#endif #ifdef CONFIG_X86_64 /* True if mm supports a task running in 32 bit compatibility mode. */
arch/x86/include/asm/mmu_context.h +21 -7
@@ -33,6 +33,7 @@ static inline void load_mm_cr4(struct mm_struct *mm)static inline void load_mm_cr4 (struct mm_struct *mm) {}#endif +#ifdef CONFIG_MODIFY_LDT_SYSCALL /* * ldt_structs can be allocated, used, and freed, but they are never * modified while live.
@@ -48,8 +49,23 @@ struct ldt_struct { int size; }; +/* +* Used for LDT copy/destruction. +*/ +int init_new_context (struct task_struct *tsk,struct mm_struct *mm); +void destroy_context (struct mm_struct *mm); +#else /* CONFIG_MODIFY_LDT_SYSCALL */ +static inline int init_new_context (struct task_struct *tsk, +struct mm_struct *mm) +{ +return 0 ; +} +static inline void destroy_context (struct mm_struct *mm) {} +#endif +static inline void load_mm_ldt (struct mm_struct *mm) { +#ifdef CONFIG_MODIFY_LDT_SYSCALL struct ldt_struct *ldt;/* lockless_dereference synchronizes with smp_store_release */
@@ -73,17 +89,13 @@ static inline void load_mm_ldt(struct mm_struct *mm)set_ldt (ldt->entries, ldt->size); elseclear_LDT (); +#else +clear_LDT (); +#endif DEBUG_LOCKS_WARN_ON (preemptible ()); } -/* -* Used for LDT copy/destruction. -*/ -int init_new_context (struct task_struct *tsk,struct mm_struct *mm); -void destroy_context (struct mm_struct *mm); - -static inline void enter_lazy_tlb (struct mm_struct *mm,struct task_struct *tsk) {#ifdef CONFIG_SMP
@@ -114,6 +126,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,/* Load per-mm CR4 state */ load_mm_cr4 (next); +#ifdef CONFIG_MODIFY_LDT_SYSCALL /* * Load the LDT, if the LDT is different. *
@@ -128,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,*/ if (unlikely (prev->context.ldt != next->context.ldt))load_mm_ldt (next); +#endif }#ifdef CONFIG_SMP else {
arch/x86/kernel/Makefile +2 -1
@@ -25,7 +25,8 @@ CFLAGS_irq.o := -I$(src)/../include/asm/trace obj-y := process_$(BITS).o signal.o obj-$(CONFIG_COMPAT) += signal_compat.o obj-y += traps.o irq.o irq_$(BITS).o dumpstack_$(BITS).o -obj-y += time.o ioport.o ldt.o dumpstack.o nmi.o +obj-y += time.o ioport.o dumpstack.o nmi.o +obj-$(CONFIG_MODIFY_LDT_SYSCALL) += ldt.o obj-y += setup.o x86_init.o i8259.o irqinit.o jump_label.o obj-$(CONFIG_IRQ_WORK) += irq_work.o obj-y += probe_roms.o
arch/x86/kernel/cpu/perf_event.c +4 -0
@@ -2179,6 +2179,7 @@ static unsigned long get_segment_base(unsigned int segment)int idx = segment >>3 ;if ((segment &SEGMENT_TI_MASK ) ==SEGMENT_LDT ) { +#ifdef CONFIG_MODIFY_LDT_SYSCALL struct ldt_struct *ldt;if (idx >LDT_ENTRIES )
@@ -2190,6 +2191,9 @@ static unsigned long get_segment_base(unsigned int segment)return 0 ; desc = &ldt->entries[idx]; +#else +return 0 ; +#endif }else {if (idx >GDT_ENTRIES )return 0 ;
arch/x86/kernel/process_64.c +2 -0
@@ -121,6 +121,7 @@ void __show_regs(struct pt_regs *regs, int all) void release_thread(struct task_struct *dead_task) { if (dead_task->mm) { +#ifdef CONFIG_MODIFY_LDT_SYSCALL if (dead_task->mm->context.ldt) { pr_warn("WARNING: dead process %s still has LDT? <%p/%d>\n", dead_task->comm,
@@ -128,6 +129,7 @@ void release_thread(struct task_struct *dead_task) dead_task->mm->context.ldt->size); BUG(); } +#endif } }
arch/x86/kernel/step.c +2 -0
@@ -18,6 +18,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *rereturn addr; } +#ifdef CONFIG_MODIFY_LDT_SYSCALL /* * We'll assume that the code segments in the GDT * are all zero-based. That is largely true: the
@@ -45,6 +46,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re }mutex_unlock (&child->mm->context.lock); } +#endif return addr; }
kernel/sys_ni.c +1 -0
@@ -140,6 +140,7 @@ cond_syscall(sys_sgetmask);cond_syscall (sys_ssetmask);cond_syscall (sys_vm86old);cond_syscall (sys_vm86); +cond_syscall (sys_modify_ldt);cond_syscall (sys_ipc);cond_syscall (compat_sys_ipc);cond_syscall (compat_sys_sysctl);
Quoted 2 lines · Andy Lutomirski
This is intended for x86/urgent. Sorry for taking so long, but it seemed nice to avoid breaking Xen.
Very much appreciated. Thanks!
Quoted 10 lines
This fixes the "dazed and confused" issue which was exposed by the CVE-2015-5157 fix. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.
Also, servers and embedded systems should probably turn off
modify_ldt. This makes that possible.Xen people, can you test patch 1? It works for me on my evil 32-bit Xen virtio setup.
So the LDT issue seems to have gone away, which is good.
However, I did get this from my single vcpu guest test
[OK] LDT entry 0 is invalid
[SKIP] Cannot set affinity to CPU 1
[RUN] Test exec
[ 3.638967] CPU 0 set the LDT [OK] LDT entry 0 has AR 0x0040FA00 and limit 0x0000002A [ 3.639380] ------------[ cut here ]------------ [ 3.639389] WARNING: CPU: 0 PID: 383 at /local/linux-mainline.git/arch/x86/include/asm/mmu_context.h:96 flush_old_exec+0x7fd/0xb70() [ 3.639397] DEBUG_LOCKS_WARN_ON(!irqs_disabled()) [ 3.639401] CPU: 0 PID: 383 Comm: ldt_gdt_32 Not tainted 4.2.0-rc4+ #21 [ 3.639407] 00000000 00000000 c6fb9e10 c13b63ab 00000000 c6fb9e40 c105f299 c149476c [ 3.639417] c6fb9e6c 0000017f c148d5ac 00000060 c11463dd c11463dd c750aa00 c765e280 [ 3.639426] c765fb80 c6fb9e58 c105f333 00000009 c6fb9e50 c149476c c6fb9e6c c6fb9e9c [ 3.639436] Call Trace: [ 3.639442] [<c13b63ab>] dump_stack+0x48/0x60 [ 3.639447] [<c105f299>] warn_slowpath_common+0x89/0xc0 [ 3.639451] [<c11463dd>] ? flush_old_exec+0x7fd/0xb70 [ 3.639456] [<c11463dd>] ? flush_old_exec+0x7fd/0xb70 [ 3.639460] [<c105f333>] warn_slowpath_fmt+0x33/0x40 [ 3.639464] [<c11463dd>] flush_old_exec+0x7fd/0xb70 [ 3.639470] [<c118fdc4>] load_elf_binary+0x2b4/0x1060 [ 3.639475] [<c10a360e>] ? lock_release+0x27e/0x4e0 [ 3.639480] [<c10a3931>] ? lock_acquire+0xc1/0x240 [ 3.639484] [<c1146b6e>] search_binary_handler+0x4e/0xd0 [ 3.639489] [<c114721c>] do_execveat_common+0x62c/0x910 [ 3.639493] [<c114716d>] ? do_execveat_common+0x57d/0x910 [ 3.639498] [<c1147524>] do_execve+0x24/0x30 [ 3.639502] [<c1147788>] SyS_execve+0x28/0x40 [ 3.639507] [<c13bd497>] syscall_call+0x7/0x7 [ 3.639511] ---[ end trace 95a382309b1f72bd ]--- [OK] LDT entry 0 has AR 0x0040FA00 and limit 0x0000002A [OK] Child succeeded [ 3.639897] CPU 0 cleared the LDT
And this from a two-vcpu test:
[RUN] Cross-CPU LDT invalidation
[ 5.260315] CPU 1 set the LDT [ 5.260324] CPU 0 set the LDT [ 5.268245] CPU 0 cleared the LDT [ 5.268261] ------------[ cut here ]------------ [ 5.268263] CPU 1 cleared the LDT [ 5.268272] WARNING: CPU: 0 PID: 389 at /local/linux-mainline.git/kernel/locking/lockdep.c:2639 trace_hardirqs_off_caller+0xa9/0xb0() [ 5.268280] DEBUG_LOCKS_WARN_ON(!irqs_disabled()) [ 5.268285] CPU: 0 PID: 389 Comm: ldt_gdt_32 Not tainted 4.2.0-rc4+ #21 [ 5.268291] 00000000 00000000 c6863f38 c13b63ab 00000000 c6863f68 c105f299 c149476c [ 5.268301] c6863f94 00000185 c1499b9c 00000a4f c109f8b9 c109f8b9 c13be3a7 00002000 [ 5.268311] c101b470 c6863f80 c105f333 00000009 c6863f78 c149476c c6863f94 c6863f9c [ 5.268320] Call Trace: [ 5.268326] [<c13b63ab>] dump_stack+0x48/0x60 [ 5.268331] [<c105f299>] warn_slowpath_common+0x89/0xc0 [ 5.268336] [<c109f8b9>] ? trace_hardirqs_off_caller+0xa9/0xb0 [ 5.268340] [<c109f8b9>] ? trace_hardirqs_off_caller+0xa9/0xb0 [ 5.268346] [<c13be3a7>] ? error_code+0x5b/0x64 [ 5.268352] [<c101b470>] ? do_device_not_available+0x50/0x50 [ 5.268357] [<c105f333>] warn_slowpath_fmt+0x33/0x40 [ 5.268361] [<c109f8b9>] trace_hardirqs_off_caller+0xa9/0xb0 [ 5.268367] [<c10029ec>] trace_hardirqs_off_thunk+0xc/0x10 [ 5.268371] [<c13be3a7>] ? error_code+0x5b/0x64 [ 5.268376] [<c13b0000>] ? xen_build_mfn_list_list+0x2a0/0x300 [ 5.268381] [<c101b470>] ? do_device_not_available+0x50/0x50 [ 5.268386] ---[ end trace da759e1fe4c971e6 ]--- [ 5.268434] CPU 1 set the LDT [ 5.268441] CPU 0 set the LDT [ 5.268537] CPU 0 cleared the LDT [ 5.268543] CPU 1 cleared the LDT [ 5.268629] CPU 1 set the LDT [ 5.268634] CPU 0 set the LDT [ 5.268726] CPU 0 cleared the LDT [ 5.268732] CPU 1 cleared the LDT [ 5.268818] CPU 1 set the LDT [ 5.268823] CPU 0 set the LDT [ 5.268916] CPU 0 cleared the LDT [ 5.268922] CPU 1 cleared the LDT [ 5.341360] CPU 1 set the LDT [ 5.341369] CPU 0 set the LDT [ 5.341528] CPU 0 cleared the LDT [ 5.341538] CPU 1 cleared the LDT
[OK] All 5 iterations succeeded
I am going to try some 64bit tests now.
~Andrew
Quoted 2 lines · Andy Lutomirski
This is intended for x86/urgent. Sorry for taking so long, but it seemed nice to avoid breaking Xen.
Very much appreciated. Thanks!
Quoted 10 lines
This fixes the "dazed and confused" issue which was exposed by the CVE-2015-5157 fix. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.
Also, servers and embedded systems should probably turn off
modify_ldt. This makes that possible.Xen people, can you test patch 1? It works for me on my evil 32-bit Xen virtio setup.
So the LDT issue seems to have gone away, which is good.
However, I did get this from my single vcpu guest test
[OK] LDT entry 0 is invalid
[SKIP] Cannot set affinity to CPU 1
[RUN] Test exec
[ 3.638967] CPU 0 set the LDT [OK] LDT entry 0 has AR 0x0040FA00 and limit 0x0000002A [ 3.639380] ------------[ cut here ]------------ [ 3.639389] WARNING: CPU: 0 PID: 383 at /local/linux-mainline.git/arch/x86/include/asm/mmu_context.h:96 flush_old_exec+0x7fd/0xb70() [ 3.639397] DEBUG_LOCKS_WARN_ON(!irqs_disabled()) [ 3.639401] CPU: 0 PID: 383 Comm: ldt_gdt_32 Not tainted 4.2.0-rc4+ #21 [ 3.639407] 00000000 00000000 c6fb9e10 c13b63ab 00000000 c6fb9e40 c105f299 c149476c [ 3.639417] c6fb9e6c 0000017f c148d5ac 00000060 c11463dd c11463dd c750aa00 c765e280 [ 3.639426] c765fb80 c6fb9e58 c105f333 00000009 c6fb9e50 c149476c c6fb9e6c c6fb9e9c [ 3.639436] Call Trace: [ 3.639442] [<c13b63ab>] dump_stack+0x48/0x60 [ 3.639447] [<c105f299>] warn_slowpath_common+0x89/0xc0 [ 3.639451] [<c11463dd>] ? flush_old_exec+0x7fd/0xb70 [ 3.639456] [<c11463dd>] ? flush_old_exec+0x7fd/0xb70 [ 3.639460] [<c105f333>] warn_slowpath_fmt+0x33/0x40 [ 3.639464] [<c11463dd>] flush_old_exec+0x7fd/0xb70 [ 3.639470] [<c118fdc4>] load_elf_binary+0x2b4/0x1060 [ 3.639475] [<c10a360e>] ? lock_release+0x27e/0x4e0 [ 3.639480] [<c10a3931>] ? lock_acquire+0xc1/0x240 [ 3.639484] [<c1146b6e>] search_binary_handler+0x4e/0xd0 [ 3.639489] [<c114721c>] do_execveat_common+0x62c/0x910 [ 3.639493] [<c114716d>] ? do_execveat_common+0x57d/0x910 [ 3.639498] [<c1147524>] do_execve+0x24/0x30 [ 3.639502] [<c1147788>] SyS_execve+0x28/0x40 [ 3.639507] [<c13bd497>] syscall_call+0x7/0x7 [ 3.639511] ---[ end trace 95a382309b1f72bd ]--- [OK] LDT entry 0 has AR 0x0040FA00 and limit 0x0000002A [OK] Child succeeded [ 3.639897] CPU 0 cleared the LDT
And this from a two-vcpu test:
[RUN] Cross-CPU LDT invalidation
[ 5.260315] CPU 1 set the LDT [ 5.260324] CPU 0 set the LDT [ 5.268245] CPU 0 cleared the LDT [ 5.268261] ------------[ cut here ]------------ [ 5.268263] CPU 1 cleared the LDT [ 5.268272] WARNING: CPU: 0 PID: 389 at /local/linux-mainline.git/kernel/locking/lockdep.c:2639 trace_hardirqs_off_caller+0xa9/0xb0() [ 5.268280] DEBUG_LOCKS_WARN_ON(!irqs_disabled()) [ 5.268285] CPU: 0 PID: 389 Comm: ldt_gdt_32 Not tainted 4.2.0-rc4+ #21 [ 5.268291] 00000000 00000000 c6863f38 c13b63ab 00000000 c6863f68 c105f299 c149476c [ 5.268301] c6863f94 00000185 c1499b9c 00000a4f c109f8b9 c109f8b9 c13be3a7 00002000 [ 5.268311] c101b470 c6863f80 c105f333 00000009 c6863f78 c149476c c6863f94 c6863f9c [ 5.268320] Call Trace: [ 5.268326] [<c13b63ab>] dump_stack+0x48/0x60 [ 5.268331] [<c105f299>] warn_slowpath_common+0x89/0xc0 [ 5.268336] [<c109f8b9>] ? trace_hardirqs_off_caller+0xa9/0xb0 [ 5.268340] [<c109f8b9>] ? trace_hardirqs_off_caller+0xa9/0xb0 [ 5.268346] [<c13be3a7>] ? error_code+0x5b/0x64 [ 5.268352] [<c101b470>] ? do_device_not_available+0x50/0x50 [ 5.268357] [<c105f333>] warn_slowpath_fmt+0x33/0x40 [ 5.268361] [<c109f8b9>] trace_hardirqs_off_caller+0xa9/0xb0 [ 5.268367] [<c10029ec>] trace_hardirqs_off_thunk+0xc/0x10 [ 5.268371] [<c13be3a7>] ? error_code+0x5b/0x64 [ 5.268376] [<c13b0000>] ? xen_build_mfn_list_list+0x2a0/0x300 [ 5.268381] [<c101b470>] ? do_device_not_available+0x50/0x50 [ 5.268386] ---[ end trace da759e1fe4c971e6 ]--- [ 5.268434] CPU 1 set the LDT [ 5.268441] CPU 0 set the LDT [ 5.268537] CPU 0 cleared the LDT [ 5.268543] CPU 1 cleared the LDT [ 5.268629] CPU 1 set the LDT [ 5.268634] CPU 0 set the LDT [ 5.268726] CPU 0 cleared the LDT [ 5.268732] CPU 1 cleared the LDT [ 5.268818] CPU 1 set the LDT [ 5.268823] CPU 0 set the LDT [ 5.268916] CPU 0 cleared the LDT [ 5.268922] CPU 1 cleared the LDT [ 5.341360] CPU 1 set the LDT [ 5.341369] CPU 0 set the LDT [ 5.341528] CPU 0 cleared the LDT [ 5.341538] CPU 1 cleared the LDT
[OK] All 5 iterations succeeded
I am going to try some 64bit tests now.
~Andrew
Boris Ostrovsky
reviewerQuoted 21 lines · Andrew Cooper
Quoted 2 lines · Andy Lutomirski
This is intended for x86/urgent. Sorry for taking so long, but it seemed nice to avoid breaking Xen.
Very much appreciated. Thanks!
Quoted 9 lines
This fixes the "dazed and confused" issue which was exposed by the CVE-2015-5157 fix. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.
Also, servers and embedded systems should probably turn off
modify_ldt. This makes that possible.Xen people, can you test patch 1? It works for me on my evil 32-bit Xen virtio setup.
So the LDT issue seems to have gone away, which is good.
However, I did get this from my single vcpu guest test
[OK] LDT entry 0 is invalid
[SKIP] Cannot set affinity to CPU 1
[RUN] Test exec
Quoted 7 lines
[ 3.638967] CPU 0 set the LDT [OK] LDT entry 0 has AR 0x0040FA00 and limit 0x0000002A [ 3.639380] ------------[ cut here ]------------ [ 3.639389] WARNING: CPU: 0 PID: 383 at /local/linux-mainline.git/arch/x86/include/asm/mmu_context.h:96 flush_old_exec+0x7fd/0xb70() [ 3.639397] DEBUG_LOCKS_WARN_ON(!irqs_disabled())
You must be running v5 (or earlier). This is fixed in v6 --- it is now 'DEBUG_LOCKS_WARN_ON(preemptible());'
-boris
Boris Ostrovsky
reviewerQuoted 21 lines · Andrew Cooper
Quoted 2 lines · Andy Lutomirski
This is intended for x86/urgent. Sorry for taking so long, but it seemed nice to avoid breaking Xen.
Very much appreciated. Thanks!
Quoted 9 lines
This fixes the "dazed and confused" issue which was exposed by the CVE-2015-5157 fix. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.
Also, servers and embedded systems should probably turn off
modify_ldt. This makes that possible.Xen people, can you test patch 1? It works for me on my evil 32-bit Xen virtio setup.
So the LDT issue seems to have gone away, which is good.
However, I did get this from my single vcpu guest test
[OK] LDT entry 0 is invalid
[SKIP] Cannot set affinity to CPU 1
[RUN] Test exec
Quoted 7 lines
[ 3.638967] CPU 0 set the LDT [OK] LDT entry 0 has AR 0x0040FA00 and limit 0x0000002A [ 3.639380] ------------[ cut here ]------------ [ 3.639389] WARNING: CPU: 0 PID: 383 at /local/linux-mainline.git/arch/x86/include/asm/mmu_context.h:96 flush_old_exec+0x7fd/0xb70() [ 3.639397] DEBUG_LOCKS_WARN_ON(!irqs_disabled())
You must be running v5 (or earlier). This is fixed in v6 --- it is now 'DEBUG_LOCKS_WARN_ON(preemptible());'
-boris
Quoted 30 lines · Boris Ostrovsky
Quoted 21 lines · Andrew Cooper
Quoted 2 lines · Andy Lutomirski
This is intended for x86/urgent. Sorry for taking so long, but it seemed nice to avoid breaking Xen.
Very much appreciated. Thanks!
Quoted 9 lines
This fixes the "dazed and confused" issue which was exposed by the CVE-2015-5157 fix. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.
Also, servers and embedded systems should probably turn off
modify_ldt. This makes that possible.Xen people, can you test patch 1? It works for me on my evil 32-bit Xen virtio setup.
So the LDT issue seems to have gone away, which is good.
However, I did get this from my single vcpu guest test
[OK] LDT entry 0 is invalid
[SKIP] Cannot set affinity to CPU 1
[RUN] Test execQuoted 7 lines
[ 3.638967] CPU 0 set the LDT [OK] LDT entry 0 has AR 0x0040FA00 and limit 0x0000002A [ 3.639380] ------------[ cut here ]------------ [ 3.639389] WARNING: CPU: 0 PID: 383 at /local/linux-mainline.git/arch/x86/include/asm/mmu_context.h:96 flush_old_exec+0x7fd/0xb70() [ 3.639397] DEBUG_LOCKS_WARN_ON(!irqs_disabled())
Quoted 2 lines
You must be running v5 (or earlier). This is fixed in v6 --- it is now 'DEBUG_LOCKS_WARN_ON(preemptible());'
Hmm - I definitely have the correct code, but did a complete clean and rebuild, and the issue went away. I presume I had something stale in the build.
I am still seeing
[ 5.496264] WARNING: CPU: 0 PID: 389 at /local/linux-mainline.git/kernel/locking/lockdep.c:2639 trace_hardirqs_off_caller+0xa9/0xb0() [ 5.496272] DEBUG_LOCKS_WARN_ON(!irqs_disabled()) [ 5.496276] CPU: 0 PID: 389 Comm: ldt_gdt_32 Not tainted 4.2.0-rc4+ #21
But that looks incidental, and unrelated to these fixes.
~Andrew