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: [EMAIL PROTECTED], [EMAIL PROTECTED]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
