All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf probe: Fix kprobe blacklist checking condition
@ 2017-08-29 12:57 Li Bin
  2017-08-29 14:51 ` Arnaldo Carvalho de Melo
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Li Bin @ 2017-08-29 12:57 UTC (permalink / raw)
  To: Peter Zijlstra, Namhyung Kim, Arnaldo Carvalho de Melo, Masami Hiramatsu
  Cc: linux-kernel, Wang Nan, huawei.libin, zhangmengting

The commit 9aaf5a5("perf probe: Check kprobes blacklist
when adding new events"), perf probe supports checking
the blacklist of the fuctions which can not be probed.
But the checking condition is wrong, that the end_addr
of the symbol which is the start_addr of the next symbol
can't be included.

Signed-off-by: Li Bin <huawei.libin@huawei.com>
---
 tools/perf/util/probe-event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a2670e9..bf7c928 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2373,7 +2373,7 @@ static int kprobe_blacklist__load(struct list_head *blacklist)
 	struct kprobe_blacklist_node *node;
 
 	list_for_each_entry(node, blacklist, list) {
-		if (node->start <= address && address <= node->end)
+		if (node->start <= address && address < node->end)
 			return node;
 	}
 
-- 
1.7.12.4

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

* Re: [PATCH] perf probe: Fix kprobe blacklist checking condition
  2017-08-29 12:57 [PATCH] perf probe: Fix kprobe blacklist checking condition Li Bin
@ 2017-08-29 14:51 ` Arnaldo Carvalho de Melo
  2017-08-29 21:25 ` [tip:perf/core] " tip-bot for Li Bin
  2017-08-30 12:28 ` [PATCH] " Masami Hiramatsu
  2 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-08-29 14:51 UTC (permalink / raw)
  To: Li Bin
  Cc: Peter Zijlstra, Namhyung Kim, Masami Hiramatsu, linux-kernel,
	Wang Nan, zhangmengting

