[PATCH bpf-next v3] bpf: log error code on trampoline unlink failure

[PATCH bpf-next v3] bpf: log error code on trampoline unlink failure

[xu.xin16]

From: xu xin <xu.xin16@zte.com.cn>

Replace silent WARN_ON_ONCE with WARN_ONCE that prints the actual error
code from bpf_trampoline_unlink_prog(). This aids debugging of race
conditions during link teardown, while keeping the warning rate limited
to avoid log flooding.

This will be very helpful for speeding up trouble-shooting of some crash
UAF due to bpf_trampoline_unlink_prog failures.

No change to unlink behavior.

Signed-off-by: xu xin <xu.xin16@zte.com.cn>
---
Change log
V2->v3: 
Add up the case in bpf_trampoline_multi_detach. And remove the unneeded 'if (err)'

v1->v2:
1) clean the subject name by remove 'syscall' suggested by Leon Hwang
https://lore.kernel.org/all/20a444b2-aeed-4af8-ba76-e994e2c14087@linux.dev/
2) Add up the missed case in kernel/bpf/trampoline.c:bpf_shim_tramp_link_release()

 kernel/bpf/syscall.c    |  8 +++++---
 kernel/bpf/trampoline.c | 12 ++++++++----
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6db306d23b47..52c57f8beb37 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3626,10 +3626,12 @@ static void bpf_tracing_link_release(struct bpf_link *link)
 {
 	struct bpf_tracing_link *tr_link =
 		container_of(link, struct bpf_tracing_link, link.link);
+	int err;

-	WARN_ON_ONCE(bpf_trampoline_unlink_prog(&tr_link->link.node,
-						tr_link->trampoline,
-						tr_link->tgt_prog));
+	err = bpf_trampoline_unlink_prog(&tr_link->link.node,
+					tr_link->trampoline,
+					tr_link->tgt_prog);
+	WARN_ONCE(err, "bpf_trampoline_unlink_prog returns error: %d\n", err);

 	bpf_trampoline_put(tr_link->trampoline);

diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 1a721fc4bef5..a34ddb97ff6b 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -997,12 +997,15 @@ static void bpf_shim_tramp_link_release(struct bpf_link *link)
 {
 	struct bpf_shim_tramp_link *shim_link =
 		container_of(link, struct bpf_shim_tramp_link, link.link);
+	int err;

 	/* paired with 'shim_link->trampoline = tr' in bpf_trampoline_link_cgroup_shim */
 	if (!shim_link->trampoline)
 		return;

-	WARN_ON_ONCE(bpf_trampoline_unlink_prog(&shim_link->link.node, shim_link->trampoline, NULL));
+	err = bpf_trampoline_unlink_prog(&shim_link->link.node, shim_link->trampoline, NULL);
+	WARN_ONCE(err, "bpf_trampoline_unlink_prog returns error: %d\n", err);
+
 	bpf_trampoline_put(shim_link->trampoline);
 }

@@ -1712,15 +1715,16 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
 {
 	struct bpf_tracing_multi_data *data = &link->data;
 	struct bpf_tracing_multi_node *mnode;
-	int i;
+	int i, err;

 	trampoline_lock_all();

 	for_each_mnode(mnode, link) {
 		data->entry = &mnode->entry;
 		bpf_trampoline_multi_attach_init(mnode->trampoline);
-		WARN_ON_ONCE(__bpf_trampoline_unlink_prog(&mnode->node, mnode->trampoline,
-					NULL, &trampoline_multi_ops, data));
+		err = __bpf_trampoline_unlink_prog(&mnode->node, mnode->trampoline,NULL,
+					&trampoline_multi_ops, data);
+		WARN_ONCE(err, "__bpf_trampoline_unlink_prog returns error: %d\n", err);
 	}

 	if (ftrace_hash_count(data->unreg))
-- 
2.25.1

Re: [PATCH bpf-next v3] bpf: log error code on trampoline unlink failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] If `bpf_trampoline_unlink_prog` fails to update the physical trampoline image, the detach path ignores the error and proceeds to free the BPF program, resulting in a Use-After-Free (UAF) when the active trampoline continues to call the freed memory.
--

--- Patch [1]: [PATCH bpf-next v3] bpf: log error code on trampoline unlink failure ---
commit 0b0618c3d340947a280eadeb940ccfebcad04f6f
Author: xu xin <xu.xin16@zte.com.cn>

bpf: log error code on trampoline unlink failure

This commit replaces silent WARN_ON_ONCE with WARN_ONCE to log the actual
error code from bpf_trampoline_unlink_prog() during link teardown.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 0ff9e3aa293d5..b4b4fc49bba79 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3547,10 +3547,12 @@ static void bpf_tracing_link_release(struct bpf_link *link)
>  {
>  	struct bpf_tracing_link *tr_link =
>  		container_of(link, struct bpf_tracing_link, link.link);
> +	int err;
>  
> -	WARN_ON_ONCE(bpf_trampoline_unlink_prog(&tr_link->link.node,
> -						tr_link->trampoline,
> -						tr_link->tgt_prog));
> +	err = bpf_trampoline_unlink_prog(&tr_link->link.node,
> +					tr_link->trampoline,
> +					tr_link->tgt_prog);
> +	WARN_ONCE(err, "bpf_trampoline_unlink_prog returns error: %d\n", err);
>  
>  	bpf_trampoline_put(tr_link->trampoline);

[Severity: Critical]
This isn't a bug introduced by this patch, but since the commit message
mentions troubleshooting UAF crashes here, does this error path directly cause
that UAF?

If bpf_trampoline_update() fails inside __bpf_trampoline_unlink_prog(), the
program is removed from the software list, but the physical trampoline is not
updated:

__bpf_trampoline_unlink_prog() {
    ...
    bpf_trampoline_remove_prog(tr, node);
    return bpf_trampoline_update(tr, true /* lock_direct_mutex */, ops, data);
}

Because bpf_tracing_link_release() merely logs the error and continues, it
will proceed to tear down the link and free the BPF program memory.

Will the unmodified physical trampoline then blindly jump into the now-freed
BPF program on the next kernel function invocation?

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 129d07db117ec..9b9eea02dd58d 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -1004,12 +1004,15 @@ static void bpf_shim_tramp_link_release(struct bpf_link *link)
>  {
>  	struct bpf_shim_tramp_link *shim_link =
>  		container_of(link, struct bpf_shim_tramp_link, link.link);
> +	int err;
>  
>  	/* paired with 'shim_link->trampoline = tr' in bpf_trampoline_link_cgroup_shim */
>  	if (!shim_link->trampoline)
>  		return;
>  
> -	WARN_ON_ONCE(bpf_trampoline_unlink_prog(&shim_link->link.node, shim_link->trampoline, NULL));
> +	err = bpf_trampoline_unlink_prog(&shim_link->link.node, shim_link->trampoline, NULL);
> +	WARN_ONCE(err, "bpf_trampoline_unlink_prog returns error: %d\n", err);
> +
>  	bpf_trampoline_put(shim_link->trampoline);
>  }

[Severity: Critical]
This is a pre-existing issue, but this shim release callback appears to suffer
from the exact same flaw.

If the unlink fails, it ignores the error and continues, leaving the hardware
hook active for a program that is about to be freed.

> @@ -1720,15 +1723,16 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
>  {
>  	struct bpf_tracing_multi_data *data = &link->data;
>  	struct bpf_tracing_multi_node *mnode;
> -	int i;
> +	int i, err;
>  
>  	trampoline_lock_all();
>  
>  	for_each_mnode(mnode, link) {
>  		data->entry = &mnode->entry;
>  		bpf_trampoline_multi_attach_init(mnode->trampoline);
> -		WARN_ON_ONCE(__bpf_trampoline_unlink_prog(&mnode->node, mnode->trampoline,
> -					NULL, &trampoline_multi_ops, data));
> +		err = __bpf_trampoline_unlink_prog(&mnode->node, mnode->trampoline,NULL,
> +					&trampoline_multi_ops, data);
> +		WARN_ONCE(err, "__bpf_trampoline_unlink_prog returns error: %d\n", err);
>  	}

[Severity: Critical]
This is also a pre-existing issue.

Similar to the other paths, this loop ignores unlink failures. Can this allow
all attached programs to be freed even if their physical hooks remain active?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728142621001Ee2lrcnZpeNuxtxRES69Z@zte.com.cn?part=1

Re: [PATCH bpf-next v3] bpf: log error code on trampoline unlink failure

[Leon Hwang]

On 28/7/26 14:26, xu.xin16@zte.com.cn wrote:
> From: xu xin <xu.xin16@zte.com.cn>
> 
> Replace silent WARN_ON_ONCE with WARN_ONCE that prints the actual error
> code from bpf_trampoline_unlink_prog(). This aids debugging of race
> conditions during link teardown, while keeping the warning rate limited
> to avoid log flooding.
> 
> This will be very helpful for speeding up trouble-shooting of some crash
> UAF due to bpf_trampoline_unlink_prog failures.
> 
> No change to unlink behavior.
> 
> Signed-off-by: xu xin <xu.xin16@zte.com.cn>


Better to use Xu Xin as your formal name.

> ---
> Change log
> V2->v3: 
> Add up the case in bpf_trampoline_multi_detach. And remove the unneeded 'if (err)'
> 
> v1->v2:
> 1) clean the subject name by remove 'syscall' suggested by Leon Hwang
> https://lore.kernel.org/all/20a444b2-aeed-4af8-ba76-e994e2c14087@linux.dev/
> 2) Add up the missed case in kernel/bpf/trampoline.c:bpf_shim_tramp_link_release()
> 
>  kernel/bpf/syscall.c    |  8 +++++---
>  kernel/bpf/trampoline.c | 12 ++++++++----
>  2 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6db306d23b47..52c57f8beb37 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3626,10 +3626,12 @@ static void bpf_tracing_link_release(struct bpf_link *link)
>  {
>  	struct bpf_tracing_link *tr_link =
>  		container_of(link, struct bpf_tracing_link, link.link);
> +	int err;
> 
> -	WARN_ON_ONCE(bpf_trampoline_unlink_prog(&tr_link->link.node,
> -						tr_link->trampoline,
> -						tr_link->tgt_prog));
> +	err = bpf_trampoline_unlink_prog(&tr_link->link.node,
> +					tr_link->trampoline,
> +					tr_link->tgt_prog);
> +	WARN_ONCE(err, "bpf_trampoline_unlink_prog returns error: %d\n", err);
> 
>  	bpf_trampoline_put(tr_link->trampoline);
> 
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 1a721fc4bef5..a34ddb97ff6b 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -997,12 +997,15 @@ static void bpf_shim_tramp_link_release(struct bpf_link *link)
>  {
>  	struct bpf_shim_tramp_link *shim_link =
>  		container_of(link, struct bpf_shim_tramp_link, link.link);
> +	int err;
> 
>  	/* paired with 'shim_link->trampoline = tr' in bpf_trampoline_link_cgroup_shim */
>  	if (!shim_link->trampoline)
>  		return;
> 
> -	WARN_ON_ONCE(bpf_trampoline_unlink_prog(&shim_link->link.node, shim_link->trampoline, NULL));
> +	err = bpf_trampoline_unlink_prog(&shim_link->link.node, shim_link->trampoline, NULL);
> +	WARN_ONCE(err, "bpf_trampoline_unlink_prog returns error: %d\n", err);
> +
>  	bpf_trampoline_put(shim_link->trampoline);
>  }
> 
> @@ -1712,15 +1715,16 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
>  {
>  	struct bpf_tracing_multi_data *data = &link->data;
>  	struct bpf_tracing_multi_node *mnode;
> -	int i;
> +	int i, err;
> 
>  	trampoline_lock_all();
> 
>  	for_each_mnode(mnode, link) {
>  		data->entry = &mnode->entry;
>  		bpf_trampoline_multi_attach_init(mnode->trampoline);
> -		WARN_ON_ONCE(__bpf_trampoline_unlink_prog(&mnode->node, mnode->trampoline,
> -					NULL, &trampoline_multi_ops, data));
> +		err = __bpf_trampoline_unlink_prog(&mnode->node, mnode->trampoline,NULL,


NIT: a space is missing at the position between ',' and 'NULL'.

> +					&trampoline_multi_ops, data);

NIT: align '&' with previous line's '&'.

Other than that,

Acked-by: Leon Hwang <leon.hwang@linux.dev>

> +		WARN_ONCE(err, "__bpf_trampoline_unlink_prog returns error: %d\n", err);
>  	}
> 
>  	if (ftrace_hash_count(data->unreg))