| Message ID | 20250415112750.1477339-1-guoj17@chinatelecom.cn (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | ftrace: make ftrace_location() more accurate | expand |
On Tue, 15 Apr 2025 19:27:50 +0800 Jin Guo <menglong8.dong@gmail.com> wrote: > The function ftrace_location is used to lookup the ftrace location with an > ip. However, the result that it returns can be wrong in some case. > > Let's image that we have the following kallsyms: > > ffffffff812c35f0 T sys_ni_syscall > ffffffff812c38b0 W __pfx___x64_sys_io_pgetevents_time32 > ffffffff812c38c0 W __x64_sys_io_pgetevents_time32 Have you tried the latest kernel? because on x86, weak functions that are not used should no longer be in the ftrace table. That is, you should never see __ftrace_invalid_addres_* anymore. See https://lore.kernel.org/all/20250218195918.255228630@goodmis.org/ That was just added this merge window, which would make your patch obsolete. -- Steve
On Tue, Apr 15, 2025 at 7:45 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Tue, 15 Apr 2025 19:27:50 +0800 > Jin Guo <menglong8.dong@gmail.com> wrote: > > > The function ftrace_location is used to lookup the ftrace location with an > > ip. However, the result that it returns can be wrong in some case. > > > > Let's image that we have the following kallsyms: > > > > ffffffff812c35f0 T sys_ni_syscall > > ffffffff812c38b0 W __pfx___x64_sys_io_pgetevents_time32 > > ffffffff812c38c0 W __x64_sys_io_pgetevents_time32 > > Have you tried the latest kernel? because on x86, weak functions that are > not used should no longer be in the ftrace table. That is, you should never > see __ftrace_invalid_addres_* anymore. > > See https://lore.kernel.org/all/20250218195918.255228630@goodmis.org/ > > That was just added this merge window, which would make your patch obsolete. Hi, Steve you are right, I haven't updated my code in over a month, and the latest kernel works fine. > > -- Steve
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 1a48aedb5255..8c9a4009c997 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -1658,8 +1658,13 @@ unsigned long ftrace_location(unsigned long ip) return 0; /* map sym+0 to __fentry__ */ - if (!offset) - loc = ftrace_location_range(ip, ip + size - 1); + if (!offset) { + size -= 1; +#ifdef FTRACE_MCOUNT_MAX_OFFSET + size = min(size, FTRACE_MCOUNT_MAX_OFFSET); +#endif + loc = ftrace_location_range(ip, ip + size); + } } return loc; }
The function ftrace_location is used to lookup the ftrace location with an ip. However, the result that it returns can be wrong in some case. Let's image that we have the following kallsyms: ffffffff812c35f0 T sys_ni_syscall ffffffff812c38b0 W __pfx___x64_sys_io_pgetevents_time32 ffffffff812c38c0 W __x64_sys_io_pgetevents_time32 And the symbol has corresponding ftrace location like this: ffffffff812c35f4 sys_ni_syscall ffffffff812c3624 __ftrace_invalid_address___52 ffffffff812c3654 __ftrace_invalid_address___100 ffffffff812c3684 __ftrace_invalid_address___148 ffffffff812c36b4 __ftrace_invalid_address___196 ffffffff812c36e4 __ftrace_invalid_address___244 ffffffff812c3714 __ftrace_invalid_address___292 ffffffff812c3744 __ftrace_invalid_address___340 ffffffff812c3774 __ftrace_invalid_address___388 ffffffff812c37a4 __ftrace_invalid_address___436 ffffffff812c37d4 __ftrace_invalid_address___484 ffffffff812c3804 __ftrace_invalid_address___532 ffffffff812c3834 __ftrace_invalid_address___580 ffffffff812c3864 __ftrace_invalid_address___628 ffffffff812c3894 __ftrace_invalid_address___676 ffffffff812c38c4 __x64_sys_io_pgetevents_time32 ffffffff812c38f4 __ia32_sys_io_pgetevents_time32 When we want to lookup the ftrace location for sys_ni_syscall, the ftrace location "ffffffff812c3894" can be returned, which is totally wrong. The reason is that we get a wrong function size from kallsyms_lookup_size_offset(), because of the existing of weak functions. And when we lookup with ftrace_location_range(), it will return the first matched location between 0xffffffff812c35f0 and ffffffff812c38c0, as it searches by binary on a page. When the macro "FTRACE_MCOUNT_MAX_OFFSET" exists, we can limit the lookup size no more than FTRACE_MCOUNT_MAX_OFFSET to avoid such error. The victim of this problem can be BPF_PROG_TYPE_TRACING. When we want to hook the function sys_ni_syscall with BPF_FENTRY, it will attach to __ftrace_invalid_address___676(0xffffffff812c3894) successfully instead, and the user totally can't aware it, which is dangerous. Signed-off-by: Jin Guo <guoj17@chinatelecom.cn> --- kernel/trace/ftrace.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)