Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1697758 > unrolled thread
| Started by | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| First post | 2017-07-27 09:10 +0200 |
| Last post | 2017-07-27 15:30 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH -tip v4] [BUGFIX] kprobes/x86: Do not jump-optimize kprobes on irq entry code Masami Hiramatsu <mhiramat@kernel.org> - 2017-07-27 09:10 +0200
Re: [PATCH -tip v4] [BUGFIX] kprobes/x86: Do not jump-optimize kprobes on irq entry code Ingo Molnar <mingo@kernel.org> - 2017-07-27 10:30 +0200
Re: [PATCH -tip v4] [BUGFIX] kprobes/x86: Do not jump-optimize kprobes on irq entry code Masami Hiramatsu <mhiramat@kernel.org> - 2017-07-27 15:30 +0200
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2017-07-27 09:10 +0200 |
| Subject | [PATCH -tip v4] [BUGFIX] kprobes/x86: Do not jump-optimize kprobes on irq entry code |
| Message-ID | <u7L5L-344-3@gated-at.bofh.it> |
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 <mhiramat@kernel.org> Reported-by: Francis Deslauriers <francis.deslauriers@efficios.com> Tested-by: Francis Deslauriers <francis.deslauriers@efficios.com> --- 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 <linux/kallsyms.h> #include <linux/ftrace.h> #include <linux/frame.h> +#include <linux/interrupt.h> #include <asm/text-patching.h> #include <asm/cacheflush.h> @@ -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) /* * We want to know which function is an entrypoint of a hardirq or a softirq. */
[toc] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-07-27 10:30 +0200 |
| Subject | Re: [PATCH -tip v4] [BUGFIX] kprobes/x86: Do not jump-optimize kprobes on irq entry code |
| Message-ID | <u7Mlc-3KZ-11@gated-at.bofh.it> |
| In reply to | #1697758 |
* Masami Hiramatsu <mhiramat@kernel.org> 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 <mhiramat@kernel.org> > Reported-by: Francis Deslauriers <francis.deslauriers@efficios.com> > Tested-by: Francis Deslauriers <francis.deslauriers@efficios.com> > --- > 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 <linux/kallsyms.h> > #include <linux/ftrace.h> > #include <linux/frame.h> > +#include <linux/interrupt.h> > > #include <asm/text-patching.h> > #include <asm/cacheflush.h> > @@ -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. Thanks, Ingo
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2017-07-27 15:30 +0200 |
| Subject | Re: [PATCH -tip v4] [BUGFIX] kprobes/x86: Do not jump-optimize kprobes on irq entry code |
| Message-ID | <u7R1x-6CD-19@gated-at.bofh.it> |
| In reply to | #1697799 |
On Thu, 27 Jul 2017 10:24:46 +0200 Ingo Molnar <mingo@kernel.org> wrote: > > * Masami Hiramatsu <mhiramat@kernel.org> 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 <mhiramat@kernel.org> > > Reported-by: Francis Deslauriers <francis.deslauriers@efficios.com> > > Tested-by: Francis Deslauriers <francis.deslauriers@efficios.com> > > --- > > 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 <linux/kallsyms.h> > > #include <linux/ftrace.h> > > #include <linux/frame.h> > > +#include <linux/interrupt.h> > > > > #include <asm/text-patching.h> > > #include <asm/cacheflush.h> > > @@ -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 <mhiramat@kernel.org>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web