Messages in this thread |  | | | Date | Thu, 03 Jan 2008 12:15:54 -0500 | | From | Masami Hiramatsu <> | | Subject | Re: [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
|  |