From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751607AbdG0NUb (ORCPT ); Thu, 27 Jul 2017 09:20:31 -0400 Received: from mail.kernel.org ([198.145.29.99]:43776 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751441AbdG0NUS (ORCPT ); Thu, 27 Jul 2017 09:20:18 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E77B922C9E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=mhiramat@kernel.org Date: Thu, 27 Jul 2017 22:19:55 +0900 From: Masami Hiramatsu To: Ingo Molnar Cc: Francis Deslauriers , mathieu.desnoyers@efficios.com, Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , x86@kernel.org, Ananth N Mavinakayanahalli , Anil S Keshavamurthy , "David S . Miller" , linux-kernel@vger.kernel.org Subject: Re: [PATCH -tip v4] [BUGFIX] kprobes/x86: Do not jump-optimize kprobes on irq entry code Message-Id: <20170727221955.bf0075adcdc6ca222c8d8266@kernel.org> In-Reply-To: <20170727082446.xfyzne6hnbxhgwrx@gmail.com> References: <150113888679.3169.18059401640325215627.stgit@devbox> <20170727082446.xfyzne6hnbxhgwrx@gmail.com> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.31; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 27 Jul 2017 10:24:46 +0200 Ingo Molnar wrote: > > * Masami Hiramatsu wrote: > > > Since the kernel segment registers are not prepared at the > > entry of irq-entry code, if a kprobe on such code is > > jump-optimized, accessing per-cpu variables may cause > > kernel panic. > > However, if the kprobe is not optimized, it kicks int3 > > exception and set segment registers correctly. > > > > This checks probe-address and if it is in irq-entry code, > > it prohibits optimizing such kprobes. This means we can > > continuously probing such interrupt handlers by kprobes > > but it is not optimized anymore. > > > > Signed-off-by: Masami Hiramatsu > > Reported-by: Francis Deslauriers > > Tested-by: Francis Deslauriers > > --- > > Changes in v4: > > - Fix to use CONFIG_OPTPROBES instead of CONFIG_KPROBES. > > --- > > arch/x86/kernel/kprobes/opt.c | 9 ++++++--- > > include/asm-generic/vmlinux.lds.h | 6 ++++-- > > include/linux/interrupt.h | 3 ++- > > 3 files changed, 12 insertions(+), 6 deletions(-) > > > > diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c > > index 69ea0bc..c26e7f9 100644 > > --- a/arch/x86/kernel/kprobes/opt.c > > +++ b/arch/x86/kernel/kprobes/opt.c > > @@ -29,6 +29,7 @@ > > #include > > #include > > #include > > +#include > > > > #include > > #include > > @@ -251,10 +252,12 @@ static int can_optimize(unsigned long paddr) > > > > /* > > * Do not optimize in the entry code due to the unstable > > - * stack handling. > > + * stack handling and registers setup. > > */ > > - if ((paddr >= (unsigned long)__entry_text_start) && > > - (paddr < (unsigned long)__entry_text_end)) > > + if (((paddr >= (unsigned long)__entry_text_start) && > > + (paddr < (unsigned long)__entry_text_end)) || > > + ((paddr >= (unsigned long)__irqentry_text_start) && > > + (paddr < (unsigned long)__irqentry_text_end))) > > return 0; > > > > /* Check there is enough space for a relative jump. */ > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > > index da0be9a..3092a1f 100644 > > --- a/include/asm-generic/vmlinux.lds.h > > +++ b/include/asm-generic/vmlinux.lds.h > > @@ -483,7 +483,8 @@ > > *(.entry.text) \ > > VMLINUX_SYMBOL(__entry_text_end) = .; > > > > -#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN) > > +#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN) || \ > > + defined(CONFIG_OPTPROBES) > > #define IRQENTRY_TEXT \ > > ALIGN_FUNCTION(); \ > > VMLINUX_SYMBOL(__irqentry_text_start) = .; \ > > @@ -493,7 +494,8 @@ > > #define IRQENTRY_TEXT > > #endif > > > > -#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN) > > +#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN) || \ > > + defined(CONFIG_OPTPROBES) > > #define SOFTIRQENTRY_TEXT \ > > ALIGN_FUNCTION(); \ > > VMLINUX_SYMBOL(__softirqentry_text_start) = .; \ > > diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h > > index a2fdddd..f95b046 100644 > > --- a/include/linux/interrupt.h > > +++ b/include/linux/interrupt.h > > @@ -726,7 +726,8 @@ extern int early_irq_init(void); > > extern int arch_probe_nr_irqs(void); > > extern int arch_early_irq_init(void); > > > > -#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN) > > +#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN) || \ > > + defined(CONFIG_OPTPROBES) > > Please split the generic changes to asm-generic/vmlinux.lds.h and interrupt.h into > a separate patch, with its own changelog, because this is not a kprobes/x86 only > patch anymore. OK. BTW, I think it is a good time to make a new (hidden) Kconfig entry for irqentry, like CONFIG_IRQENTRY and above 3 configs select it. What would you think? Thank you, > > Thanks, > > Ingo -- Masami Hiramatsu