* Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx> > > If an unused weak function was traced, it's call to fentry will still > exist, which gets added into the __mcount_loc table. Ftrace will use > kallsyms to retrieve the name for each location in __mcount_loc to display > it in the available_filter_functions and used to enable functions via the > name matching in set_ftrace_filter/notrace. Enabling these functions do > nothing but enable an unused call to ftrace_caller. If a traced weak > function is overridden, the symbol of the function would be used for it, > which will either created duplicate names, or if the previous function was > not traced, it would be incorrectly listed in available_filter_functions > as a function that can be traced. > > This became an issue with BPF[1] as there are tooling that enables the > direct callers via ftrace but then checks to see if the functions were > actually enabled. The case of one function that was marked notrace, but > was followed by an unused weak function that was traced. The unused > function's call to fentry was added to the __mcount_loc section, and > kallsyms retrieved the untraced function's symbol as the weak function was > overridden. Since the untraced function would not get traced, the BPF > check would detect this and fail. > > The real fix would be to fix kallsyms to not show address of weak > functions as the function before it. But that would require adding code in > the build to add function size to kallsyms so that it can know when the > function ends instead of just using the start of the next known symbol. Yeah, so I actually have a (prototype...) objtool based kallsyms implementation in the (way too large) fast-headers tree that is both faster & allows such details in principle: 431bca135cf8 kbuild/linker: Gather all the __kallsyms sections into a single table 6bc7af02e402 objtool/kallsyms: Copy the symbol name and offset to the new __kallsyms ELF section e1e85b2fab9e objtool/kallsyms: Increase section size dynamically 3528d607641b objtool/kallsyms: Use zero entry size for section 2555dd62348a objtool/kallsyms: Output variable length strings c114b71f8547 objtool/kallsyms: Add kallsyms_offsets[] table cfcfce6cb51f objtool/kallsyms: Change name to __kallsyms_strs 134160bb2de1 objtool/kallsyms: Split out process_kallsyms_symbols() 33347a4b46e0 objtool/kallsyms: Add relocations 86626e9e6603 objtool/kallsyms: Skip discarded sections 7dd9fef6fbb0 kallsyms/objtool: Print out the new symbol table on the kernel side c82d94b33a1f objtool/kallsyms: Use struct kallsyms_entry a66ee5034008 objtool/kallsyms, kallsyms/objtool: Share 'struct kallsyms_entry' definition e496159e5282 kallsyms/objtool: Process entries 47fc63ef3fa8 objtool/kallsyms: Switch to 64-bit absolute relocations c54fcc03cd64 objtool/kallsyms: Fix initial offset eac3c107aa6e kallsyms/objtool: Emit System.map-alike symbol list ebfb7e14b8ca objtool/kallsyms: Skip undefined symbols 25b69ef5666b objtool/kallsyms: Update symbol filter 3b26a82c733f objtool/kallsyms: Add the CONFIG_KALLSYMS_FAST Kconfig option & its related Kconfig switches 9350c25942f8 kallsyms/objtool: Make the kallsyms work even if the generic tables are not there 4a0a120bde05 objtool/kallsyms, x86/relocs: Detect & ignore __kallsyms_offset section relocations 87c5244f1fa8 kallsyms/objtool: Introduce linear table of symbol structures: kallsyms_syms[] 51dafdefc61f kallsyms/objtool: Split fast vs. generic functions e4123a40125f kallsyms/objtool: Sort symbols by address and deduplicate them 2d738c69965a kallsyms/objtool: Utilize the kallsyms_syms[] table in kallsyms_expand_symbol() and kallsyms_sym_address() 997ffe217a34 kallsyms/objtool: Port kallsyms_relative_base functionality to the kallsyms_syms[] offsets > In the mean time, this is a work around. Add a FTRACE_MCOUNT_MAX_OFFSET > macro that if defined, ftrace will ignore any function that has its call > to fentry/mcount that has an offset from the symbol that is greater than > FTRACE_MCOUNT_MAX_OFFSET. > > If CONFIG_HAVE_FENTRY is defined for x86, define FTRACE_MCOUNT_MAX_OFFSET > to zero, which will have ftrace ignore all locations that are not at the > start of the function. > > [1] https://lore.kernel.org/all/20220412094923.0abe90955e5db486b7bca279@xxxxxxxxxx/ > > Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx> LGTM. I suppose you'd like to merge this via the tracing tree? If so: Acked-by: Ingo Molnar <mingo@xxxxxxxxxx> Thanks, Ingo
