linux-kernel - Re: [PATCHv5 bpf-next 1/9] ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <95c13c0b793604796037ca67c191e1bfce22770dd020107c7179a698c5d49b91@mail.kernel.org>
Date: Mon, 15 Dec 2025 21:31:51 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: jolsa@...nel.org,rostedt@...nel.org,revest@...gle.com,mark.rutland@....com
Cc: bpf@...r.kernel.org,linux-kernel@...r.kernel.org,linux-trace-kernel@...r.kernel.org,linux-arm-kernel@...ts.infradead.org,ast@...nel.org,daniel@...earbox.net,andrii@...nel.org,menglong8.dong@...il.com,song@...nel.org,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [PATCHv5 bpf-next 1/9] ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag

> 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():

    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));

[ ... ]

> 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