Re: [PATCH 14/14] scripts/sorttable: ftrace: Do not add weak functions to available_filter_functions

[Peter Zijlstra <peterz@infradead.org>]

On Fri, Jan 03, 2025 at 12:10:08PM +0100, Jiri Olsa wrote:
> On Thu, Jan 02, 2025 at 01:58:59PM -0500, Steven Rostedt wrote:
> > From: Steven Rostedt <rostedt@goodmis.org>
> > 
> > When a function is annotated as "weak" and is overridden, the code is not
> > removed. If it is traced, the fentry/mcount location in the weak function
> > will be referenced by the "__mcount_loc" section. This will then be added
> > to the available_filter_functions list. Since only the address of the
> > functions are listed, to find the name to show, a search of kallsyms is
> > used.
> > 
> > Since kallsyms will return the function by simply finding the function
> > that the address is after but before the next function, an address of a
> > weak function will show up as the function before it. This is because
> > kallsyms does not save names of weak functions. This has caused issues in
> > the past, as now the traced weak function will be listed in
> > available_filter_functions with the name of the function before it.
> > 
> > At best, this will cause the previous function's name to be listed twice.
> > At worse, if the previous function was marked notrace, it will now show up
> > as a function that can be traced. Note that it only shows up that it can
> > be traced but will not be if enabled, which causes confusion.
> > 
> >  https://lore.kernel.org/all/20220412094923.0abe90955e5db486b7bca279@kernel.org/
> > 
> > The commit b39181f7c6907 ("ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid
> > adding weak function") was a workaround to this by checking the function
> > address before printing its name. If the address was too far from the
> > function given by the name then instead of printing the name it would
> > print: __ftrace_invalid_address___<invalid-offset>
> > 
> > The real issue is that these invalid addresses are listed in the ftrace
> > table look up which available_filter_functions is derived from. A place
> > holder must be listed in that file because set_ftrace_filter may take a
> > series of indexes into that file instead of names to be able to do O(1)
> > lookups to enable filtering (many tools use this method).
> > 
> > Even if kallsyms saved the size of the function, it does not remove the
> > need of having these place holders. The real solution is to not add a weak
> > function into the ftrace table in the first place.
> > 
> > To solve this, the sorttable.c code that sorts the mcount regions during
> > the build is modified to take a "nm -S vmlinux" input, sort it, and any
> > function listed in the mcount_loc section that is not within a boundary of
> > the function list given by nm is considered a weak function and is zeroed
> > out. Note, this does not mean they will remain zero when booting as KASLR
> > will still shift those addresses.
> 
> hi,
> fyi this seems to remove several functions from available_filter_functions,
> that bpf relay on.. like update_socket_protocol or bpf_rstat_flush:
> 
> 	__bpf_hook_start();
> 
> 	__weak noinline int update_socket_protocol(int family, int type, int protocol)
> 	{
> 		return protocol;
> 	}
> 
> 	__bpf_hook_end();
> 
> 
> 	[root@qemu-1 tracing]# cat available_filter_functions | grep update_socket_protocol
> 	[root@qemu-1 tracing]# cat /proc/kallsyms | grep update_socket_protocol
> 	ffffffff821d58b0 W __pfx_update_socket_protocol
> 	ffffffff821d58c0 W update_socket_protocol
> 
> not sure why that fits the condition above for removal

Check your build, if update_socket_protocol() is no longer in the symbol
table for your vmlinux.o then the linker deleted the symbol and things
work as advertised.

If its still there, these patches have a wobbly.