Re: [PATCHv5 bpf-next 1/9] ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag
From: bot+bpf-ci@kernel.org
Date: 2025-12-15 21:31:51
Also in:
bpf, linux-arm-kernel, lkml
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
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():
direct = ftrace_find_rec_direct(rec->ip);
if (direct)
seq_printf(m, "\n\tdirect-->%pS", (void *)direct);
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));
[ ... ]
quoted hunk ↗ jump to hunk
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++) {
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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