kern/58596: bpf(4) MP-safety issues

[campbell+netbsd%mumble.net@localhost]

>Number:         58596
>Category:       kern
>Synopsis:       bpf(4) MP-safety issues
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Aug 14 13:15:00 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, ...
>Organization:
The NeFoundatBPF tion
>Environment:
>Description:
Correctness issues:

1. access to bif_mbuf_head/tail is not serialized by a lock, and it's not clear the caller is always guaranteed to run on the same CPU
2. concurrent bpf_read on the same instance is not serialized, which might lead to uiomove on d->bd_hbuf in one thread while another thread is rotating the buffer
3. selnotify is done outside the lock
4. fownsignal is done in softint context (_bpf_mtap/_bpf_mptap2 -> bpf_deliver -> catchpacket [under pserialize] -> bpf_wakeup -> fownsignal) but takes the adaptive lock proc_lock which is probably forbidden in softint and certainly forbidden during pserialize (comment in bpf_deliver: /* Assume catchpacket doesn't sleep */)
5. updates to bd_pid are not consistently serialized by a lock (also bd_pid concept seems incoherent and appears to be only used for stat, why bother?)
6. bpf_detachd does pserialize_perform under a spin lock (bd_buf_mtx), which is forbidden
7. callout_halt in bpf_ioctl and bpf_read doesn't prevent concurrent select/poll from starting up timer again (also unclear why bpf_ioctl or bpf_read should cancel a concurrent select/poll)

Performance issues:

8. bpf_write shouldn't need to take bd_mtx; should do it with pserialize
>How-To-Repeat:
1. code inspection
2. write a program that opens /dev/bpf and reads from the same fd in two threads at once
>Fix:
Yes, please!