lkml.org 
[lkml]   [2019]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH -tip v3 04/10] x86/kprobes: Prohibit probing on IRQ handlers directly
    On Tue, Mar 26, 2019 at 11:50:52PM +0900, Masami Hiramatsu wrote:
    > On Mon, 25 Mar 2019 17:23:34 -0400
    > Steven Rostedt <rostedt@goodmis.org> wrote:
    >
    > > 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).
    >
    > OK, it seems this patch is a bit redundant, because
    > I found that these interrupt handler issue has been fixed
    > by Andrea's commit before merge this patch.
    >
    > commit a50480cb6d61d5c5fc13308479407b628b6bc1c5
    > Author: Andrea Righi <righi.andrea@gmail.com>
    > Date: Thu Dec 6 10:56:48 2018 +0100
    >
    > kprobes/x86: Blacklist non-attachable interrupt functions
    >
    > These interrupt functions are already non-attachable by kprobes.
    > Blacklist them explicitly so that they can show up in
    > /sys/kernel/debug/kprobes/blacklist and tools like BCC can use this
    > additional information.
    >
    > This description is a bit odd (maybe his patch is after mine?) I think
    > while updating this series, the patches were merged out of order.
    > Anyway, with above patch, the core problematic probe points are blacklisted.

    This is the previous thread when I posted my patch (not sure if it helps
    to figure out what happened - maybe it was just an out of order merge
    issue, like you said):

    https://lkml.org/lkml/2018/12/6/212

    >
    > >
    > > 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 ensured that if I revert both of this patch and Andrea's patch,
    > I can reproduce this with probing on apic_timer_interrupt().
    >
    > > 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?
    >
    > Yes, it seems you're right. Andrea's commit specifically fixed the
    > issue and mine is redundant. (I'm not sure why do_IRQ is in
    > __irqentry_text...)

    Not sure if there are specific reasons for that, but do_IRQ is part of
    __irqentry_text because it's explicitly marked with __irq_entry.

    >
    > So, Ingo, please revert this, since this bug already has been fixed by
    > commit a50480cb6d61 ("kprobes: x86_64: blacklist non-attachable interrupt
    > functions")
    >
    > BTW, for further error investigation, I attached my kconfig which is
    > usually I'm testing (some options can be changed) on Qemu.
    > I'm using my mini-container shellscript ( https://github.com/mhiramat/mincs
    > ) which supports qemu-container.
    >
    >
    > Thank you,
    >
    > --
    > Masami Hiramatsu <mhiramat@kernel.org>

    Thanks,
    -Andrea

    \
     
     \ /
      Last update: 2019-03-26 16:18    [W:4.254 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site