On Fri, Jun 09, 2023 at 11:29:59AM -0700, Andrii Nakryiko wrote:

On Fri, Jun 9, 2023 at 9:44 AM Jiri Olsa [off-list ref] wrote:

quoted

On Fri, Jun 09, 2023 at 09:24:10AM +0100, Mark Rutland wrote:

quoted

On Thu, Jun 08, 2023 at 04:55:40PM -0700, Andrii Nakryiko wrote:

quoted

On Thu, Jun 8, 2023 at 4:27 PM Steven Rostedt [off-list ref] wrote:

quoted

On Thu, 8 Jun 2023 15:43:03 -0700 Andrii Nakryiko [off-list ref] wrote:

quoted

On Thu, Jun 8, 2023 at 2:26 PM Jiri Olsa [off-list ref] wrote:

quoted

There are BPF tools that allow user to specify regex/glob of kernel
functions to attach to. This regex/glob is checked against
available_filter_functions to check which functions are traceable. All
good. But then also it's important to have corresponding memory
addresses for selected functions (for many reasons, e.g., to have
non-ambiguous and fast attachment by address instead of by name, or
for some post-processing based on captured IP addresses, etc). And
that means that now we need to also parse /proc/kallsyms and
cross-join it with data fetched from available_filter_functions.

All this is unnecessary if avalable_filter_functions would just
provide function address in the first place. It's a huge
simplification. And saves memory and CPU.

Do you need the address of the function entry-point or the address of the
patch-site within the function? Those can differ, and the rec->ip address won't
necessarily equal the address in /proc/kallsyms, so the pointer in
/proc/kallsyms won't (always) match the address we could print for the ftrace site.

On arm64, today we can have offsets of +0, +4, and +8, and within a single
kernel image different functions can have different offsets. I suspect in
future that we may have more potential offsets (e.g. due to changes for HW/SW
CFI).

so we need that for kprobe_multi bpf link, which is based on fprobe,
and that uses ftrace_set_filter_ips to setup the ftrace_ops filter

and ftrace_set_filter_ips works fine with ip address being the address
of the patched instruction (it's matched in ftrace_location)

but right, I did not realize this.. it might cause confusion if people
don't know it's patch-side addresses..  not sure if there's easy way to
get real function address out of rec->ip, but it will also get more
complicated on x86 when IBT is enabled, will check

ok, sorry, I'm confused. Two questions:

1. when attaching kprobe_multi, does bpf() syscall expect function
address or (func+offset_of_mcount) address? I hope it's the former,
just function's address?

it can be both, the fprobe/ftrace filter setup will take care of looking
up and translating the provided address to the patch-side address

2. If rec->ip is not function's address, can we somehow adjust the
value to be a function address before printing it?

that's tricky because on x86 with IBT we would need to read the first
instruction and check if it's endbr to get the real address, like we
do in get_entry_ip:

	#ifdef CONFIG_X86_KERNEL_IBT
	static unsigned long get_entry_ip(unsigned long fentry_ip)
	{
		u32 instr;

		/* Being extra safe in here in case entry ip is on the page-edge. */
		if (get_kernel_nofault(instr, (u32 *) fentry_ip - 1))
			return fentry_ip;
		if (is_endbr(instr))
			fentry_ip -= ENDBR_INSN_SIZE;
		return fentry_ip;
	}
	#else
	#define get_entry_ip(fentry_ip) fentry_ip
	#endif


I'm not familiar with arm implementation, so I'm not sure if there's
a way to do this

but in any case Steven wants to keep the patch-side address:
  https://lore.kernel.org/bpf/CAEf4BzbgsLOoLKyscq6S95QeehVoAzOnQ=xmsFz8dfEUAnhObw@mail.gmail.com/T/#m19a97bbc8f8236ad869c9f8ad0cc7dbce0722d92 (local)

jirka

In short, I think it's confusing to have addresses with +0 or +4 or +8
offsets. It would be great if we can just keep it +0 at the interface
level (when attach and in available_filter_functions_addrs).

quoted

or we could just use patch-side addresses and reflect that in the file's
name like 'available_filter_functions_patch_addrs' .. it's already long
name ;-)

jirka