Re: [PATCH v2 0/9] kprobes: Fix and improve blacklist symbols

From: Masami Hiramatsu
Date: Sun Jan 13 2019 - 09:23:24 EST


On Sat, 12 Jan 2019 14:33:24 +0100
Andrea Righi <righi.andrea@xxxxxxxxx> wrote:

> On Sat, Jan 12, 2019 at 11:25:40AM +0900, Masami Hiramatsu wrote:
> ...
> > And I found several functions which must be blacklisted.
> > - optprobe template code, which is just a template code and
> > never be executed. Moreover, since it can be copied and
> > reused, if we probe it, it modifies the template code and
> > can cause a crash. ([1/9][2/9])
> > - functions which is called before kprobe_int3_handler()
> > handles kprobes. This can cause a breakpoint recursion. ([3/9])
> > - IRQ entry text, which should not be probed since register/pagetable
> > status has not been stable at that point. ([4/9])
> > - Suffixed symbols, like .constprop, .part etc. Those suffixed
> > symbols never be blacklisted even if the non-suffixed version
> > has been blacklisted. ([5/9])
> > - hardirq tracer also works before int3 handling. ([6/9])
> > - preempt_check debug function also is involved in int3 handling.
> > ([7/9])
> > - RCU debug routine is also called before kprobe_int3_handler().
> > ([8/9])
> > - Some lockdep functions are also involved in int3 handling.
> > ([9/9])
> >
> > Of course there still may be some functions which can be called
> > by configuration change, I'll continue to test it.
>
> Hi Masami,
>
> I think I've found another recursion problem. Could you include also
> this one?

Yeah, if I will make new version, but basically please feel free to
send such blacklist update patch to LKML, me and Ingo :)

>
> Thanks,
>
> From: Andrea Righi <righi.andrea@xxxxxxxxx>
> Subject: [PATCH] kprobes: prohibit probing on bsearch()
>
> Since kprobe breakpoing handler is using bsearch(), probing on this
> routine can cause recursive breakpoint problem.
>
> int3
> ->do_int3()
> ->ftrace_int3_handler()
> ->ftrace_location()
> ->ftrace_location_range()
> ->bsearch() -> int3
>
> Prohibit probing on bsearch().
>
> Signed-off-by: Andrea Righi <righi.andrea@xxxxxxxxx>

This looks good to me.

Acked-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>

Thank you,


> ---
> lib/bsearch.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/lib/bsearch.c b/lib/bsearch.c
> index 18b445b010c3..82512fe7b33c 100644
> --- a/lib/bsearch.c
> +++ b/lib/bsearch.c
> @@ -11,6 +11,7 @@
>
> #include <linux/export.h>
> #include <linux/bsearch.h>
> +#include <linux/kprobes.h>
>
> /*
> * bsearch - binary search an array of elements
> @@ -53,3 +54,4 @@ void *bsearch(const void *key, const void *base, size_t num, size_t size,
> return NULL;
> }
> EXPORT_SYMBOL(bsearch);
> +NOKPROBE_SYMBOL(bsearch);
> --
> 2.17.1
>


--
Masami Hiramatsu <mhiramat@xxxxxxxxxx>