[PATCH bpf-next] bpf:syscall: Log error code on trampoline unlink failure

[PATCH bpf-next] bpf:syscall: 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.

Signed-off-by: xu xin <xu.xin16@zte.com.cn>
---
 kernel/bpf/syscall.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6db306d23b47..2348dc33abf4 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3626,10 +3626,13 @@ 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);
+	if (err)
+		WARN_ONCE(err, "bpf_trampoline_unlink_prog returns error: %d\n", err);

 	bpf_trampoline_put(tr_link->trampoline);

-- 
2.25.1

Re: [PATCH bpf-next] bpf:syscall: Log error code on trampoline unlink failure

[sashiko-bot]

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

New issues:
- [Medium] Incomplete implementation of error logging for `bpf_trampoline_unlink_prog` failures.

Pre-existing issues:
- [Critical] Ignoring the error from bpf_trampoline_unlink_prog() leads to a severe Use-After-Free (UAF) of the BPF program during link teardown.
--

--- Patch [1]: [PATCH bpf-next] bpf:syscall: Log error code on trampoline unlink failure ---
commit 14d6ea130ad662c6d03b816504502f1ebc3164e4
Author: xu xin <xu.xin16@zte.com.cn>

bpf:syscall: Log error code on trampoline unlink failure
    
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.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 0ff9e3aa293d5..9cbe00bc8d051 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3547,10 +3547,13 @@ 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);
> +	if (err)
> +		WARN_ONCE(err, "bpf_trampoline_unlink_prog returns error: %d\n", err);

[Severity: Critical]
This is a pre-existing issue, but does ignoring the error from
bpf_trampoline_unlink_prog() lead to a Use-After-Free of the BPF program?

If bpf_trampoline_update() fails inside __bpf_trampoline_unlink_prog()
(e.g., due to -ENOMEM or -EBUSY), the old trampoline image remains active
and hooked to the target function.

However, bpf_tracing_link_release() logs the error and proceeds with
teardown, eventually calling bpf_prog_put() on tr_link->tgt_prog. After an
RCU grace period, the program memory is freed.

Subsequent calls to the target function will execute the old trampoline
image, which will jump into the freed BPF program memory. Is there a way
to safely rollback or defer the teardown when this fails?

[Severity: Medium]
The commit message states:
"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"

Could this change be incomplete? Looking at bpf_shim_tramp_link_release()
and bpf_trampoline_multi_detach() in kernel/bpf/trampoline.c, they both
still swallow the error code:

kernel/bpf/trampoline.c:bpf_shim_tramp_link_release() {
    ...
    WARN_ON_ONCE(bpf_trampoline_unlink_prog(&shim_link->link.node,
                                            shim_link->trampoline,
                                            NULL));
    ...
}

Should these be updated as well to ensure debugging context is available
for cgroup shims and multi-attach links?

>  	bpf_trampoline_put(tr_link->trampoline);
>

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

Re: [PATCH bpf-next] bpf:syscall: Log error code on trampoline unlink failure

[Leon Hwang]

In subject, 'bpf:' is enough, 'syscall:' can be dropped.

On 16/7/26 11:34, 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.


Have you tried to trace bpf_trampoline_unlink_prog() with fexit?

bpftrace -lv 'fexit:bpf_trampoline_unlink_prog'
kretfunc:vmlinux:bpf_trampoline_unlink_prog
    struct bpf_tramp_node * node
    struct bpf_trampoline * tr
    struct bpf_prog * tgt_prog
    int retval

Since bpf_trampoline_unlink_prog() can be traced with fexit, it seems
unnecessary for this change?

Thanks,
Leon

> [...]

Re: [PATCH bpf-next] bpf:syscall: Log error code on trampoline unlink failure

[xu.xin16]

> In subject, 'bpf:' is enough, 'syscall:' can be dropped.

Ok, thanks.

> 
> On 16/7/26 11:34, 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.
> 
> 
> Have you tried to trace bpf_trampoline_unlink_prog() with fexit?
> 
> bpftrace -lv 'fexit:bpf_trampoline_unlink_prog'
> kretfunc:vmlinux:bpf_trampoline_unlink_prog
>     struct bpf_tramp_node * node
>     struct bpf_trampoline * tr
>     struct bpf_prog * tgt_prog
>     int retval
> 
> Since bpf_trampoline_unlink_prog() can be traced with fexit, it seems
> unnecessary for this change?

Well, the crash may happen when the kernel is booting, or too hard to reproduce,
in which case, bpftrace does not work.

So this change will be more helpful, I believe.

Thanks,
xu xin

Re: [PATCH bpf-next] bpf:syscall: Log error code on trampoline unlink failure

[Leon Hwang]

On 16/7/26 13:55, xu.xin16@zte.com.cn wrote:
>> In subject, 'bpf:' is enough, 'syscall:' can be dropped.
> 
> Ok, thanks.
> 
>>
>> On 16/7/26 11:34, 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.
>>
>>
>> Have you tried to trace bpf_trampoline_unlink_prog() with fexit?
>>
>> bpftrace -lv 'fexit:bpf_trampoline_unlink_prog'
>> kretfunc:vmlinux:bpf_trampoline_unlink_prog
>>     struct bpf_tramp_node * node
>>     struct bpf_trampoline * tr
>>     struct bpf_prog * tgt_prog
>>     int retval
>>
>> Since bpf_trampoline_unlink_prog() can be traced with fexit, it seems
>> unnecessary for this change?
> 
> Well, the crash may happen when the kernel is booting, or too hard to reproduce,
> in which case, bpftrace does not work.


Do you enable kernel.panic_on_warn sysctl?

If it's not enabled, the kernel won't crash, I think.

Thanks,
Leon

> 
> So this change will be more helpful, I believe.
> 
> Thanks,
> xu xin
> 

Re: [PATCH bpf-next] bpf:syscall: Log error code on trampoline unlink failure

[xu.xin16]

> > Well, the crash may happen when the kernel is booting, or too hard to reproduce,
> > in which case, bpftrace does not work.
> 
> 
> Do you enable kernel.panic_on_warn sysctl?
> 
> If it's not enabled, the kernel won't crash, I think.

I'd like to clarify that the panic in our case is not triggered by the WARN itself.
Instead, the real crash is a use‑after‑free (UAF) that happens when one CPU is
executing a trampoline while another CPU is concurrently freeing it. This results
in an illegal instruction (or page fault) at the PC, which is fatal and will panic
the kernel regardless of panic_on_warn – because it's an Oops with Fatal exception
in interrupt.

Re: [PATCH bpf-next] bpf:syscall: Log error code on trampoline unlink failure

[Leon Hwang]

On 16/7/26 14:26, xu.xin16@zte.com.cn wrote:
>>> Well, the crash may happen when the kernel is booting, or too hard to reproduce,
>>> in which case, bpftrace does not work.
>>
>>
>> Do you enable kernel.panic_on_warn sysctl?
>>
>> If it's not enabled, the kernel won't crash, I think.
> 
> I'd like to clarify that the panic in our case is not triggered by the WARN itself.
> Instead, the real crash is a use‑after‑free (UAF) that happens when one CPU is
> executing a trampoline while another CPU is concurrently freeing it. This results
> in an illegal instruction (or page fault) at the PC, which is fatal and will panic
> the kernel regardless of panic_on_warn – because it's an Oops with Fatal exception
> in interrupt.


Why not fix the root cause instead?

If you want some help from upstream to fix the root cause, pls provide
the crash dmesg and the reproducing demo.

Thanks,
Leon

Re: [PATCH bpf-next] bpf:syscall: Log error code on trampoline unlink failure

[xu.xin16]

> >>> Well, the crash may happen when the kernel is booting, or too hard to reproduce,
> >>> in which case, bpftrace does not work.
> >>
> >>
> >> Do you enable kernel.panic_on_warn sysctl?
> >>
> >> If it's not enabled, the kernel won't crash, I think.
> > 
> > I'd like to clarify that the panic in our case is not triggered by the WARN itself.
> > Instead, the real crash is a use‑after‑free (UAF) that happens when one CPU is
> > executing a trampoline while another CPU is concurrently freeing it. This results
> > in an illegal instruction (or page fault) at the PC, which is fatal and will panic
> > the kernel regardless of panic_on_warn – because it's an Oops with Fatal exception
> > in interrupt.
> 
> 
> Why not fix the root cause instead?
> 
> If you want some help from upstream to fix the root cause, pls provide
> the crash dmesg and the reproducing demo.

It's already fixed due to some OOT drivers.

I think if we have this patch, it can speed up trouble-shooting.

Re: [PATCH bpf-next] bpf:syscall: Log error code on trampoline unlink failure

[Leon Hwang]

On 16/7/26 15:03, xu.xin16@zte.com.cn wrote:
>>>>> Well, the crash may happen when the kernel is booting, or too hard to reproduce,
>>>>> in which case, bpftrace does not work.
>>>>
>>>>
>>>> Do you enable kernel.panic_on_warn sysctl?
>>>>
>>>> If it's not enabled, the kernel won't crash, I think.
>>>
>>> I'd like to clarify that the panic in our case is not triggered by the WARN itself.
>>> Instead, the real crash is a use‑after‑free (UAF) that happens when one CPU is
>>> executing a trampoline while another CPU is concurrently freeing it. This results
>>> in an illegal instruction (or page fault) at the PC, which is fatal and will panic
>>> the kernel regardless of panic_on_warn – because it's an Oops with Fatal exception
>>> in interrupt.
>>
>>
>> Why not fix the root cause instead?
>>
>> If you want some help from upstream to fix the root cause, pls provide
>> the crash dmesg and the reproducing demo.
> 
> It's already fixed due to some OOT drivers.
> 
> I think if we have this patch, it can speed up trouble-shooting.

OK. Let's wait for Jiri's opinion.

Thanks,
Leon