lkml.org 
[lkml]   [2008]   [Jan]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] x86: kprobes change kprobe_handler flow
    Hi Abhishek,

    Masami Hiramatsu wrote:
    ...
    >>>> + case KPROBE_HIT_SS:
    >>>> + if (*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
    >>>> + regs->flags &= ~TF_MASK;
    >>>> + regs->flags |= kcb->kprobe_saved_flags;
    >>>> + } else {
    >>>> + /* BUG? */
    >>>> + }
    >>>> + break;
    >>> If my thought is correct, we don't need to use swich-case here,
    >>> Because there are only 2 cases, KPROBE_HIT_SSDONE (x86-64 only)
    >>> or others.
    >>> As a result, this function just setups re-entrance.
    >> As you've also pointed out in your previous reply, this case is
    >> peculiar and therefore I believe it should be marked as a BUG(). I've
    >> left the original case, if (kcb->kprobe_status==KPROBE_HIT_SS) &&
    >> (*p->ainsn.insn == BREAKPOINT_INSTRUCTION), untouched and is handled
    >> as it was before. However, if (kcb->kprobe_status==KPROBE_HIT_SS) &&
    >> !(*p->ainsn.insn == BREAKPOINT_INSTRUCTION), then instead of
    >> incrementing nmissed count like before, it should cry out a BUG. This
    >> is not an ordinary recursive probe handling case which should update
    >> the nmissed count.
    >
    > Hmm, I can not agree, because it is possible to insert a kprobe
    > into kprobe's instruction buffer. If it should be a bug, we must
    > check it when registering the kprobe.

    I discussed it with other maintainers and knew that current kprobes
    does not allow user to insert a kprobe to another kprobe's instruction
    buffer, because register_kprobe ensures the insertion address is text.
    Now I changed my mind. I think that case (p && kprobe_running() &&
    kcb->kprobe_status==KPROBE_HIT_SS) is BUG(), even if (*p->ainsn.insn ==
    BREAKPOINT_INSTRUCTION).

    > (And also, in *p->ainsn.insn == BREAKPOINT_INSTRUCTION case, I doubt
    > that the kernel can handle this "orphaned" breakpoint, because the
    > breakpoint address has been changed.)

    I also changed my mind. In this case, the kernel debugger can retrieve
    correct breakpoint address by using kprobe_running() as below.
    ---
    kp = kprobe_running();
    if (kp)
    addr = kp->addr;
    else
    addr = regs->ip;
    ---

    The last discussion point is that we should restore flags or not if
    (!p && kprobe_running() && kcb->kprobe_status==KPROBE_HIT_SS).
    I think we do not need to do that if the debugger premises that
    kprobes exists.

    Thank you,

    --
    Masami Hiramatsu

    Software Engineer
    Hitachi Computer Products (America) Inc.
    Software Solutions Division

    e-mail: mhiramat@redhat.com, masami.hiramatsu.pt@hitachi.com



    \
     
     \ /
      Last update: 2008-01-03 18:21    [W:0.043 / U:0.040 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site