Em Tue, Aug 29, 2017 at 08:57:23PM +0800, Li Bin escreveu:
> The commit 9aaf5a5("perf probe: Check kprobes blacklist
> when adding new events"), perf probe supports checking
> the blacklist of the fuctions which can not be probed.
> But the checking condition is wrong, that the end_addr
> of the symbol which is the start_addr of the next symbol
> can't be included.

Applied, with the following committer notes added:

    Committer notes:
    
    IOW make it match its kernel counterpart in kernel/kprobes.c:
    
      bool within_kprobe_blacklist(unsigned long addr)
    
    Each entry have as its end address not its end address, but the first
    address _outside_ that symbol, which for related functions, is the first
    address of the next symbol, like these from kernel/trace/trace_probe.c:
    
    0xffffffffbd198df0-0xffffffffbd198e40   print_type_u8
    0xffffffffbd198e40-0xffffffffbd198e90   print_type_u16
    0xffffffffbd198e90-0xffffffffbd198ee0   print_type_u32
    0xffffffffbd198ee0-0xffffffffbd198f30   print_type_u64
    0xffffffffbd198f30-0xffffffffbd198f80   print_type_s8
    0xffffffffbd198f80-0xffffffffbd198fd0   print_type_s16
    0xffffffffbd198fd0-0xffffffffbd199020   print_type_s32
    0xffffffffbd199020-0xffffffffbd199070   print_type_s64
    0xffffffffbd199070-0xffffffffbd1990c0   print_type_x8
    0xffffffffbd1990c0-0xffffffffbd199110   print_type_x16
    0xffffffffbd199110-0xffffffffbd199160   print_type_x32
    0xffffffffbd199160-0xffffffffbd1991b0   print_type_x64
    
    But not always:
    
    0xffffffffbd1997b0-0xffffffffbd1997c0   fetch_kernel_stack_address (kernel/trace/trace_probe.c)
    0xffffffffbd1c57f0-0xffffffffbd1c58b0   __context_tracking_enter   (kernel/context_tracking.c)
    
    Fixes: 9aaf5a5f479b ("perf probe: Check kprobes blacklist when adding new events")

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

* [tip:perf/core] perf probe: Fix kprobe blacklist checking condition
  2017-08-29 12:57 [PATCH] perf probe: Fix kprobe blacklist checking condition Li Bin
  2017-08-29 14:51 ` Arnaldo Carvalho de Melo
@ 2017-08-29 21:25 ` tip-bot for Li Bin
  2017-08-30 12:28 ` [PATCH] " Masami Hiramatsu
  2 siblings, 0 replies; 4+ messages in thread
From: tip-bot for Li Bin @ 2017-08-29 21:25 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, wangnan0, tglx, namhyung, mhiramat, hpa, acme,
	huawei.libin, linux-kernel, peterz

Commit-ID:  2c29461e273abaf149cf8220c3403e9d67dd8b61
Gitweb:     http://git.kernel.org/tip/2c29461e273abaf149cf8220c3403e9d67dd8b61
Author:     Li Bin <huawei.libin@huawei.com>
AuthorDate: Tue, 29 Aug 2017 20:57:23 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 29 Aug 2017 11:14:12 -0300

perf probe: Fix kprobe blacklist checking condition

The commit 9aaf5a5f479b ("perf probe: Check kprobes blacklist when
adding new events"), 'perf probe' supports checking the blacklist of the
fuctions which can not be probed.  But the checking condition is wrong,
that the end_addr of the symbol which is the start_addr of the next
symbol can't be included.

Committer notes:

IOW make it match its kernel counterpart in kernel/kprobes.c:

  bool within_kprobe_blacklist(unsigned long addr)

Each entry have as its end address not its end address, but the first
address _outside_ that symbol, which for related functions, is the first
address of the next symbol, like these from kernel/trace/trace_probe.c:

0xffffffffbd198df0-0xffffffffbd198e40	print_type_u8
0xffffffffbd198e40-0xffffffffbd198e90	print_type_u16
0xffffffffbd198e90-0xffffffffbd198ee0	print_type_u32
0xffffffffbd198ee0-0xffffffffbd198f30	print_type_u64
0xffffffffbd198f30-0xffffffffbd198f80	print_type_s8
0xffffffffbd198f80-0xffffffffbd198fd0	print_type_s16
0xffffffffbd198fd0-0xffffffffbd199020	print_type_s32
0xffffffffbd199020-0xffffffffbd199070	print_type_s64
0xffffffffbd199070-0xffffffffbd1990c0	print_type_x8
0xffffffffbd1990c0-0xffffffffbd199110	print_type_x16
0xffffffffbd199110-0xffffffffbd199160	print_type_x32
0xffffffffbd199160-0xffffffffbd1991b0	print_type_x64

But not always:

0xffffffffbd1997b0-0xffffffffbd1997c0	fetch_kernel_stack_address (kernel/trace/trace_probe.c)
0xffffffffbd1c57f0-0xffffffffbd1c58b0	__context_tracking_enter   (kernel/context_tracking.c)

Signed-off-by: Li Bin <huawei.libin@huawei.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: zhangmengting@huawei.com
Fixes: 9aaf5a5f479b ("perf probe: Check kprobes blacklist when adding new events")
Link: http://lkml.kernel.org/r/1504011443-7269-1-git-send-email-huawei.libin@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index d7cd114..b7aaf9b 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2395,7 +2395,7 @@ kprobe_blacklist__find_by_address(struct list_head *blacklist,
 	struct kprobe_blacklist_node *node;
 
 	list_for_each_entry(node, blacklist, list) {
-		if (node->start <= address && address <= node->end)
+		if (node->start <= address && address < node->end)
 			return node;
 	}
 

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

* Re: [PATCH] perf probe: Fix kprobe blacklist checking condition
  2017-08-29 12:57 [PATCH] perf probe: Fix kprobe blacklist checking condition Li Bin
  2017-08-29 14:51 ` Arnaldo Carvalho de Melo
  2017-08-29 21:25 ` [tip:perf/core] " tip-bot for Li Bin
@ 2017-08-30 12:28 ` Masami Hiramatsu
  2 siblings, 0 replies; 4+ messages in thread
From: Masami Hiramatsu @ 2017-08-30 12:28 UTC (permalink / raw)
  To: Li Bin
  Cc: Peter Zijlstra, Namhyung Kim, Arnaldo Carvalho de Melo,
	linux-kernel, Wang Nan, zhangmengting

On Tue, 29 Aug 2017 20:57:23 +0800
Li Bin <huawei.libin@huawei.com> wrote:

> The commit 9aaf5a5("perf probe: Check kprobes blacklist
> when adding new events"), perf probe supports checking
> the blacklist of the fuctions which can not be probed.
> But the checking condition is wrong, that the end_addr
> of the symbol which is the start_addr of the next symbol
> can't be included.

Oops, right.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

BTW, should we use memory_contains() macro for this check too...

> 
> Signed-off-by: Li Bin <huawei.libin@huawei.com>
> ---
>  tools/perf/util/probe-event.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index a2670e9..bf7c928 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2373,7 +2373,7 @@ static int kprobe_blacklist__load(struct list_head *blacklist)
>  	struct kprobe_blacklist_node *node;
>  
>  	list_for_each_entry(node, blacklist, list) {
> -		if (node->start <= address && address <= node->end)
> +		if (node->start <= address && address < node->end)
>  			return node;
>  	}
>  
> -- 
> 1.7.12.4
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

end of thread, other threads:[~2017-08-30 12:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-29 12:57 [PATCH] perf probe: Fix kprobe blacklist checking condition Li Bin
2017-08-29 14:51 ` Arnaldo Carvalho de Melo
2017-08-29 21:25 ` [tip:perf/core] " tip-bot for Li Bin
2017-08-30 12:28 ` [PATCH] " Masami Hiramatsu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.