[PATCH 3/3] x86: WARN_ON breakpoints from .kprobes.text section

[PATCH 3/3] x86: WARN_ON breakpoints from .kprobes.text section

[Abhishek Sagar]

Identify breakpoints in .kprobes.text section. These certainly aren't kprobe traps. However, we make an exception for the breakpoint hardcoded into jprobe_return.

Signed-off-by: Abhishek Sagar <sagar.abhishek@gmail.com>
---

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index 45f2949..f3d13d0 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -961,6 +961,7 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
 				       unsigned long val, void *data)
 {
 	struct die_args *args = data;
+	unsigned long addr = kprobe_bkpt_addr(args->regs);
 	int ret = NOTIFY_DONE;
 
 	if (args->regs && user_mode_vm(args->regs))
@@ -968,7 +969,14 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
 
 	switch (val) {
 	case DIE_INT3:
-		if (kprobe_handler(args->regs))
+		if (in_kprobes_functions(addr) &&
+		    !is_jprobe_bkpt((u8 *)addr)) {
+			/* A breakpoint has made it's way to the .kprobes.text
+			 * section (excluding jprobe_return). This could be
+			 * due to an external debugger. */
+			WARN_ON(1);
+			
+		} else if (kprobe_handler(args->regs))
 			ret = NOTIFY_STOP;
 		break;
 	case DIE_DEBUG:

Re: [PATCH 3/3] x86: WARN_ON breakpoints from .kprobes.text section

[Masami Hiramatsu]

Hi Abhishek,

Abhishek Sagar wrote:
> Identify breakpoints in .kprobes.text section. These certainly aren't kprobe 
> traps. However, we make an exception for the breakpoint hardcoded into jprobe_return.

Sorry, I can not understand what issue these patches can solve.
The breakpoint which is inserted by external debugger will be checked by
kprobe_handler() and be passed to other exception_notify_handlers.
In that case, we don't need to warn it.
I think current code is enough simple.

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH 3/3] x86: WARN_ON breakpoints from .kprobes.text section

[Abhishek Sagar]

On 1/27/08, Masami Hiramatsu <mhiramat@redhat.com> wrote:
> Sorry, I can not understand what issue these patches can solve.
> The breakpoint which is inserted by external debugger will be checked by
> kprobe_handler() and be passed to other exception_notify_handlers.
> In that case, we don't need to warn it.
> I think current code is enough simple.

kprobe_handler has a blanket check for all non-kprobe breakpoints.
They're all left to the kernel to handle. This is fine. Although
external debuggers are free to plant breakpoints anywhere, they should
be discouraged from doing so inside .kprobes.text region. Placing them
there may lead to recursive page-fault/trap handling. It's a defensive
check. I hope I've been able to clarify.

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

Thanks,
Abhishek Sagar

Re: [PATCH 3/3] x86: WARN_ON breakpoints from .kprobes.text section

[Masami Hiramatsu]

Hi Abhishek,

Abhishek Sagar wrote:
> On 1/27/08, Masami Hiramatsu <mhiramat@redhat.com> wrote:
>> Sorry, I can not understand what issue these patches can solve.
>> The breakpoint which is inserted by external debugger will be checked by
>> kprobe_handler() and be passed to other exception_notify_handlers.
>> In that case, we don't need to warn it.
>> I think current code is enough simple.
> 
> kprobe_handler has a blanket check for all non-kprobe breakpoints.
> They're all left to the kernel to handle. This is fine. Although
> external debuggers are free to plant breakpoints anywhere, they should
> be discouraged from doing so inside .kprobes.text region. Placing them
> there may lead to recursive page-fault/trap handling. It's a defensive
> check. I hope I've been able to clarify.

Thank you for explanation, I hope I can understand it.
Even if it causes a trap recursively, it could be checked (and ignored) by
longjump_break_handler(), and passed to the debugger correctly.
Please consider that someone expands jprobe(jprobe2) which uses
jprobe_return2() instead of jprobe_return(), how would you handle it?
Current kprobes provides an opportunity to those external probe frameworks
for checking it by themselves.

By the way, external kernel debugger knows how kprobes (and exception notifier)
works, so I think it can fetch his exception before kprobes does (by tweaking
notifier chain, etc).
(I hope all external kernel debuggers take care of it. :-))

