On Mon, Dec 15, 2025 at 09:31:51PM +0000, [email protected] 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


Reply via email to