Re: [PATCHv5 bpf-next 1/9] ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag

Jiri Olsa
2025-12-17T08:40:20Z

On Mon, Dec 15, 2025 at 09:31:51PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > index 015dd1049bea..505b7d3f5641 100644
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
>
> [ ... ]
>
> > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > index 976d89011b15..b9a358d7a78f 100644
> > --- a/kernel/bpf/trampoline.c
> > +++ b/kernel/bpf/trampoline.c
> > @@ -214,10 +214,15 @@ static int modify_fentry(struct bpf_trampoline *tr, u32 orig_flags,
> > int ret;
> >
> > if (tr->func.ftrace_managed) {
> > + unsigned long addr = (unsigned long) new_addr;
> > +
> > + if (bpf_trampoline_use_jmp(tr->flags))
> > + addr = ftrace_jmp_set(addr);
> > +
> > if (lock_direct_mutex)
> > - ret = modify_ftrace_direct(tr->fops, (long)new_addr);
> > + ret = modify_ftrace_direct(tr->fops, addr);
> > else
> > - ret = modify_ftrace_direct_nolock(tr->fops, (long)new_addr);
> > + ret = modify_ftrace_direct_nolock(tr->fops, addr);
> > } else {
> > ret = bpf_trampoline_update_fentry(tr, orig_flags, old_addr,
> > new_addr);
> > @@ -240,10 +245,15 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
> > }
> >
> > if (tr->func.ftrace_managed) {
> > + unsigned long addr = (unsigned long) new_addr;
> > +
> > + if (bpf_trampoline_use_jmp(tr->flags))
> > + addr = ftrace_jmp_set(addr);
> > +
> > ret = ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1);
> > if (ret)
> > return ret;
> > - ret = register_ftrace_direct(tr->fops, (long)new_addr);
> > + ret = register_ftrace_direct(tr->fops, addr);
> > } else {
> > ret = bpf_trampoline_update_fentry(tr, 0, NULL, new_addr);
> > }
>
> After this change, register_ftrace_direct() receives an address with
> the LSB bit already set (via ftrace_jmp_set) and stores it as-is in
> entry->direct. This address then flows through to ftrace_find_rec_direct()
> which returns entry->direct unchanged.
>
> In available_filter_functions_show():

hum, there's no such function.. AFAICS it's t_show called through
ftrace_avail_fops->ftrace_avail_open->show_ftrace_seq_ops

>
> direct = ftrace_find_rec_direct(rec->ip);
> if (direct)
> seq_printf(m, "\n\tdirect-->%pS", (void *)direct);
>

but yep, it's related problem, that Menglong said he will address in
separate patch

thanks,
jirka

> Can the debug output show misleading symbol offsets? When the LSB is
> set, %pS will resolve addresses like "function+0x1" instead of just
> "function". This makes ftrace debug output harder to read for users
> trying to understand direct call attachments.
>
> The fix would be to strip the LSB bit before printing:
> seq_printf(m, "\n\tdirect-->%pS", (void *)ftrace_jmp_get(direct));
>
> [ ... ]
>
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index bbb37c0f8c6c..b0dc911411f1 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -6017,15 +6017,8 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
> > if (ftrace_hash_empty(hash))
> > return -EINVAL;
> >
> > - /* This is a "raw" address, and this should never happen. */
> > - if (WARN_ON_ONCE(ftrace_is_jmp(addr)))
> > - return -EINVAL;
> > -
> > mutex_lock(&direct_mutex);
> >
> > - if (ops->flags & FTRACE_OPS_FL_JMP)
> > - addr = ftrace_jmp_set(addr);
> > -
> > /* Make sure requested entries are not already registered.. */
> > size = 1 << hash->size_bits;
> > for (i = 0; i < size; i++) {
>
> [ ... ]
>
> > @@ -6146,13 +6139,6 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
> >
> > lockdep_assert_held_once(&direct_mutex);
> >
> > - /* This is a "raw" address, and this should never happen. */
> > - if (WARN_ON_ONCE(ftrace_is_jmp(addr)))
> > - return -EINVAL;
> > -
> > - if (ops->flags & FTRACE_OPS_FL_JMP)
> > - addr = ftrace_jmp_set(addr);
> > -
> > /* Enable the tmp_ops to have the same functions as the direct ops */
> > ftrace_ops_init(&tmp_ops);
> > tmp_ops.func_hash = ops->func_hash;
>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20247816064