'perf probe' and symbols from .text.<something>

'perf probe' and symbols from .text.<something>

[Evgenii Shatokhin]

Hi,

It seems, 'perf probe' can only see functions from .text section in the 
kernel modules, but not from .text.unlikely or other .text.* sections.

For example, with kernel 5.11 and nf_conntrack.ko with debug info, 'perf 
probe' succeeds for nf_conntrack_attach() from .text and fails for 
nf_ct_resolve_clash() from .text.unlikely:

------------
# perf probe -v -m nf_conntrack nf_ct_resolve_clash
probe-definition(0): nf_ct_resolve_clash
symbol:nf_ct_resolve_clash file:(null) line:0 offset:0 return:0 lazy:(null)
0 arguments
Failed to get build-id from nf_conntrack.
Cache open error: -1
Open Debuginfo file: 
/lib/modules/5.11.0-test01/kernel/net/netfilter/nf_conntrack.ko
Try to find probe point from debuginfo.
Matched function: nf_ct_resolve_clash [33616]
Probe point found: nf_ct_resolve_clash+0
Found 1 probe_trace_events.
Post processing failed or all events are skipped. (-2)
Probe point 'nf_ct_resolve_clash' not found.
   Error: Failed to add events. Reason: No such file or directory (Code: -2)

# perf probe -v -m nf_conntrack nf_conntrack_attach
probe-definition(0): nf_conntrack_attach
symbol:nf_conntrack_attach file:(null) line:0 offset:0 return:0 lazy:(null)
0 arguments
Failed to get build-id from nf_conntrack.
Cache open error: -1
Open Debuginfo file: 
/lib/modules/5.11.0-test01/kernel/net/netfilter/nf_conntrack.ko
Try to find probe point from debuginfo.
Matched function: nf_conntrack_attach [2c8c3]
Probe point found: nf_conntrack_attach+0
Found 1 probe_trace_events.
Opening /sys/kernel/tracing//kprobe_events write=1
Opening /sys/kernel/tracing//README write=0
Writing event: p:probe/nf_conntrack_attach 
nf_conntrack:nf_conntrack_attach+0
Added new event:
   probe:nf_conntrack_attach (on nf_conntrack_attach in nf_conntrack)
------------

Is there a way to allow probing of functions in .text.<something> ?

Of course, one could place probes using absolute addresses of the 
functions but that would be less convenient.

This also affects many livepatch modules where the kernel code can be 
compiled with -ffunction-sections and each function may end up in a 
separate section .text.<function_name>. 'perf probe' cannot be used 
there, except with the absolute addresses.

