Em Thu, Apr 23, 2020 at 08:01:04PM +0900, Masami Hiramatsu escreveu:

Fix to check kprobe blacklist address correctly with relocated address by adjusting debuginfo address.

Since the address in the debuginfo is same as objdump, it is different from relocated kernel address with KASLR. Thus, the perf-probe always misses to catch the blacklisted addresses.

Thanks, applied, sorry for the delay,

- Arnaldo

Without this patch, perf probe can not detect the blacklist addresses on KASLR enabled kernel.

========= # perf probe kprobe_dispatcher Failed to write event: Invalid argument Error: Failed to add events. =========

With this patch, it correctly shows the error message.

========= # perf probe kprobe_dispatcher kprobe_dispatcher is blacklisted function, skip it. Probe point 'kprobe_dispatcher' not found. Error: Failed to add events. =========

Fixes: 9aaf5a5f479b ("perf probe: Check kprobes blacklist when adding new events") Signed-off-by: Masami Hiramatsu mhiramat@kernel.org Cc: stable@vger.kernel.org

---

tools/perf/util/probe-event.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index eea132f512b0..f75df63309be 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -102,7 +102,7 @@ void exit_probe_symbol_maps(void) symbol__exit(); } -static struct ref_reloc_sym *kernel_get_ref_reloc_sym(void) +static struct ref_reloc_sym *kernel_get_ref_reloc_sym(struct map **pmap) { /* kmap->ref_reloc_sym should be set if host_machine is initialized */ struct kmap *kmap; @@ -114,6 +114,10 @@ static struct ref_reloc_sym *kernel_get_ref_reloc_sym(void) kmap = map__kmap(map); if (!kmap) return NULL;

• if (pmap)

• *pmap = map;
  

• return kmap->ref_reloc_sym;

} @@ -125,7 +129,7 @@ static int kernel_get_symbol_address_by_name(const char *name, u64 *addr, struct map *map; /* ref_reloc_sym is just a label. Need a special fix*/

• reloc_sym = kernel_get_ref_reloc_sym();

• reloc_sym = kernel_get_ref_reloc_sym(NULL); if (reloc_sym && strcmp(name, reloc_sym->name) == 0) *addr = (reloc) ? reloc_sym->addr : reloc_sym->unrelocated_addr; else {

@@ -745,6 +749,7 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs, int ntevs) { struct ref_reloc_sym *reloc_sym;

• struct map *map; char *tmp; int i, skipped = 0;

@@ -753,7 +758,7 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs, return post_process_offline_probe_trace_events(tevs, ntevs, symbol_conf.vmlinux_name);

• reloc_sym = kernel_get_ref_reloc_sym();

• reloc_sym = kernel_get_ref_reloc_sym(&map); if (!reloc_sym) { pr_warning("Relocated base symbol is not found!\n"); return -EINVAL;

@@ -764,9 +769,13 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs, continue; if (tevs[i].point.retprobe && !kretprobe_offset_is_supported()) continue;

• /* If we found a wrong one, mark it by NULL symbol */
  

• /*
  

•  * If we found a wrong one, mark it by NULL symbol.
  

•  * Since addresses in debuginfo is same as objdump, we need
  

•  * to convert it to addresses on memory.
  

•  */
  

  if (kprobe_warn_out_range(tevs[i].point.symbol,

• 			  tevs[i].point.address)) {
  

• 	map__objdump_2mem(map, tevs[i].point.address))) {
  tmp = NULL;
  skipped++;
  

  } else {

@@ -2936,7 +2945,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev, /* Note that the symbols in the kmodule are not relocated */ if (!pev->uprobes && !pev->target && (!pp->retprobe || kretprobe_offset_is_supported())) {

• reloc_sym = kernel_get_ref_reloc_sym();
  

• reloc_sym = kernel_get_ref_reloc_sym(NULL);
  

  if (!reloc_sym) { pr_warning("Relocated base symbol is not found!\n"); ret = -EINVAL;

Em Thu, Apr 23, 2020 at 08:01:13PM +0900, Masami Hiramatsu escreveu:

Since commit 03db8b583d1c ("perf tools: Fix maps__find_symbol_by_name()") introduced map address range check in maps__find_symbol_by_name(), we can not get "_etext" from kernel map because _etext is placed on the edge of the kernel .text section (= kernel map in perf.)

To fix this issue, this checks the address correctness by map address range information (map->start and map->end) instead of using _etext address.

This can cause an error if the target inlined function is embedded in both __init function and normal function.

For exaample, request_resource() is a normal function but also embedded in __init reserve_setup(). In this case, the probe point in reserve_setup() must be skipped. However, without this fix, it failes to setup all probe points. ================ # ./perf probe -v request_resource probe-definition(0): request_resource symbol:request_resource file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Looking at the vmlinux_path (8 entries long) Using /usr/lib/debug/lib/modules/5.5.17-200.fc31.x86_64/vmlinux for symbols Open Debuginfo file: /usr/lib/debug/lib/modules/5.5.17-200.fc31.x86_64/vmlinux Try to find probe point from debuginfo. Matched function: request_resource [15e29ad] found inline addr: 0xffffffff82fbf892 Probe point found: reserve_setup+204 found inline addr: 0xffffffff810e9790 Probe point found: request_resource+0 Found 2 probe_trace_events. Opening /sys/kernel/debug/tracing//kprobe_events write=1 Opening /sys/kernel/debug/tracing//README write=0 Writing event: p:probe/request_resource _text+33290386 Failed to write event: Invalid argument Error: Failed to add events. Reason: Invalid argument (Code: -22) ================

With this fix,

================ # ./perf probe request_resource reserve_setup is out of .text, skip it. Added new events: (null):(null) (on request_resource)

But what is this (null):(null) probe? Confusing :-)

Thanks for working on this!

- Arnaldo

probe:request_resource (on request_resource)

You can now use it in all perf tools, such as:

perf record -e probe:request_resource -aR sleep 1

================

Fixes: 03db8b583d1c ("perf tools: Fix maps__find_symbol_by_name()") Reported-by: Arnaldo Carvalho de Melo acme@kernel.org Signed-off-by: Masami Hiramatsu mhiramat@kernel.org Cc: stable@vger.kernel.org

---

tools/perf/util/probe-event.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index f75df63309be..a5387e03e365 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -236,21 +236,22 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs) static bool kprobe_blacklist__listed(unsigned long address); static bool kprobe_warn_out_range(const char *symbol, unsigned long address) {

• u64 etext_addr = 0;
• int ret;
• /* Get the address of _etext for checking non-probable text symbol */
• ret = kernel_get_symbol_address_by_name("_etext", &etext_addr,

• 				false, false);
  

• struct map *map;
• bool ret = false;

• if (ret == 0 && etext_addr < address)

• pr_warning("%s is out of .text, skip it.\n", symbol);
  

• else if (kprobe_blacklist__listed(address))

• map = kernel_get_module_map(NULL);
• if (map) {

• ret = address <= map->start || map->end < address;
  

• if (ret)
  

• 	pr_warning("%s is out of .text, skip it.\n", symbol);
  

• map__put(map);
  

• }
• if (!ret && kprobe_blacklist__listed(address)) { pr_warning("%s is blacklisted function, skip it.\n", symbol);

• else

• return false;
  

• ret = true;
  

• }

• return true;

• return ret;

} /*

Em Thu, Apr 23, 2020 at 08:01:22PM +0900, Masami Hiramatsu escreveu:

When a probe point is expanded to several places (like inlined) and if some of them are skipped because of blacklisted or __init function, those trace_events has no event name. It must be skipped while showing results.

Without this fix, you can see "(null):(null)" on the list,

Ok, you broke the patch in two, I think its better to combine both, ok?

- Arnaldo

# ./perf probe request_resource reserve_setup is out of .text, skip it. Added new events: (null):(null) (on request_resource) probe:request_resource (on request_resource)

You can now use it in all perf tools, such as:

perf record -e probe:request_resource -aR sleep 1

===========

With this fix, it is ignored.

# ./perf probe request_resource reserve_setup is out of .text, skip it. Added new events: probe:request_resource (on request_resource)

You can now use it in all perf tools, such as:

perf record -e probe:request_resource -aR sleep 1

===========

Fixes: 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of .text") Signed-off-by: Masami Hiramatsu mhiramat@kernel.org Cc: stable@vger.kernel.org

---

tools/perf/builtin-probe.c | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c index 70548df2abb9..6b1507566770 100644 --- a/tools/perf/builtin-probe.c +++ b/tools/perf/builtin-probe.c @@ -364,6 +364,9 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs) for (k = 0; k < pev->ntevs; k++) { struct probe_trace_event *tev = &pev->tevs[k];

• 	/* Skipped events have no event name */
  

• 	if (!tev->event)
  

• 		continue;
  

/* We use tev's name for showing new events */ show_perf_probe_event(tev->group, tev->event, pev,

Em Fri, Apr 24, 2020 at 08:33:05AM +0900, Masami Hiramatsu escreveu:

On Thu, 23 Apr 2020 11:01:39 -0300 Arnaldo Carvalho de Melo acme@kernel.org wrote:

...

Em Thu, Apr 23, 2020 at 08:01:22PM +0900, Masami Hiramatsu escreveu:

...

When a probe point is expanded to several places (like inlined) and if some of them are skipped because of blacklisted or __init function, those trace_events has no event name. It must be skipped while showing results.

Without this fix, you can see "(null):(null)" on the list,

Ok, you broke the patch in two, I think its better to combine both, ok?

No, if an inlined function is embedded in blacklisted areas, it also shows same "(null):(null)" without [2/3].

Reordering the patches is OK, but this is still an independent fix.

Ok, so I'll try reordering, so that we don't see it in the cset log for the other fix.

Thanks for the clarification,

- Arnaldo

Thank you,

...

• Arnaldo

...

# ./perf probe request_resource reserve_setup is out of .text, skip it. Added new events: (null):(null) (on request_resource) probe:request_resource (on request_resource)

You can now use it in all perf tools, such as:

perf record -e probe:request_resource -aR sleep 1

===========

With this fix, it is ignored.

# ./perf probe request_resource reserve_setup is out of .text, skip it. Added new events: probe:request_resource (on request_resource)

You can now use it in all perf tools, such as:

perf record -e probe:request_resource -aR sleep 1

===========

Fixes: 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of .text") Signed-off-by: Masami Hiramatsu mhiramat@kernel.org Cc: stable@vger.kernel.org

---

tools/perf/builtin-probe.c | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c index 70548df2abb9..6b1507566770 100644 --- a/tools/perf/builtin-probe.c +++ b/tools/perf/builtin-probe.c @@ -364,6 +364,9 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs) for (k = 0; k < pev->ntevs; k++) { struct probe_trace_event *tev = &pev->tevs[k];

• 	/* Skipped events have no event name */
  

• 	if (!tev->event)
  

• 		continue;
  

/* We use tev's name for showing new events */ show_perf_probe_event(tev->group, tev->event, pev,

--

• Arnaldo

-- Masami Hiramatsu mhiramat@kernel.org