Thank you again,

> 
> Thanks,
> Abhishek Sagar

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH 3/3] x86: WARN_ON breakpoints from .kprobes.text section

[Abhishek Sagar]

On 1/28/08, Masami Hiramatsu <mhiramat@redhat.com> wrote:
> Thank you for explanation, I hope I can understand it.
> Even if it causes a trap recursively, it could be checked (and ignored) by
> longjump_break_handler(), and passed to the debugger correctly.

Yes, all non-kprobe breakpoints are left to the kernel to be handled.
The objective here is to intercept the trap handling of a certain
category of such breakpoints and emit a warning. The premise being
that .kprobes.text section is a logical breakpoint-free zone.

> Please consider that someone expands jprobe(jprobe2) which uses
> jprobe_return2() instead of jprobe_return(), how would you handle it?

By a simple modification of is_jprobe_bkpt() (defined in patch #2 of
this series).

> Current kprobes provides an opportunity to those external probe frameworks
> for checking it by themselves.

Could you clarirfy this with some example. For now I'm assuming that
by external probe frameworks you mean kernel modules using kprobes. If
they embed breakpoints in their handlers, then they will simply not be
caught by this check because thay cannot lie in the .kprobes.text
section.

> By the way, external kernel debugger knows how kprobe (and exception notifier)
> works, so I think it can fetch his exception before kprobes does (by tweaking
> notifier chain, etc).
> (I hope all external kernel debuggers take care of it. :-))

I would image that from a code correctness's point of view it
shouldn't matter. In any case, nothing can be done if the kprobe
exception notifier is circumvented.

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

--
Thanks,
Abhishek Sagar

Re: [PATCH 3/3] x86: WARN_ON breakpoints from .kprobes.text section

[Masami Hiramatsu]

Hi,

Abhishek Sagar wrote:
> On 1/28/08, Masami Hiramatsu <mhiramat@redhat.com> wrote:
>> Thank you for explanation, I hope I can understand it.
>> Even if it causes a trap recursively, it could be checked (and ignored) by
>> longjump_break_handler(), and passed to the debugger correctly.
> 
> Yes, all non-kprobe breakpoints are left to the kernel to be handled.
> The objective here is to intercept the trap handling of a certain
> category of such breakpoints and emit a warning. The premise being
> that .kprobes.text section is a logical breakpoint-free zone.

Oh, I think I've gotten what misleads you.
The .kprobes.text section is a KPROBES-FREE zone. There may be
breakpoints owned by other debuggers and hand-coded breakpoints
(like as jprobe_return).

>> Please consider that someone expands jprobe(jprobe2) which uses
>> jprobe_return2() instead of jprobe_return(), how would you handle it?
> 
> By a simple modification of is_jprobe_bkpt() (defined in patch #2 of
> this series).

IMO, one of advantages of current logic is that you can add another break_handler-based
probe as an module without any patches. Even if someone makes fooprobe which is
not a jprobe variant, current logic can treat it correctly.
(Another advantage is the performance. Current logic checks only if there is a
 running kprobe and there is no kprobes related to the trapped address, instead of
 checking address section every time when each breakpoint is hit.)

>> Current kprobes provides an opportunity to those external probe frameworks
>> for checking it by themselves.
> 
> Could you clarirfy this with some example. For now I'm assuming that
> by external probe frameworks you mean kernel modules using kprobes.

Yes, I mentioned it above.

> If
> they embed breakpoints in their handlers, then they will simply not be
> caught by this check because thay cannot lie in the .kprobes.text
> section.

They cannot lie kprobes in the .kprobes.text section, but can put
breakpoints by hand. this is the reason why kprobes provides break_handler.

Thanks,
Best Regards,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com