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

[Masami Hiramatsu]

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>