lkml.org 
[lkml]   [2022]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid adding weak function

* Steven Rostedt <rostedt@goodmis.org> wrote:

> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> 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@kernel.org/
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

LGTM.

I suppose you'd like to merge this via the tracing tree? If so:

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

Ingo

\
 
 \ /
  Last update: 2022-05-26 04:51    [W:0.106 / U:0.592 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site