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

[PATCH bpf-next v4] 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>
Acked-by: Leon Hwang <leon.hwang@linux.dev>
---
Change log:
- Small fixes according to the v3 disccusion:
  https://lore.kernel.org/all/82926caa-e16a-457a-8d75-61dfc8dac102@linux.dev/

 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..49dceea3d51e 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 v4] 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] Preexisting Use-After-Free vulnerability when bpf_trampoline_unlink_prog fails. This problem wasn't introduced by this patch, but the callers unconditionally drop BPF program references even when detachment fails.
--

--- Patch [1]: [PATCH bpf-next v4] bpf: log error code on trampoline unlink failure  ---
commit 04adb71914e050f3eeb5224ff162eb88dcf506a9
Author: Xu Xin <xu.xin16@zte.com.cn>

bpf: log error code on trampoline unlink failure 

This commit replaces silent WARN_ON_ONCE macros with WARN_ONCE to print the
actual error code from bpf_trampoline_unlink_prog(). This is intended to
help debug race conditions and use-after-free crashes 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 is a pre-existing issue, but does dropping the program reference here
lead to a use-after-free if the unlink actually fails?

If bpf_trampoline_update() fails during the teardown process, the active
trampoline image retains the call to the BPF program:

kernel/bpf/trampoline.c:__bpf_trampoline_unlink_prog() {
    ...
    bpf_trampoline_remove_prog(tr, node);
    return bpf_trampoline_update(tr, true /* lock_direct_mutex */, ops, data);
}

When this error bubbles up to bpf_tracing_link_release(), the active image
is left intact and continues to jump to the BPF program. However, we proceed
to unconditionally call bpf_prog_put() shortly after the trampoline put:

kernel/bpf/syscall.c:bpf_tracing_link_release() {
    ...
    err = bpf_trampoline_unlink_prog(...);
    WARN_ONCE(err, ...);
    
    bpf_trampoline_put(tr_link->trampoline);
    
    /* tgt_prog is NULL if target is a kernel function */
    if (tr_link->tgt_prog)
        bpf_prog_put(tr_link->tgt_prog);
}

Once the RCU grace period elapses, the BPF program's memory will be freed.
Could the active trampoline then jump into freed memory the next time the
targeted kernel function executes, resulting in the UAF crash mentioned in
the commit message?

[ ... ]

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