[PATCH 0/3][RFC] x86: Catch stray non-kprobe breakpoints

[PATCH 0/3][RFC] x86: Catch stray non-kprobe breakpoints

[Abhishek Sagar]

Greetings,

Non kprobe breakpoints in the kernel might lie inside the .kprobes.text section. Such breakpoints can easily be identified by in_kprobes_functions and can be caught early. These are problematic and a warning should be emitted to discourage them (in any rare case, if they actually occur).

For this, a check can route the trap handling of such breakpoints away from kprobe_handler (which ends up calling even more functions marked as __kprobes) from inside kprobe_exceptions_notify.

All comments/suggestions are welcome.

--
Thanks & Regards,
Abhishek Sagar

Re: [PATCH 0/3][RFC] x86: Catch stray non-kprobe breakpoints

[Ananth N Mavinakayanahalli]

On Sun, Jan 27, 2008 at 02:38:56PM +0530, Abhishek Sagar wrote:
> Greetings,
> 
> Non kprobe breakpoints in the kernel might lie inside the .kprobes.text section. Such breakpoints can easily be identified by in_kprobes_functions and can be caught early. These are problematic and a warning should be emitted to discourage them (in any rare case, if they actually occur).

Why? As Masami indicated in an earlier reply, the annotation is to
prevent *only* kprobes.
 
> For this, a check can route the trap handling of such breakpoints away from kprobe_handler (which ends up calling even more functions marked as __kprobes) from inside kprobe_exceptions_notify.

Well.. we pass on control of a !kprobe breakpoint to the kernel. This is
exactly what permits debuggers like xmon to work fine now. I don't see
any harm in such breakpoints being handled autonomously without any sort
of kprobe influence.

Ananth

Re: [PATCH 0/3][RFC] x86: Catch stray non-kprobe breakpoints

[Abhishek Sagar]

On 1/29/08, Ananth N Mavinakayanahalli <ananth@in.ibm.com> wrote:
> > Non kprobe breakpoints in the kernel might lie inside the .kprobes.text section. Such breakpoints can easily be identified by in_kprobes_functions and can be caught early. These are problematic and a warning should be emitted to discourage them (in any rare case, if they actually occur).
>
> Why? As Masami indicated in an earlier reply, the annotation is to
> prevent *only* kprobes.

May be I'm completely off the mark here, but shouldn't a small subset
of this section simply be 'breakpoint-free' rather than 'kprobe-free'?
Placing a breakpoint on kprobe_handler (say) can loop into a recursive
trap without allowing the debugger's notifier chain to be invoked. I'm
assuming that non-kprobe exception notifiers may (or even should) run
after kprobe's notifier callback (kprobe_exceptions_notify).

> > For this, a check can route the trap handling of such breakpoints away from kprobe_handler (which ends up calling even more functions marked as __kprobes) from inside kprobe_exceptions_notify.
>
> Well.. we pass on control of a !kprobe breakpoint to the kernel. This is
> exactly what permits debuggers like xmon to work fine now.

This will still happen. It doesn't stop non-kprobe breakpoints from
being handled, wherever they may be.

> I don't see any harm in such breakpoints being handled autonomously
> without any sort of kprobe influence.

Here's what seems to be happening currently:

int3 (non-kprobe) -> do_int3 ->kprobe_exceptions_notify ->
kprobe_handler (passes the buck to the kernel) -> non-krpobe/debugger
exception handler.

Here's what the patch will do:

int3 (non-kprobe) -> do_int3 ->kprobe_exceptions_notify ->
WARN_ON/kprobe_handler -> non-kprobe/debugger exception handler.

The WARN_ON (and not a BUG_ON) will be hit iff:
(in_kprobes_functions(addr) && !is_jprobe_bkpt(addr))

> Ananth

I hope I've understood the point you were making, or at least came close :-).

--
Thanks,
Abhishek

Re: [PATCH 0/3][RFC] x86: Catch stray non-kprobe breakpoints