Moreover, if FGKASLR patches are merged 
(https://lwn.net/Articles/832434/) and the kernel is built with FGKASLR 
enabled, -ffunction-sections will be used too. 'perf probe' will be 
unable to see the kernel functions then.

Regards,
Evgenii

Re: 'perf probe' and symbols from .text.<something>

[Josh Poimboeuf]

On Thu, Feb 18, 2021 at 08:09:17PM +0300, Evgenii Shatokhin wrote:
> Is there a way to allow probing of functions in .text.<something> ?
> 
> Of course, one could place probes using absolute addresses of the functions
> but that would be less convenient.
> 
> This also affects many livepatch modules where the kernel code can be
> compiled with -ffunction-sections and each function may end up in a separate
> section .text.<function_name>. 'perf probe' cannot be used there, except
> with the absolute addresses.
> 
> Moreover, if FGKASLR patches are merged (https://lwn.net/Articles/832434/)
> and the kernel is built with FGKASLR enabled, -ffunction-sections will be
> used too. 'perf probe' will be unable to see the kernel functions then.

A hack fix like the below would probably work, but as you pointed out,
FGKASLR is going to be a problem.  I suspect the proper fix is for perf
to learn how to deal with multiple executable ELF sections.

diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 69b9b71a6a47..0c522a87f6ce 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -4,6 +4,10 @@
  * combine them automatically.
  */
 SECTIONS {
+	.text : {
+		*(.text)
+		*(.text.*)
+	}
 	/DISCARD/ : {
 		*(.discard)
 		*(.discard.*)

Re: 'perf probe' and symbols from .text.<something>

[Masami Hiramatsu]

Hi Evgenii,

On Thu, 18 Feb 2021 20:09:17 +0300
Evgenii Shatokhin <eshatokhin@virtuozzo.com> wrote:

> Hi,
> 
> It seems, 'perf probe' can only see functions from .text section in the 
> kernel modules, but not from .text.unlikely or other .text.* sections.
> 
> For example, with kernel 5.11 and nf_conntrack.ko with debug info, 'perf 
> probe' succeeds for nf_conntrack_attach() from .text and fails for 
> nf_ct_resolve_clash() from .text.unlikely:

Thanks for reporting it!

> 
> ------------
> # perf probe -v -m nf_conntrack nf_ct_resolve_clash
> probe-definition(0): nf_ct_resolve_clash
> symbol:nf_ct_resolve_clash file:(null) line:0 offset:0 return:0 lazy:(null)
> 0 arguments
> Failed to get build-id from nf_conntrack.
> Cache open error: -1
> Open Debuginfo file: 
> /lib/modules/5.11.0-test01/kernel/net/netfilter/nf_conntrack.ko
> Try to find probe point from debuginfo.
> Matched function: nf_ct_resolve_clash [33616]
> Probe point found: nf_ct_resolve_clash+0
> Found 1 probe_trace_events.
> Post processing failed or all events are skipped. (-2)
> Probe point 'nf_ct_resolve_clash' not found.
>    Error: Failed to add events. Reason: No such file or directory (Code: -2)

The above log shows, an error occured while post_process_probe_trace_events(),
and the error code is -ENOENT (-2).
  ----
                pr_debug("Found %d probe_trace_events.\n", ntevs);
                ret = post_process_probe_trace_events(pev, *tevs, ntevs,
                                        pev->target, pev->uprobes, dinfo);
                if (ret < 0 || ret == ntevs) {
                        pr_debug("Post processing failed or all events are skipped. (%d)\n", ret);
  ----

In that function, map__find_symbol() failure will return -ENOENT.

  ----
/* Adjust symbol name and address */
static int post_process_probe_trace_point(struct probe_trace_point *tp,
                                           struct map *map, unsigned long offs)
{
        struct symbol *sym;
        u64 addr = tp->address - offs;

        sym = map__find_symbol(map, addr);
        if (!sym)
                return -ENOENT;
  ----

So it seems "map" may not load the symbol out of ".text".
This need to be fixed, since the map is widely used in the perf.

Anyway, since this is on a module, so even if it can not find the symbol
from map (or failed to load a map), it can fail back to the original symbol.
Let me fix that.

> # perf probe -v -m nf_conntrack nf_conntrack_attach
> probe-definition(0): nf_conntrack_attach
> symbol:nf_conntrack_attach file:(null) line:0 offset:0 return:0 lazy:(null)
> 0 arguments
> Failed to get build-id from nf_conntrack.
> Cache open error: -1
> Open Debuginfo file: 
> /lib/modules/5.11.0-test01/kernel/net/netfilter/nf_conntrack.ko
> Try to find probe point from debuginfo.
> Matched function: nf_conntrack_attach [2c8c3]
> Probe point found: nf_conntrack_attach+0
> Found 1 probe_trace_events.
> Opening /sys/kernel/tracing//kprobe_events write=1
> Opening /sys/kernel/tracing//README write=0
> Writing event: p:probe/nf_conntrack_attach 
> nf_conntrack:nf_conntrack_attach+0
> Added new event:
>    probe:nf_conntrack_attach (on nf_conntrack_attach in nf_conntrack)
> ------------
> 
> Is there a way to allow probing of functions in .text.<something> ?

I need to check how machine__kernel_maps() generated maps cut down .text.unlikely.
Arnaldo, I thought the maps in machine__kernel_maps() are generated from
kallsyms (doesn't check .text) right?

> 
> Of course, one could place probes using absolute addresses of the 
> functions but that would be less convenient.
> 
> This also affects many livepatch modules where the kernel code can be 
> compiled with -ffunction-sections and each function may end up in a 
> separate section .text.<function_name>. 'perf probe' cannot be used 
> there, except with the absolute addresses.
> 
> Moreover, if FGKASLR patches are merged 
> (https://lwn.net/Articles/832434/) and the kernel is built with FGKASLR 
> enabled, -ffunction-sections will be used too. 'perf probe' will be 
> unable to see the kernel functions then.

Hmm, if the FGKASLAR really randomizes the symbol address, perf-probe
should give up "_text-relative" probe for that kernel, and must fallback
to the "symbol-based" probe. (Are there any way to check the FGKASLR is on?)
The problem of "symbol-based" probe is that local (static) symbols
may share a same name sometimes. In that case, it can not find correct
symbol. (Maybe I can find a candidate from its size.)
Anyway, sometimes the security and usability are trade-off.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: 'perf probe' and symbols from .text.<something>

[Masami Hiramatsu]

On Tue, 23 Feb 2021 00:05:08 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hi Evgenii,
> 
> On Thu, 18 Feb 2021 20:09:17 +0300
> Evgenii Shatokhin <eshatokhin@virtuozzo.com> wrote:
> 
> > Hi,
> > 
> > It seems, 'perf probe' can only see functions from .text section in the 
> > kernel modules, but not from .text.unlikely or other .text.* sections.
> > 
> > For example, with kernel 5.11 and nf_conntrack.ko with debug info, 'perf 
> > probe' succeeds for nf_conntrack_attach() from .text and fails for 
> > nf_ct_resolve_clash() from .text.unlikely:
> 
> Thanks for reporting it!
> 
> > 
> > ------------
> > # perf probe -v -m nf_conntrack nf_ct_resolve_clash
> > probe-definition(0): nf_ct_resolve_clash
> > symbol:nf_ct_resolve_clash file:(null) line:0 offset:0 return:0 lazy:(null)
> > 0 arguments
> > Failed to get build-id from nf_conntrack.
> > Cache open error: -1
> > Open Debuginfo file: 
> > /lib/modules/5.11.0-test01/kernel/net/netfilter/nf_conntrack.ko
> > Try to find probe point from debuginfo.
> > Matched function: nf_ct_resolve_clash [33616]
> > Probe point found: nf_ct_resolve_clash+0
> > Found 1 probe_trace_events.
> > Post processing failed or all events are skipped. (-2)
> > Probe point 'nf_ct_resolve_clash' not found.
> >    Error: Failed to add events. Reason: No such file or directory (Code: -2)
> 
[...]

> > Is there a way to allow probing of functions in .text.<something> ?

BTW, just for putting a probe on nf_ct_resolve_clash, please give the module *path*
instead of the module *name*. For example,

perf probe -v -m /lib/modules/5.11.0-test01/kernel/net/netfilter/nf_conntrack.ko nf_ct_resolve_clash

This should work (at least works for me), because it directly loads the symbols from the .ko file.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: 'perf probe' and symbols from .text.<something>

[Josh Poimboeuf]

On Tue, Feb 23, 2021 at 12:05:08AM +0900, Masami Hiramatsu wrote:
> > Of course, one could place probes using absolute addresses of the 
> > functions but that would be less convenient.
> > 
> > This also affects many livepatch modules where the kernel code can be 
> > compiled with -ffunction-sections and each function may end up in a 
> > separate section .text.<function_name>. 'perf probe' cannot be used 
> > there, except with the absolute addresses.
> > 
> > Moreover, if FGKASLR patches are merged 
> > (https://lwn.net/Articles/832434/) and the kernel is built with FGKASLR 
> > enabled, -ffunction-sections will be used too. 'perf probe' will be 
> > unable to see the kernel functions then.
> 
> Hmm, if the FGKASLAR really randomizes the symbol address, perf-probe
> should give up "_text-relative" probe for that kernel, and must fallback
> to the "symbol-based" probe. (Are there any way to check the FGKASLR is on?)
> The problem of "symbol-based" probe is that local (static) symbols
> may share a same name sometimes. In that case, it can not find correct
> symbol. (Maybe I can find a candidate from its size.)
> Anyway, sometimes the security and usability are trade-off.

We had a similar issue with FGKASLR and live patching.  The proposed
solution is a new linker flag which eliminates duplicates: -z
unique-symbol.

https://sourceware.org/bugzilla/show_bug.cgi?id=26391

-- 
Josh

Re: 'perf probe' and symbols from .text.<something>

[Masami Hiramatsu]

On Mon, 22 Feb 2021 11:51:50 -0600
Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> On Tue, Feb 23, 2021 at 12:05:08AM +0900, Masami Hiramatsu wrote:
> > > Of course, one could place probes using absolute addresses of the 
> > > functions but that would be less convenient.
> > > 
> > > This also affects many livepatch modules where the kernel code can be 
> > > compiled with -ffunction-sections and each function may end up in a 
> > > separate section .text.<function_name>. 'perf probe' cannot be used 
> > > there, except with the absolute addresses.
> > > 
> > > Moreover, if FGKASLR patches are merged 
> > > (https://lwn.net/Articles/832434/) and the kernel is built with FGKASLR 
> > > enabled, -ffunction-sections will be used too. 'perf probe' will be 
> > > unable to see the kernel functions then.
> > 
> > Hmm, if the FGKASLAR really randomizes the symbol address, perf-probe
> > should give up "_text-relative" probe for that kernel, and must fallback
> > to the "symbol-based" probe. (Are there any way to check the FGKASLR is on?)
> > The problem of "symbol-based" probe is that local (static) symbols
> > may share a same name sometimes. In that case, it can not find correct
> > symbol. (Maybe I can find a candidate from its size.)
> > Anyway, sometimes the security and usability are trade-off.
> 
> We had a similar issue with FGKASLR and live patching.  The proposed
> solution is a new linker flag which eliminates duplicates: -z
> unique-symbol.
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=26391

Interesting, but it might not be enough for perf-probe.
Since the perf-probe has to handle both dwarf and elf, both must be
changed. I think the problem is that the dwarf is generated while
compiling, but this -z seems converting elf symbols in linkage.
As far as I can see, this appends ".COUNT" suffix to the non-unique
symbols in the linkage phase. Is that also applied to dwarf too?

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: 'perf probe' and symbols from .text.<something>

[Masami Hiramatsu]

On Tue, 23 Feb 2021 10:23:31 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Mon, 22 Feb 2021 11:51:50 -0600
> Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 
> > On Tue, Feb 23, 2021 at 12:05:08AM +0900, Masami Hiramatsu wrote:
> > > > Of course, one could place probes using absolute addresses of the 
> > > > functions but that would be less convenient.
> > > > 
> > > > This also affects many livepatch modules where the kernel code can be 
> > > > compiled with -ffunction-sections and each function may end up in a 
> > > > separate section .text.<function_name>. 'perf probe' cannot be used 
> > > > there, except with the absolute addresses.
> > > > 
> > > > Moreover, if FGKASLR patches are merged 
> > > > (https://lwn.net/Articles/832434/) and the kernel is built with FGKASLR 
> > > > enabled, -ffunction-sections will be used too. 'perf probe' will be 
> > > > unable to see the kernel functions then.
> > > 
> > > Hmm, if the FGKASLAR really randomizes the symbol address, perf-probe
> > > should give up "_text-relative" probe for that kernel, and must fallback
> > > to the "symbol-based" probe. (Are there any way to check the FGKASLR is on?)
> > > The problem of "symbol-based" probe is that local (static) symbols
> > > may share a same name sometimes. In that case, it can not find correct
> > > symbol. (Maybe I can find a candidate from its size.)
> > > Anyway, sometimes the security and usability are trade-off.
> > 
> > We had a similar issue with FGKASLR and live patching.  The proposed
> > solution is a new linker flag which eliminates duplicates: -z
> > unique-symbol.
> > 
> > https://sourceware.org/bugzilla/show_bug.cgi?id=26391
> 
> Interesting, but it might not be enough for perf-probe.
> Since the perf-probe has to handle both dwarf and elf, both must be
> changed. I think the problem is that the dwarf is generated while
> compiling, but this -z seems converting elf symbols in linkage.
> As far as I can see, this appends ".COUNT" suffix to the non-unique
> symbols in the linkage phase. Is that also applied to dwarf too?

Ah, OK. If there is an offline elf binary with symbol map, I can convert
DWARF symbol -> address -> offline elf symbol (unique name)-> kallsyms.
Currently, it directly converts address by kallsyms, so I will change it
to find elf-symbol and solve address by kallsyms in post processing.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: 'perf probe' and symbols from .text.<something>

[Josh Poimboeuf]

On Tue, Feb 23, 2021 at 04:36:19PM +0900, Masami Hiramatsu wrote:
> On Tue, 23 Feb 2021 10:23:31 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > On Mon, 22 Feb 2021 11:51:50 -0600
> > Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > 
> > > On Tue, Feb 23, 2021 at 12:05:08AM +0900, Masami Hiramatsu wrote:
> > > > > Of course, one could place probes using absolute addresses of the 
> > > > > functions but that would be less convenient.
> > > > > 
> > > > > This also affects many livepatch modules where the kernel code can be 
> > > > > compiled with -ffunction-sections and each function may end up in a 
> > > > > separate section .text.<function_name>. 'perf probe' cannot be used 
> > > > > there, except with the absolute addresses.
> > > > > 
> > > > > Moreover, if FGKASLR patches are merged 
> > > > > (https://lwn.net/Articles/832434/) and the kernel is built with FGKASLR 
> > > > > enabled, -ffunction-sections will be used too. 'perf probe' will be 
> > > > > unable to see the kernel functions then.
> > > > 
> > > > Hmm, if the FGKASLAR really randomizes the symbol address, perf-probe
> > > > should give up "_text-relative" probe for that kernel, and must fallback
> > > > to the "symbol-based" probe. (Are there any way to check the FGKASLR is on?)
> > > > The problem of "symbol-based" probe is that local (static) symbols
> > > > may share a same name sometimes. In that case, it can not find correct
> > > > symbol. (Maybe I can find a candidate from its size.)
> > > > Anyway, sometimes the security and usability are trade-off.
> > > 
> > > We had a similar issue with FGKASLR and live patching.  The proposed
> > > solution is a new linker flag which eliminates duplicates: -z
> > > unique-symbol.
> > > 
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=26391
> > 
> > Interesting, but it might not be enough for perf-probe.
> > Since the perf-probe has to handle both dwarf and elf, both must be
> > changed. I think the problem is that the dwarf is generated while
> > compiling, but this -z seems converting elf symbols in linkage.
> > As far as I can see, this appends ".COUNT" suffix to the non-unique
> > symbols in the linkage phase. Is that also applied to dwarf too?
> 
> Ah, OK. If there is an offline elf binary with symbol map, I can convert
> DWARF symbol -> address -> offline elf symbol (unique name)-> kallsyms.
> Currently, it directly converts address by kallsyms, so I will change it
> to find elf-symbol and solve address by kallsyms in post processing.

DWARF sections have references to the ELF symbols, which are renamed by
the linker.  So DWARF should automatically show the new symbol name.

And kallsyms is generated after the kernel is linked.  So I'm not sure I
understand the problem.

-- 
Josh

Re: 'perf probe' and symbols from .text.<something>

[Masami Hiramatsu]

On Tue, 23 Feb 2021 13:45:46 -0600
Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> On Tue, Feb 23, 2021 at 04:36:19PM +0900, Masami Hiramatsu wrote:
> > On Tue, 23 Feb 2021 10:23:31 +0900
> > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > 
> > > On Mon, 22 Feb 2021 11:51:50 -0600
> > > Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > > 
> > > > On Tue, Feb 23, 2021 at 12:05:08AM +0900, Masami Hiramatsu wrote:
> > > > > > Of course, one could place probes using absolute addresses of the 
> > > > > > functions but that would be less convenient.
> > > > > > 
> > > > > > This also affects many livepatch modules where the kernel code can be 
> > > > > > compiled with -ffunction-sections and each function may end up in a 
> > > > > > separate section .text.<function_name>. 'perf probe' cannot be used 
> > > > > > there, except with the absolute addresses.
> > > > > > 
> > > > > > Moreover, if FGKASLR patches are merged 
> > > > > > (https://lwn.net/Articles/832434/) and the kernel is built with FGKASLR 
> > > > > > enabled, -ffunction-sections will be used too. 'perf probe' will be 
> > > > > > unable to see the kernel functions then.
> > > > > 
> > > > > Hmm, if the FGKASLAR really randomizes the symbol address, perf-probe
> > > > > should give up "_text-relative" probe for that kernel, and must fallback
> > > > > to the "symbol-based" probe. (Are there any way to check the FGKASLR is on?)
> > > > > The problem of "symbol-based" probe is that local (static) symbols
> > > > > may share a same name sometimes. In that case, it can not find correct
> > > > > symbol. (Maybe I can find a candidate from its size.)
> > > > > Anyway, sometimes the security and usability are trade-off.
> > > > 
> > > > We had a similar issue with FGKASLR and live patching.  The proposed
> > > > solution is a new linker flag which eliminates duplicates: -z
> > > > unique-symbol.
> > > > 
> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=26391
> > > 
> > > Interesting, but it might not be enough for perf-probe.
> > > Since the perf-probe has to handle both dwarf and elf, both must be
> > > changed. I think the problem is that the dwarf is generated while
> > > compiling, but this -z seems converting elf symbols in linkage.
> > > As far as I can see, this appends ".COUNT" suffix to the non-unique
> > > symbols in the linkage phase. Is that also applied to dwarf too?
> > 
> > Ah, OK. If there is an offline elf binary with symbol map, I can convert
> > DWARF symbol -> address -> offline elf symbol (unique name)-> kallsyms.
> > Currently, it directly converts address by kallsyms, so I will change it
> > to find elf-symbol and solve address by kallsyms in post processing.
> 
> DWARF sections have references to the ELF symbols, which are renamed by
> the linker.  So DWARF should automatically show the new symbol name.

OK, I'll check what elfutils provides about that information.
> 
> And kallsyms is generated after the kernel is linked.  So I'm not sure I
> understand the problem.

Actually, perf-probe currently uses subprogram DIE(Dwarf node) name for
the symbol name and post-process tries to find correct symbol name
from kallsyms by the address.
So I have to change it to find the ELF symbol name from DIE itself.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH] perf-probe: Failback to symbol-base probe for probes on module

[Masami Hiramatsu]

If an error occurs on post processing (this converts probe point to
_text relative address for identifying non-unique symbols) for the
probes on module, failback to symbol-base probe.

There are many non-unique name symbols (local static functions can
be in the different name spaces) in the kernel. If perf-probe uses
a symbol-based probe definition, it can be put on an unintended
symbol. To avoid such mistake, perf-probe tries to convert the
address to the _text relative address expression.

However, such symbol duplication is rare for only one module. Thus
even if the conversion fails, perf probe can failback to the symbol
based probe.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/probe-event.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a9cff3a50ddf..af946f68e32e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -779,16 +779,16 @@ post_process_module_probe_trace_events(struct probe_trace_event *tevs,
 
 	map = get_target_map(module, NULL, false);
 	if (!map || debuginfo__get_text_offset(dinfo, &text_offs, true) < 0) {
-		pr_warning("Failed to get ELF symbols for %s\n", module);
-		return -EINVAL;
+		pr_info("NOTE: Failed to get ELF symbols for %s. Use symbol based probe.\n", module);
+		return 0;
 	}
 
 	mod_name = find_module_name(module);
 	for (i = 0; i < ntevs; i++) {
-		ret = post_process_probe_trace_point(&tevs[i].point,
-						map, (unsigned long)text_offs);
-		if (ret < 0)
-			break;
+		if (post_process_probe_trace_point(&tevs[i].point,
+				map, (unsigned long)text_offs) < 0)
+			pr_info("NOTE: %s is not in the symbol map. Use symbol based probe.\n",
+				   tevs[i].point.symbol);
 		tevs[i].point.module =
 			strdup(mod_name ? mod_name : module);
 		if (!tevs[i].point.module) {

Re: [PATCH] perf-probe: Failback to symbol-base probe for probes on module

[Masami Hiramatsu]

Please ignore this. I will send a better fix.

Thanks,

On Tue, 23 Feb 2021 10:48:30 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> If an error occurs on post processing (this converts probe point to
> _text relative address for identifying non-unique symbols) for the
> probes on module, failback to symbol-base probe.
> 
> There are many non-unique name symbols (local static functions can
> be in the different name spaces) in the kernel. If perf-probe uses
> a symbol-based probe definition, it can be put on an unintended
> symbol. To avoid such mistake, perf-probe tries to convert the
> address to the _text relative address expression.
> 
> However, such symbol duplication is rare for only one module. Thus
> even if the conversion fails, perf probe can failback to the symbol
> based probe.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/util/probe-event.c |   12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index a9cff3a50ddf..af946f68e32e 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -779,16 +779,16 @@ post_process_module_probe_trace_events(struct probe_trace_event *tevs,
>  
>  	map = get_target_map(module, NULL, false);
>  	if (!map || debuginfo__get_text_offset(dinfo, &text_offs, true) < 0) {
> -		pr_warning("Failed to get ELF symbols for %s\n", module);
> -		return -EINVAL;
> +		pr_info("NOTE: Failed to get ELF symbols for %s. Use symbol based probe.\n", module);
> +		return 0;
>  	}
>  
>  	mod_name = find_module_name(module);
>  	for (i = 0; i < ntevs; i++) {
> -		ret = post_process_probe_trace_point(&tevs[i].point,
> -						map, (unsigned long)text_offs);
> -		if (ret < 0)
> -			break;
> +		if (post_process_probe_trace_point(&tevs[i].point,
> +				map, (unsigned long)text_offs) < 0)
> +			pr_info("NOTE: %s is not in the symbol map. Use symbol based probe.\n",
> +				   tevs[i].point.symbol);
>  		tevs[i].point.module =
>  			strdup(mod_name ? mod_name : module);
>  		if (!tevs[i].point.module) {
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: 'perf probe' and symbols from .text.<something>

[Masami Hiramatsu]

Hi,

On Tue, 23 Feb 2021 00:05:08 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

>   ----
> /* Adjust symbol name and address */
> static int post_process_probe_trace_point(struct probe_trace_point *tp,
>                                            struct map *map, unsigned long offs)
> {
>         struct symbol *sym;
>         u64 addr = tp->address - offs;
> 
>         sym = map__find_symbol(map, addr);
>         if (!sym)
>                 return -ENOENT;
>   ----
> 
> So it seems "map" may not load the symbol out of ".text".
> This need to be fixed, since the map is widely used in the perf.

OK, I found a root cause of this issue.
dso__process_kernel_symbol() (which invoked from map__load() path) only adds the
symbols in ".text" section to the symbol list. It must be fixed.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH] perf-probe: dso: Add symbols in .text.* subsections to text symbol map in kenrel modules

[Masami Hiramatsu]

The kernel modules have .text.* subsections such as .text.unlikely.
Since dso__process_kernel_symbol() only identify the symbols in the ".text"
section as the text symbols and inserts it in the default dso in the map,
the symbols in such subsections can not be found by map__find_symbol().

This adds the symbols in those subsections to the default dso in the map so
that map__find_symbol() can find them. This solves the perf-probe issue on
probing online module.

Without this fix, probing on a symbol in .text.unlikely fails.
  ----
  # perf probe -m nf_conntrack nf_l4proto_log_invalid
  Probe point 'nf_l4proto_log_invalid' not found.
    Error: Failed to add events.
  ----

With this fix, it works because map__find_symbol() can find the symbol
correctly.
  ----
  # perf probe -m nf_conntrack nf_l4proto_log_invalid
  Added new event:
    probe:nf_l4proto_log_invalid (on nf_l4proto_log_invalid in nf_conntrack)

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

  	perf record -e probe:nf_l4proto_log_invalid -aR sleep 1

  ----

Reported-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/symbol-elf.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 6dff843fd883..0c1113236913 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -985,7 +985,9 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 	if (strcmp(section_name, (curr_dso->short_name + dso->short_name_len)) == 0)
 		return 0;
 
-	if (strcmp(section_name, ".text") == 0) {
+	/* .text and .text.* are included in the text dso */
+	if (strncmp(section_name, ".text", 5) == 0 &&
+	    (section_name[5] == '\0' || section_name[5] == '.')) {
 		/*
 		 * The initial kernel mapping is based on
 		 * kallsyms and identity maps.  Overwrite it to

Re: [PATCH] perf-probe: dso: Add symbols in .text.* subsections to text symbol map in kenrel modules

[Evgenii Shatokhin]

On 23.02.2021 10:37, Masami Hiramatsu wrote:
> The kernel modules have .text.* subsections such as .text.unlikely.
> Since dso__process_kernel_symbol() only identify the symbols in the ".text"
> section as the text symbols and inserts it in the default dso in the map,
> the symbols in such subsections can not be found by map__find_symbol().
> 
> This adds the symbols in those subsections to the default dso in the map so
> that map__find_symbol() can find them. This solves the perf-probe issue on
> probing online module.
> 
> Without this fix, probing on a symbol in .text.unlikely fails.
>    ----
>    # perf probe -m nf_conntrack nf_l4proto_log_invalid
>    Probe point 'nf_l4proto_log_invalid' not found.
>      Error: Failed to add events.
>    ----
> 
> With this fix, it works because map__find_symbol() can find the symbol
> correctly.
>    ----
>    # perf probe -m nf_conntrack nf_l4proto_log_invalid
>    Added new event:
>      probe:nf_l4proto_log_invalid (on nf_l4proto_log_invalid in nf_conntrack)
> 
>    You can now use it in all perf tools, such as:
> 
>    	perf record -e probe:nf_l4proto_log_invalid -aR sleep 1
> 
>    ----
> 
> Reported-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks for the fix!

It looks like it helps, at least with nf_conntrack in kernel 5.11:
---------------------
# ./perf probe -v -m nf_conntrack nf_ct_resolve_clash
probe-definition(0): nf_ct_resolve_clash
symbol:nf_ct_resolve_clash file:(null) line:0 offset:0 return:0 lazy:(null)
0 arguments
Failed to get build-id from nf_conntrack.
Cache open error: -1
Open Debuginfo file: 
/lib/modules/5.11.0-test01/kernel/net/netfilter/nf_conntrack.ko
Try to find probe point from debuginfo.
Matched function: nf_ct_resolve_clash [33616]
Probe point found: nf_ct_resolve_clash+0
Found 1 probe_trace_events.
Opening /sys/kernel/tracing//kprobe_events write=1
Opening /sys/kernel/tracing//README write=0
Writing event: p:probe/nf_ct_resolve_clash 
nf_conntrack:nf_ct_resolve_clash+0
Added new event:
   probe:nf_ct_resolve_clash (on nf_ct_resolve_clash in nf_conntrack)

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

         perf record -e probe:nf_ct_resolve_clash -aR sleep 1
---------------------

I guess, the patch is suitable for stable kernel branches as well.

Without the patch, the workaround you suggested earlier (using the full 
path to nf_conntrack.ko) works too.

> ---
>   tools/perf/util/symbol-elf.c |    4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 6dff843fd883..0c1113236913 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -985,7 +985,9 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
>   	if (strcmp(section_name, (curr_dso->short_name + dso->short_name_len)) == 0)
>   		return 0;
>   
> -	if (strcmp(section_name, ".text") == 0) {
> +	/* .text and .text.* are included in the text dso */
> +	if (strncmp(section_name, ".text", 5) == 0 &&
> +	    (section_name[5] == '\0' || section_name[5] == '.')) {
>   		/*
>   		 * The initial kernel mapping is based on
>   		 * kallsyms and identity maps.  Overwrite it to
> 
> .
> 

Regards,
Evgenii

Re: [PATCH] perf-probe: dso: Add symbols in .text.* subsections to text symbol map in kenrel modules

[Arnaldo Carvalho de Melo]

Em Tue, Feb 23, 2021 at 06:02:58PM +0300, Evgenii Shatokhin escreveu:
> On 23.02.2021 10:37, Masami Hiramatsu wrote:
> > The kernel modules have .text.* subsections such as .text.unlikely.
> > Since dso__process_kernel_symbol() only identify the symbols in the ".text"
> > section as the text symbols and inserts it in the default dso in the map,
> > the symbols in such subsections can not be found by map__find_symbol().
> > 
> > This adds the symbols in those subsections to the default dso in the map so
> > that map__find_symbol() can find them. This solves the perf-probe issue on
> > probing online module.
> > 
> > Without this fix, probing on a symbol in .text.unlikely fails.
> >    ----
> >    # perf probe -m nf_conntrack nf_l4proto_log_invalid
> >    Probe point 'nf_l4proto_log_invalid' not found.
> >      Error: Failed to add events.
> >    ----
> > 
> > With this fix, it works because map__find_symbol() can find the symbol
> > correctly.
> >    ----
> >    # perf probe -m nf_conntrack nf_l4proto_log_invalid
> >    Added new event:
> >      probe:nf_l4proto_log_invalid (on nf_l4proto_log_invalid in nf_conntrack)
> > 
> >    You can now use it in all perf tools, such as:
> > 
> >    	perf record -e probe:nf_l4proto_log_invalid -aR sleep 1
> > 
> >    ----
> > 
> > Reported-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> 
> Thanks for the fix!
> 
> It looks like it helps, at least with nf_conntrack in kernel 5.11:

So I'm taking this as you providing a:

Tested-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>

ok?

- Arnaldo

> ---------------------
> # ./perf probe -v -m nf_conntrack nf_ct_resolve_clash
> probe-definition(0): nf_ct_resolve_clash
> symbol:nf_ct_resolve_clash file:(null) line:0 offset:0 return:0 lazy:(null)
> 0 arguments
> Failed to get build-id from nf_conntrack.
> Cache open error: -1
> Open Debuginfo file:
> /lib/modules/5.11.0-test01/kernel/net/netfilter/nf_conntrack.ko
> Try to find probe point from debuginfo.
> Matched function: nf_ct_resolve_clash [33616]
> Probe point found: nf_ct_resolve_clash+0
> Found 1 probe_trace_events.
> Opening /sys/kernel/tracing//kprobe_events write=1
> Opening /sys/kernel/tracing//README write=0
> Writing event: p:probe/nf_ct_resolve_clash
> nf_conntrack:nf_ct_resolve_clash+0
> Added new event:
>   probe:nf_ct_resolve_clash (on nf_ct_resolve_clash in nf_conntrack)
> 
> You can now use it in all perf tools, such as:
> 
>         perf record -e probe:nf_ct_resolve_clash -aR sleep 1
> ---------------------
> 
> I guess, the patch is suitable for stable kernel branches as well.
> 
> Without the patch, the workaround you suggested earlier (using the full path
> to nf_conntrack.ko) works too.
> 
> > ---
> >   tools/perf/util/symbol-elf.c |    4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > index 6dff843fd883..0c1113236913 100644
> > --- a/tools/perf/util/symbol-elf.c
> > +++ b/tools/perf/util/symbol-elf.c
> > @@ -985,7 +985,9 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
> >   	if (strcmp(section_name, (curr_dso->short_name + dso->short_name_len)) == 0)
> >   		return 0;
> > -	if (strcmp(section_name, ".text") == 0) {
> > +	/* .text and .text.* are included in the text dso */
> > +	if (strncmp(section_name, ".text", 5) == 0 &&
> > +	    (section_name[5] == '\0' || section_name[5] == '.')) {
> >   		/*
> >   		 * The initial kernel mapping is based on
> >   		 * kallsyms and identity maps.  Overwrite it to
> > 
> > .
> > 
> 
> Regards,
> Evgenii

-- 

- Arnaldo

Re: [PATCH] perf-probe: dso: Add symbols in .text.* subsections to text symbol map in kenrel modules

[Evgenii Shatokhin]

On 23.02.2021 23:11, Arnaldo Carvalho de Melo wrote:
> Em Tue, Feb 23, 2021 at 06:02:58PM +0300, Evgenii Shatokhin escreveu:
>> On 23.02.2021 10:37, Masami Hiramatsu wrote:
>>> The kernel modules have .text.* subsections such as .text.unlikely.
>>> Since dso__process_kernel_symbol() only identify the symbols in the ".text"
>>> section as the text symbols and inserts it in the default dso in the map,
>>> the symbols in such subsections can not be found by map__find_symbol().
>>>
>>> This adds the symbols in those subsections to the default dso in the map so
>>> that map__find_symbol() can find them. This solves the perf-probe issue on
>>> probing online module.
>>>
>>> Without this fix, probing on a symbol in .text.unlikely fails.
>>>     ----
>>>     # perf probe -m nf_conntrack nf_l4proto_log_invalid
>>>     Probe point 'nf_l4proto_log_invalid' not found.
>>>       Error: Failed to add events.
>>>     ----
>>>
>>> With this fix, it works because map__find_symbol() can find the symbol
>>> correctly.
>>>     ----
>>>     # perf probe -m nf_conntrack nf_l4proto_log_invalid
>>>     Added new event:
>>>       probe:nf_l4proto_log_invalid (on nf_l4proto_log_invalid in nf_conntrack)
>>>
>>>     You can now use it in all perf tools, such as:
>>>
>>>     	perf record -e probe:nf_l4proto_log_invalid -aR sleep 1
>>>
>>>     ----
>>>
>>> Reported-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
>>> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
>>
>> Thanks for the fix!
>>
>> It looks like it helps, at least with nf_conntrack in kernel 5.11:
> 
> So I'm taking this as you providing a:
> 
> Tested-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
> 
> ok?

Sure, thanks!

> 
> - Arnaldo
> 
>> ---------------------
>> # ./perf probe -v -m nf_conntrack nf_ct_resolve_clash
>> probe-definition(0): nf_ct_resolve_clash
>> symbol:nf_ct_resolve_clash file:(null) line:0 offset:0 return:0 lazy:(null)
>> 0 arguments
>> Failed to get build-id from nf_conntrack.
>> Cache open error: -1
>> Open Debuginfo file:
>> /lib/modules/5.11.0-test01/kernel/net/netfilter/nf_conntrack.ko
>> Try to find probe point from debuginfo.
>> Matched function: nf_ct_resolve_clash [33616]
>> Probe point found: nf_ct_resolve_clash+0
>> Found 1 probe_trace_events.
>> Opening /sys/kernel/tracing//kprobe_events write=1
>> Opening /sys/kernel/tracing//README write=0
>> Writing event: p:probe/nf_ct_resolve_clash
>> nf_conntrack:nf_ct_resolve_clash+0
>> Added new event:
>>    probe:nf_ct_resolve_clash (on nf_ct_resolve_clash in nf_conntrack)
>>
>> You can now use it in all perf tools, such as:
>>
>>          perf record -e probe:nf_ct_resolve_clash -aR sleep 1
>> ---------------------
>>
>> I guess, the patch is suitable for stable kernel branches as well.
>>
>> Without the patch, the workaround you suggested earlier (using the full path
>> to nf_conntrack.ko) works too.
>>
>>> ---
>>>    tools/perf/util/symbol-elf.c |    4 +++-
>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
>>> index 6dff843fd883..0c1113236913 100644
>>> --- a/tools/perf/util/symbol-elf.c
>>> +++ b/tools/perf/util/symbol-elf.c
>>> @@ -985,7 +985,9 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
>>>    	if (strcmp(section_name, (curr_dso->short_name + dso->short_name_len)) == 0)
>>>    		return 0;
>>> -	if (strcmp(section_name, ".text") == 0) {
>>> +	/* .text and .text.* are included in the text dso */
>>> +	if (strncmp(section_name, ".text", 5) == 0 &&
>>> +	    (section_name[5] == '\0' || section_name[5] == '.')) {
>>>    		/*
>>>    		 * The initial kernel mapping is based on
>>>    		 * kallsyms and identity maps.  Overwrite it to
>>>
>>> .
>>>
>>
>> Regards,
>> Evgenii
>