Re: [PATCH -tip v3 04/10] x86/kprobes: Prohibit probing on IRQ handlers directly

Steven Rostedt
2019-03-25T21:23:39Z

On Wed, 13 Feb 2019 01:12:44 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Prohibit probing on IRQ handlers in irqentry_text because
> if it interrupts user mode, at that point we haven't changed
> to kernel space yet and which eventually leads a double fault.
> E.g.
>
> # echo p apic_timer_interrupt > kprobe_events

Hmm, this breaks one of my tests (which I probe on do_IRQ).

It's been working for years.

> # echo 1 > events/kprobes/enable
> PANIC: double fault, error_code: 0x0
> CPU: 1 PID: 814 Comm: less Not tainted 4.20.0-rc3+ #30
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
> RIP: 0010:error_entry+0x12/0xf0
> [snip]
> Call Trace:
> <ENTRY_TRAMPOLINE>
> ? native_iret+0x7/0x7
> ? async_page_fault+0x8/0x30
> ? trace_hardirqs_on_thunk+0x1c/0x1c
> ? error_entry+0x7c/0xf0
> ? async_page_fault+0x8/0x30
> ? native_iret+0x7/0x7
> ? int3+0xa/0x20
> ? trace_hardirqs_on_thunk+0x1c/0x1c
> ? error_entry+0x7c/0xf0
> ? int3+0xa/0x20
> ? apic_timer_interrupt+0x1/0x20
> </ENTRY_TRAMPOLINE>
> Kernel panic - not syncing: Machine halted.
> Kernel Offset: disabled

I'm not able to reproduce this (by removing this commit).

I'm thinking something else may have changed, as I've been tracing
interrupt entries for years, and interrupting userspace while doing
this.

I've even added probes where ftrace isn't (where it uses an int3) and
still haven't hit a problem.

I think this patch is swatting a symptom of a bug and not addressing
the bug itself. Can you send me the config that triggers this?

-- Steve

> ---[ end Kernel panic - not syncing: Machine halted. ]---
>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
> arch/x86/kernel/kprobes/core.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index 4ba75afba527..a034cb808e7e 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -1028,6 +1028,13 @@ NOKPROBE_SYMBOL(kprobe_fault_handler);
>
> int __init arch_populate_kprobe_blacklist(void)
> {
> + int ret;
> +
> + ret = kprobe_add_area_blacklist((unsigned long)__irqentry_text_start,
> + (unsigned long)__irqentry_text_end);
> + if (ret)
> + return ret;
> +
> return kprobe_add_area_blacklist((unsigned long)__entry_text_start,
> (unsigned long)__entry_text_end);
> }