[Ananth N Mavinakayanahalli]

On Tue, Jan 29, 2008 at 04:10:58PM +0530, Abhishek Sagar wrote:
> On 1/29/08, Ananth N Mavinakayanahalli <ananth@in.ibm.com> wrote:
> > > Non kprobe breakpoints in the kernel might lie inside the .kprobes.text section. Such breakpoints can easily be identified by in_kprobes_functions and can be caught early. These are problematic and a warning should be emitted to discourage them (in any rare case, if they actually occur).
> >
> > Why? As Masami indicated in an earlier reply, the annotation is to
> > prevent *only* kprobes.
> 
> May be I'm completely off the mark here, but shouldn't a small subset
> of this section simply be 'breakpoint-free' rather than 'kprobe-free'?
> Placing a breakpoint on kprobe_handler (say) can loop into a recursive
> trap without allowing the debugger's notifier chain to be invoked.

A well heeled debugger will necessarily take care of saving contexts
(using techniques like setjmp/longjmp, etc) to help it recover from such
nested cases (See xmon for example).

> I'm assuming that non-kprobe exception notifiers may (or even should) run
> after kprobe's notifier callback (kprobe_exceptions_notify).

Yes, any such notifier is invoked after kprobe's callback as the kprobe
notifier is always registered with the highest priority.

> > > For this, a check can route the trap handling of such breakpoints away from kprobe_handler (which ends up calling even more functions marked as __kprobes) from inside kprobe_exceptions_notify.
> >
> > Well.. we pass on control of a !kprobe breakpoint to the kernel. This is
> > exactly what permits debuggers like xmon to work fine now.
> 
> This will still happen. It doesn't stop non-kprobe breakpoints from
> being handled, wherever they may be.
> 
> > I don't see any harm in such breakpoints being handled autonomously
> > without any sort of kprobe influence.
> 
> Here's what seems to be happening currently:
> 
> int3 (non-kprobe) -> do_int3 ->kprobe_exceptions_notify ->
> kprobe_handler (passes the buck to the kernel) -> non-krpobe/debugger
> exception handler.
> 
> Here's what the patch will do:
> 
> int3 (non-kprobe) -> do_int3 ->kprobe_exceptions_notify ->
> WARN_ON/kprobe_handler -> non-kprobe/debugger exception handler.
> 
> The WARN_ON (and not a BUG_ON) will be hit iff:
> (in_kprobes_functions(addr) && !is_jprobe_bkpt(addr))

But that still is unneeded dmesg clutter, IMHO.

Ananth

Re: [PATCH 0/3][RFC] x86: Catch stray non-kprobe breakpoints

[Masami Hiramatsu]

Abhishek Sagar wrote:
> Placing a breakpoint on kprobe_handler (say) can loop into a recursive
> trap without allowing the debugger's notifier chain to be invoked. I'm
> assuming that non-kprobe exception notifiers may (or even should) run
> after kprobe's notifier callback (kprobe_exceptions_notify).

In that case, why don't you just reduce the priority of kprobe_exceptions_nb?
Then, the execution path becomes very simple.

int3 (non-kprobe) -> do_int3 -> non-krpobe/debugger

I also like to use a debugger for debugging kprobes. that will help us.

Best Regards,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH 0/3][RFC] x86: Catch stray non-kprobe breakpoints

[Abhishek Sagar]

On 1/29/08, Ananth N Mavinakayanahalli <ananth@in.ibm.com> wrote:
> > May be I'm completely off the mark here, but shouldn't a small subset
> > of this section simply be 'breakpoint-free' rather than 'kprobe-free'?
> > Placing a breakpoint on kprobe_handler (say) can loop into a recursive
> > trap without allowing the debugger's notifier chain to be invoked.
>
> A well heeled debugger will necessarily take care of saving contexts
> (using techniques like setjmp/longjmp, etc) to help it recover from such
> nested cases (See xmon for example).

