LKML Archive on lore.kernel.org
 help / color / Atom feed
* [RFC][PATCH] kprobes/x86: While list ftrace locations in kprobe blacklist areas
@ 2019-10-10 21:52 Steven Rostedt
  2019-10-10 21:55 ` Steven Rostedt
  2019-10-11  8:39 ` Masami Hiramatsu
  0 siblings, 2 replies; 4+ messages in thread
From: Steven Rostedt @ 2019-10-10 21:52 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: LKML, Ingo Molnar, Peter Zijlstra, H. Peter Anvin,
	Thomas Gleixner, Andrew Morton

From: Steven Rostedt (VMware) <rostedt@goodmis.org>

I noticed some of my old tests failing on kprobes, and realized that
this was due to black listing irq_entry functions on x86 from being
used by kprobes. IIRC, this was due to the cr2 being corrupted and
such, and I believe other things were to cause. But black listing all
irq_entry code is a big hammer to this.

 (See commit 0eae81dc9f026 "x86/kprobes: Prohibit probing on IRQ
 handlers directly" for more details)

Anyway, if kprobes is using ftrace as a hook, there shouldn't be any
problems here. If we white list ftrace locations in the range of
kprobe_add_area_blacklist(), it should be safe.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index d9770a5393c8..9d28a279282c 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -2124,6 +2124,11 @@ int kprobe_add_area_blacklist(unsigned long start, unsigned long end)
 	int ret = 0;
 
 	for (entry = start; entry < end; entry += ret) {
+#ifdef CONFIG_KPROBES_ON_FTRACE
+		/* We are safe if using ftrace */
+		if (ftrace_location(entry))
+			continue;
+#endif
 		ret = kprobe_add_ksym_blacklist(entry);
 		if (ret < 0)
 			return ret;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC][PATCH] kprobes/x86: While list ftrace locations in kprobe blacklist areas
  2019-10-10 21:52 [RFC][PATCH] kprobes/x86: While list ftrace locations in kprobe blacklist areas Steven Rostedt
@ 2019-10-10 21:55 ` Steven Rostedt
  2019-10-11  8:39 ` Masami Hiramatsu
  1 sibling, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2019-10-10 21:55 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: LKML, Ingo Molnar, Peter Zijlstra, H. Peter Anvin,
	Thomas Gleixner, Andrew Morton

On Thu, 10 Oct 2019 17:52:16 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> I noticed some of my old tests failing on kprobes, and realized that
> this was due to black listing irq_entry functions on x86 from being
> used by kprobes. IIRC, this was due to the cr2 being corrupted and
> such, and I believe other things were to cause. But black listing all
> irq_entry code is a big hammer to this.
> 
>  (See commit 0eae81dc9f026 "x86/kprobes: Prohibit probing on IRQ
>  handlers directly" for more details)

BTW, I noticed this recently (again) when running my tests by hand. I
forgot that I have my automated tests revert the above commit before
compiling the kernel it is about to test (because it tests kprobes on
irq entry locations!). My tests never had issues with kprobes on irq
entry locations.

-- Steve

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC][PATCH] kprobes/x86: While list ftrace locations in kprobe blacklist areas
  2019-10-10 21:52 [RFC][PATCH] kprobes/x86: While list ftrace locations in kprobe blacklist areas Steven Rostedt
  2019-10-10 21:55 ` Steven Rostedt
@ 2019-10-11  8:39 ` Masami Hiramatsu
  2019-10-11 13:12   ` Steven Rostedt
  1 sibling, 1 reply; 4+ messages in thread
From: Masami Hiramatsu @ 2019-10-11  8:39 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Ingo Molnar, Peter Zijlstra, H. Peter Anvin,
	Thomas Gleixner, Andrew Morton

Hi Steve,

On Thu, 10 Oct 2019 17:52:16 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt (VMware) <rostedt@goodmis.org>
> 
> I noticed some of my old tests failing on kprobes, and realized that
> this was due to black listing irq_entry functions on x86 from being
> used by kprobes. IIRC, this was due to the cr2 being corrupted and
> such, and I believe other things were to cause. But black listing all
> irq_entry code is a big hammer to this.

OK, I think if we can use ftrace for hooking, probing on "that address"
is good, but the function body still can not be probed.

> 
>  (See commit 0eae81dc9f026 "x86/kprobes: Prohibit probing on IRQ
>  handlers directly" for more details)
> 
> Anyway, if kprobes is using ftrace as a hook, there shouldn't be any
> problems here. If we white list ftrace locations in the range of
> kprobe_add_area_blacklist(), it should be safe.

Agreed.

> 
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index d9770a5393c8..9d28a279282c 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -2124,6 +2124,11 @@ int kprobe_add_area_blacklist(unsigned long start, unsigned long end)
>  	int ret = 0;
>  
>  	for (entry = start; entry < end; entry += ret) {
> +#ifdef CONFIG_KPROBES_ON_FTRACE
> +		/* We are safe if using ftrace */
> +		if (ftrace_location(entry))
> +			continue;
> +#endif

Have you tested the patch? it doesn't measure the entry function's size.
(kprobe_add_ksym_blacklist(entry) returns the function size)

Could you do this in kprobe_add_ksym_blacklist()?
Instead of continue, increment ent->start_addr by MCOUNT size(?) will be OK.
(Note that since each blacklist symbol is managed independently, you can make
 a "gap" between them as a safe area)

Thank you,

>  		ret = kprobe_add_ksym_blacklist(entry);
>  		if (ret < 0)
>  			return ret;


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC][PATCH] kprobes/x86: While list ftrace locations in kprobe blacklist areas
  2019-10-11  8:39 ` Masami Hiramatsu
@ 2019-10-11 13:12   ` Steven Rostedt
  0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2019-10-11 13:12 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: LKML, Ingo Molnar, Peter Zijlstra, H. Peter Anvin,
	Thomas Gleixner, Andrew Morton

On Fri, 11 Oct 2019 17:39:29 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hi Steve,
> 
> On Thu, 10 Oct 2019 17:52:16 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > From: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > 
> > I noticed some of my old tests failing on kprobes, and realized that
> > this was due to black listing irq_entry functions on x86 from being
> > used by kprobes. IIRC, this was due to the cr2 being corrupted and
> > such, and I believe other things were to cause. But black listing all
> > irq_entry code is a big hammer to this.  
> 
> OK, I think if we can use ftrace for hooking, probing on "that address"
> is good, but the function body still can not be probed.
> 
> > 
> >  (See commit 0eae81dc9f026 "x86/kprobes: Prohibit probing on IRQ
> >  handlers directly" for more details)
> > 
> > Anyway, if kprobes is using ftrace as a hook, there shouldn't be any
> > problems here. If we white list ftrace locations in the range of
> > kprobe_add_area_blacklist(), it should be safe.  
> 
> Agreed.
> 
> > 
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > ---
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index d9770a5393c8..9d28a279282c 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -2124,6 +2124,11 @@ int kprobe_add_area_blacklist(unsigned long start, unsigned long end)
> >  	int ret = 0;
> >  
> >  	for (entry = start; entry < end; entry += ret) {
> > +#ifdef CONFIG_KPROBES_ON_FTRACE
> > +		/* We are safe if using ftrace */
> > +		if (ftrace_location(entry))
> > +			continue;
> > +#endif  
> 
> Have you tested the patch? it doesn't measure the entry function's size.
> (kprobe_add_ksym_blacklist(entry) returns the function size)

Yeah, I tested the patch, but it is obviously buggy. It fixed the issue
I was having (was able to test against do_IRQ ;-)

> 
> Could you do this in kprobe_add_ksym_blacklist()?
> Instead of continue, increment ent->start_addr by MCOUNT size(?) will be OK.
> (Note that since each blacklist symbol is managed independently, you can make
>  a "gap" between them as a safe area)

OK, I'll move it to kprobe_add_ksym_blacklist.

-- Steve

> 
> Thank you,
> 
> >  		ret = kprobe_add_ksym_blacklist(entry);
> >  		if (ret < 0)
> >  			return ret;  
> 
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-10 21:52 [RFC][PATCH] kprobes/x86: While list ftrace locations in kprobe blacklist areas Steven Rostedt
2019-10-10 21:55 ` Steven Rostedt
2019-10-11  8:39 ` Masami Hiramatsu
2019-10-11 13:12   ` Steven Rostedt

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox