Re: [RFC PATCH] bpf: introduce TAINT_UNSAFE_BPF for mutating helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] add_taint() is called unconditionally in hot eBPF helper execution paths, causing severe cache-line bouncing and a logic mismatch.
- [Low] Malformed simple table syntax in reStructuredText documentation.
--

commit 212777c34ad78febee399b801b573989a50d8cca
Author: Aaron Tomlin <atomlin@atomlin.com>

bpf: introduce TAINT_UNSAFE_BPF for mutating helpers

This commit introduces a new kernel taint flag, TAINT_UNSAFE_BPF ("V"),
which indicates that an eBPF program utilizing unsafe or mutating
helpers has been loaded.

> diff --git a/Documentation/admin-guide/tainted-kernels.rst b/Documentation/admin-guide/tainted-kernels.rst
> index 9ead927a37c0..630f24996e7b 100644
> --- a/Documentation/admin-guide/tainted-kernels.rst
> +++ b/Documentation/admin-guide/tainted-kernels.rst
> @@ -79,30 +79,31 @@ which bits are set::
>  Table for decoding tainted state
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
> -===  ===  ======  ========================================================
> -Bit  Log  Number  Reason that got the kernel tainted
> -===  ===  ======  ========================================================
[ ... ]
> +===  ===   ======  ========================================================
> +Bit  Log   Number  Reason that got the kernel tainted
> +===  ===   ======  ========================================================
[ ... ]
> + 19  _/J   524288  userspace used a mutating debug operation in fwctl
> + 20  _/V  1048576  an unsafe eBPF program (mutating helper) was loaded
> +===  ===  =======  ========================================================

This isn't a bug, but the bottom border has 7 characters (=======) for the
Number column while the top and header borders have 6 (======). Will this
cause a Sphinx 'Malformed table' warning when rendering the documentation?

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index af7079aa0f36..4e7e5bf76dcb 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -155,6 +155,7 @@ unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
>  #ifdef CONFIG_BPF_KPROBE_OVERRIDE
>  BPF_CALL_2(bpf_override_return, struct pt_regs *, regs, unsigned long, rc)
>  {
> +	add_taint(TAINT_UNSAFE_BPF, LOCKDEP_STILL_OK);
>  	regs_set_return_value(regs, rc);
>  	override_function_with_return(regs);

Since bpf_override_return is a helper invoked at runtime and can execute
frequently on hot kprobes, will calling add_taint() here cause severe cache
line bouncing?

Looking at add_taint(), it unconditionally calls set_bit() on the global
tainted_mask:

kernel/panic.c:add_taint() {
    ...
    set_bit(flag, &tainted_mask);
    ...
}

Also, the commit message and documentation mention the system is tainted when
an unsafe eBPF program "was loaded". By applying the taint here during
execution instead of at program load time (e.g., in the verifier), could this
allow an unsafe BPF program to remain dormant and avoid detection until a
specific trigger occurs?

[ ... ]

> @@ -344,6 +345,8 @@ BPF_CALL_3(bpf_probe_write_user, void __user *, unsafe_ptr, const void *, src,
>  	if (unlikely(!nmi_uaccess_okay()))
>  		return -EPERM;
>  
> +	add_taint(TAINT_UNSAFE_BPF, LOCKDEP_STILL_OK);
> +
>  	return copy_to_user_nofault(unsafe_ptr, src, size);
>  }

Similarly here, will invoking add_taint() on every execution of
bpf_probe_write_user() cause significant performance degradation when
executed concurrently across multiple CPUs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260503035220.520479-1-atomlin@atomlin.com?part=1