Ok, but the protection/warning is not just for xmon.

> > I'm assuming that non-kprobe exception notifiers may (or even should) run
> > after kprobe's notifier callback (kprobe_exceptions_notify).
>
> Yes, any such notifier is invoked after kprobe's callback as the kprobe
> notifier is always registered with the highest priority.

Ok.

> > The WARN_ON (and not a BUG_ON) will be hit iff:
> > (in_kprobes_functions(addr) && !is_jprobe_bkpt(addr))
>
> But that still is unneeded dmesg clutter, IMHO.

Ok, a warning in my opinion would've been prudent since I think we
cannot guarantee non kprobe breakpoint users (debuggers or anything
else) from the recursive trap handling case.

> Ananth

--
Thanks,
Abhishek

Re: [PATCH 0/3][RFC] x86: Catch stray non-kprobe breakpoints

[Abhishek Sagar]

On 1/29/08, Masami Hiramatsu <mhiramat@redhat.com> wrote:
> In that case, why don't you just reduce the priority of kprobe_exceptions_nb?
> Then, the execution path becomes very simple.

Ananth mentioned that the kprobe notifier has to be the first to run.
It still wouldnt allow us to notice breakpoints on places like do_int3
etc.

> I also like to use a debugger for debugging kprobes. that will help us.

Hmm...It would increase the code-path leading upto kprobe_handler.
That's more territory to be guarded from kprobes.

> Best Regards,
>
> --
> Masami Hiramatsu
>
> Software Engineer
> Hitachi Computer Products (America) Inc.
> Software Solutions Division
>
> e-mail: mhiramat@redhat.com
--
Thanks,
Abhishek

Re: [PATCH 0/3][RFC] x86: Catch stray non-kprobe breakpoints

[Masami Hiramatsu]

Abhishek Sagar wrote:
> On 1/29/08, Masami Hiramatsu <mhiramat@redhat.com> wrote:
>> In that case, why don't you just reduce the priority of kprobe_exceptions_nb?
>> Then, the execution path becomes very simple.
> 
> Ananth mentioned that the kprobe notifier has to be the first to run.

(Hmm.. I think he has just explained current implementation:))
IMHO, since kprobes itself can not know what the external debugger
wants to do, the highest priority should be reserved for those external tools.

> It still wouldnt allow us to notice breakpoints on places like do_int3
> etc.

If you'd like to do that, my recommendation is to modify IDT directly.

>> I also like to use a debugger for debugging kprobes. that will help us.
> 
> Hmm...It would increase the code-path leading upto kprobe_handler.
> That's more territory to be guarded from kprobes.

Sure, all functions of the debugger should be marked __kprobes.
Thus it will be guarded from kprobes.

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH 0/3][RFC] x86: Catch stray non-kprobe breakpoints

[Ananth N Mavinakayanahalli]

On Tue, Jan 29, 2008 at 02:29:41PM -0500, Masami Hiramatsu wrote:
> Abhishek Sagar wrote:
> > On 1/29/08, Masami Hiramatsu <mhiramat@redhat.com> wrote:
> >> In that case, why don't you just reduce the priority of kprobe_exceptions_nb?
> >> Then, the execution path becomes very simple.
> > 
> > Ananth mentioned that the kprobe notifier has to be the first to run.
> 
> (Hmm.. I think he has just explained current implementation:))
> IMHO, since kprobes itself can not know what the external debugger
> wants to do, the highest priority should be reserved for those external tools.

The reason why kprobes needs to be the first to run is simple: it
doesn't need user intervention and if it isn't the intended recepient of
the breakpoint, it just lets the kernel take over (unlike a debugger,
which would potentially need user attention). Also, if the underlying
instruction itself is a breakpoint, we have the facility in kprobes to
single-step inline so the kernel can take control and notify any other
intended recepient of the underlying breakpoint.

As such, I believe the current situation is fine, has worked fine for
close to 4 years now and doesn't warrant any change.

Ananth