Re: [PATCH bpf-next v1 1/2] bpf: Allow bpf_res_spin_lock() in all contexts

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] bpf_res_spin_lock() remains disabled for socket filter programs, contradicting the commit title 'Allow bpf_res_spin_lock() in all contexts'.

Pre-existing issues:
- [High] When parsing BTF fields, the check for a valid `bpf_res_spin_lock` offset incorrectly checks `rec->spin_lock_off` instead of `rec->res_spin_lock_off`.
--

--- Patch [1]: [PATCH bpf-next v1 1/2] bpf: Allow bpf_res_spin_lock() in all contexts ---
commit 88a844e18ff8ff22c90389d929ce1c63353b73fe
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Allow bpf_res_spin_lock() in all contexts

This commit removes the restriction that prevents tracing programs from
using bpf_res_spin_lock(). The stated goal is to allow its use in all
contexts.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 4446f0bde88b5..bb6e16668d0d1 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -17815,7 +17815,9 @@ static int check_map_prog_compatibility(struct bpf_verifier_env *env,
>  			verbose(env, "socket filter progs cannot use bpf_spin_lock yet\n");
>  			return -EINVAL;
>  		}
> +	}

[Severity: Medium]
The commit message states the goal is to allow bpf_res_spin_lock() in all
contexts. However, just above this block in check_map_prog_compatibility(),
there is a check that still blocks it:

	if (btf_record_has_field(map->record, BPF_SPIN_LOCK | BPF_RES_SPIN_LOCK)) {
		if (prog_type == BPF_PROG_TYPE_SOCKET_FILTER) {

Does this contradict the commit title by leaving it disabled for socket
filter programs?

>  
> +	if (btf_record_has_field(map->record, BPF_SPIN_LOCK)) {
>  		if (is_tracing_prog_type(prog_type)) {
>  			verbose(env, "tracing progs cannot use bpf_spin_lock yet\n");
>  			return -EINVAL;


While reviewing this area, I also noticed a potential issue in another file.

kernel/bpf/btf.c:btf_parse_fields() {
	...
		case BPF_RES_SPIN_LOCK:
			WARN_ON_ONCE(rec->spin_lock_off >= 0);
			/* Cache offset for faster lookup at runtime */
			rec->res_spin_lock_off = rec->fields[i].offset;
			break;
	...
}

[Severity: High]
This is a pre-existing issue, but does this WARN_ON_ONCE() incorrectly check 
rec->spin_lock_off instead of rec->res_spin_lock_off?

If a user creates a BPF map value containing both a bpf_spin_lock and a
bpf_res_spin_lock, and BPF_SPIN_LOCK is parsed first, rec->spin_lock_off will
be non-negative. When BPF_RES_SPIN_LOCK is then processed, it appears this
would trivially trigger the warning, which could cause a kernel panic if
panic_on_warn is enabled.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719113551.1294284-1-memxor@gmail.com?part=1