Re: [PATCH bpf v4 5/5] bpf, sockmap: Take state lock for af_unix iter

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Incomplete fix for Use-After-Free (UAF) in unix_stream_bpf_update_proto().
- [High] Spinlock held during BPF program execution leads to context violations and deadlock risks.
--

commit 059f81e7a85c481f9c7f6dd1d0ab36e935134ac8
Author: Michal Luczaj <mhal@rbox.co>

bpf, sockmap: Take state lock for af_unix iter

This commit attempts to fix a use-after-free race condition in
unix_stream_bpf_update_proto() by acquiring the unix_state_lock() inside
the bpf_iter_unix_seq_show() callback.

> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 590a30d3b2f70..15b48cc6e9b0c 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -3737,6 +3737,7 @@ static int bpf_iter_unix_seq_show(struct seq_file *seq, void *v)
>  		return 0;
>  
>  	lock_sock(sk);
> +	unix_state_lock(sk);

[ ... ]

> @@ -3748,6 +3749,7 @@ static int bpf_iter_unix_seq_show(struct seq_file *seq, void *v)
>  	prog = bpf_iter_get_info(&meta, false);
>  	ret = unix_prog_seq_show(prog, &meta, v, uid);

Does holding a spinlock during bpf program execution introduce
sleep-in-atomic or lock inversion issues?

If the bpf iterator program is loaded with BPF_F_SLEEPABLE,
bpf_iter_run_prog() explicitly calls might_fault(). Calling might_fault()
while holding a spinlock (which disables preemption) triggers a sleep in
atomic context bug.

Also, could this introduce a lock inversion risk? The bpf programs can
interact with maps or other sockets, acquiring locks like map->lock or
bh_lock_sock. Taking unix_state_lock before these locks might create an
unnatural lock hierarchy, potentially leading to deadlocks if other kernel
paths acquire these locks in the standard order.

Additionally, does this fully resolve the use-after-free in
unix_stream_bpf_update_proto()?

Since unix_stream_bpf_update_proto() is the underlying callback used
whenever an af_unix socket is inserted into a sockmap, can it be reached
independently of this iterator?

For example, could it be reached via the bpf(BPF_MAP_UPDATE_ELEM) syscall,
or when a bpf program updates the map using a different af_unix socket than
the one being iterated? In those cases, unix_state_lock(sk) would not be
held, leaving the use-after-free unresolved.

Should the lock be acquired directly inside unix_stream_bpf_update_proto()
instead?

>  unlock:
> +	unix_state_unlock(sk);
>  	release_sock(sk);
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260414-unix-proto-update-null-ptr-deref-v4-0-2af6fe97918e@rbox.co?part=5