Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751111AbbB0FRA (ORCPT ); Fri, 27 Feb 2015 00:17:00 -0500 Received: from szxga01-in.huawei.com ([119.145.14.64]:59445 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750713AbbB0FQ6 (ORCPT ); Fri, 27 Feb 2015 00:16:58 -0500 Message-ID: <54EFFDA2.7060008@huawei.com> Date: Fri, 27 Feb 2015 13:16:18 +0800 From: Wang Nan User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.0.1 MIME-Version: 1.0 To: Andy Lutomirski , Kees Cook CC: Masami Hiramatsu , Steven Rostedt , Ingo Molnar , "H. Peter Anvin" , Thomas Gleixner , X86 ML , Oleg Nesterov , Dave Hansen , "linux-kernel@vger.kernel.org" , Li Zefan Subject: Re: [PATCH] x86, traps: maps all IDTs to fixmap area. References: <1424934412-33238-1-git-send-email-wangnan0@huawei.com> In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.111.69.129] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5858 Lines: 153 On 2015/2/27 2:31, Andy Lutomirski wrote: > On Thu, Feb 26, 2015 at 8:45 AM, Kees Cook wrote: >> On Thu, Feb 26, 2015 at 7:17 AM, Andy Lutomirski wrote: >>> On Wed, Feb 25, 2015 at 11:06 PM, Wang Nan wrote: >>>> The reason why mapping idt_table to fixmap area should also be applied >>>> to debug_idt_table and trace_idt_table. This patch does same thing for >>>> all IDTs. >>>> >>>> Signed-off-by: Wang Nan >>>> --- >>>> >>>> I believe trace_idt_table and debug_idt_table should be symmetrical with >>>> idt_table. However, Like my previous patch 'x86, traps: install gates >>>> using IST after cpu_init()', I'm not sure whether this is a practical >>>> fix. >>> >>> It shouldn't matter, since we should never enter userspace with these >>> IDTs loaded. >>> >>> --Andy >>> >>> [patch kept below for Kees' benefit] >> >> Is there a reason to use fixmap entries for these IDTs? Or rather, is >> there a situation where these IDTs are ever visible to userspace? (The >> reason to use the fixmap is to hide their "true" location from >> userspace.) > > There's also the F00F workaround, which IIRC we get for free by using > the fixmap, but that also shouldn't matter here. > What about a flaw module triggering the F00F bug in kernel space? Instead of kernel panic, the system will hang. I think tis should be a case for which my patch can help. However, the trigger condition is critical. >> >> -Kees >> >>> >>>> >>>> --- >>>> arch/x86/include/asm/fixmap.h | 6 ++++++ >>>> arch/x86/kernel/tracepoint.c | 2 +- >>>> arch/x86/kernel/traps.c | 13 +++++++++++-- >>>> arch/x86/xen/mmu.c | 6 ++++++ >>>> 4 files changed, 24 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h >>>> index f80d700..79550f4 100644 >>>> --- a/arch/x86/include/asm/fixmap.h >>>> +++ b/arch/x86/include/asm/fixmap.h >>>> @@ -90,6 +90,12 @@ enum fixed_addresses { >>>> FIX_IO_APIC_BASE_END = FIX_IO_APIC_BASE_0 + MAX_IO_APICS - 1, >>>> #endif >>>> FIX_RO_IDT, /* Virtual mapping for read-only IDT */ >>>> +#ifdef CONFIG_X86_64 >>>> + FIX_RO_DEBUG_IDT, /* Virtual mapping for read-only debug_idt_table */ >>>> +#endif >>>> +#ifdef CONFIG_TRACING >>>> + FIX_RO_TRACE_IDT, /* Virtual mapping for read-only trace_idt_table */ >>>> +#endif >>>> #ifdef CONFIG_X86_32 >>>> FIX_KMAP_BEGIN, /* reserved pte's for temporary kernel mappings */ >>>> FIX_KMAP_END = FIX_KMAP_BEGIN+(KM_TYPE_NR*NR_CPUS)-1, >>>> diff --git a/arch/x86/kernel/tracepoint.c b/arch/x86/kernel/tracepoint.c >>>> index 1c113db..296e130 100644 >>>> --- a/arch/x86/kernel/tracepoint.c >>>> +++ b/arch/x86/kernel/tracepoint.c >>>> @@ -12,7 +12,7 @@ atomic_t trace_idt_ctr = ATOMIC_INIT(0); >>>> struct desc_ptr trace_idt_descr = { NR_VECTORS * 16 - 1, >>>> (unsigned long) trace_idt_table }; >>>> >>>> -/* No need to be aligned, but done to keep all IDTs defined the same way. */ >>>> +/* Must be page-aligned because the real IDT is used in a fixmap. */ >>>> gate_desc trace_idt_table[NR_VECTORS] __page_aligned_bss; >>>> >>>> static int trace_irq_vector_refcount; >>>> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c >>>> index cf7898e..6d88c37 100644 >>>> --- a/arch/x86/kernel/traps.c >>>> +++ b/arch/x86/kernel/traps.c >>>> @@ -67,7 +67,7 @@ >>>> #include >>>> #include >>>> >>>> -/* No need to be aligned, but done to keep all IDTs defined the same way. */ >>>> +/* Must be page-aligned because the real IDT is used in a fixmap. */ >>>> gate_desc debug_idt_table[NR_VECTORS] __page_aligned_bss; >>>> #else >>>> #include >>>> @@ -998,9 +998,18 @@ void __init trap_init(void) >>>> * Set the IDT descriptor to a fixed read-only location, so that the >>>> * "sidt" instruction will not leak the location of the kernel, and >>>> * to defend the IDT against arbitrary memory write vulnerabilities. >>>> - * It will be reloaded in cpu_init() */ >>>> + * It will be reloaded in cpu_init() >>>> + */ >>>> __set_fixmap(FIX_RO_IDT, __pa_symbol(idt_table), PAGE_KERNEL_RO); >>>> idt_descr.address = fix_to_virt(FIX_RO_IDT); >>>> +#ifdef CONFIG_X86_64 >>>> + __set_fixmap(FIX_RO_DEBUG_IDT, __pa_symbol(debug_idt_table), PAGE_KERNEL_RO); >>>> + debug_idt_descr.address = fix_to_virt(FIX_RO_DEBUG_IDT); >>>> +#endif >>>> +#ifdef CONFIG_TRACING >>>> + __set_fixmap(FIX_RO_TRACE_IDT, __pa_symbol(trace_idt_table), PAGE_KERNEL_RO); >>>> + trace_idt_descr.address = fix_to_virt(FIX_RO_TRACE_IDT); >>>> +#endif >>>> >>>> /* >>>> * Should be a barrier for any external CPU state: >>>> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c >>>> index adca9e2..1fd4a4c 100644 >>>> --- a/arch/x86/xen/mmu.c >>>> +++ b/arch/x86/xen/mmu.c >>>> @@ -1984,6 +1984,12 @@ static void xen_set_fixmap(unsigned idx, phys_addr_t phys, pgprot_t prot) >>>> switch (idx) { >>>> case FIX_BTMAP_END ... FIX_BTMAP_BEGIN: >>>> case FIX_RO_IDT: >>>> +#ifdef CONFIG_X86_64 >>>> + case FIX_RO_DEBUG_IDT: >>>> +#endif >>>> +#ifdef CONFIG_TRACING >>>> + case FIX_RO_TRACE_IDT: >>>> +#endif >>>> #ifdef CONFIG_X86_32 >>>> case FIX_WP_TEST: >>>> # ifdef CONFIG_HIGHMEM >>>> -- >>>> 1.8.4 >>>> >>> >>> >>> >>> -- >>> Andy Lutomirski >>> AMA Capital Management, LLC >> >> >> >> -- >> Kees Cook >> Chrome OS Security > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/