On Mon, May 11, 2026 at 11:27 AM <[email protected]> wrote:
[ ... ]
> > +/**
> > + * bpf_wakeup_sources_get_head - Get the head of the wakeup sources list
> > + *
> > + * Return: The head of the wakeup sources list.
> > + */
> > +__bpf_kfunc void *bpf_wakeup_sources_get_head(void)
> > +{
> > + return &wakeup_sources;
> > +}
>
> What stops a BPF program from using bpf_wakeup_sources_get_head() to get
> the wakeup_sources pointer and iterating this list without taking the SRCU
> lock?
>
> The current API design requires BPF programs to manually call the lock/unlock
> functions, but there's no enforcement that they must do so before accessing
> the list. A program could call bpf_wakeup_sources_get_head() and directly
> iterate the list without proper SRCU protection, which could lead to race
> conditions or crashes.
>
> An open-coded iterator would solve this by internalizing the lock:
>
> Iterator new() acquires SRCU lock
> Iterator next() returns next wakeup_source
> Iterator destroy() releases SRCU lock
>
> This design pattern ensures that the list can never be traversed without
> proper locking. Puranjay Mohan raised this concern in v2, but it was not
> addressed in v3 or v4.
>
> Reference: https://lore.kernel.org/all/[email protected]/
This was already discussed; the response is at [1] and the BPF
iterator attempt is at [2]. In short, BPF iterators and their
tradeoffs were explored, but the current form was chosen as the best
option.
[1]:
https://lore.kernel.org/all/CAP01T74KNVT2SVRR+XPbFb1vy85W=jkp4_yd6xvtsg2avqo...@mail.gmail.com/
[2]: https://lore.kernel.org/all/[email protected]/
> > +
> > +__bpf_kfunc_end_defs();
> > +
> > +BTF_KFUNCS_START(wakeup_source_kfunc_ids)
> > +BTF_ID_FLAGS(func, bpf_wakeup_sources_read_lock, KF_ACQUIRE)
> > +BTF_ID_FLAGS(func, bpf_wakeup_sources_read_unlock, KF_RELEASE)
>
> Should bpf_wakeup_sources_read_lock have the KF_RET_NULL flag?
>
> The function returns an SRCU index cast to a pointer (incremented by 1).
> If wakeup_sources_read_lock() were to fail, the result would need to be
> checked for null. Without KF_RET_NULL, the BPF verifier won't require
> null checks on the returned pointer.
>
> Looking at bpf_wakeup_sources_read_lock():
>
> __bpf_kfunc struct bpf_ws_lock *bpf_wakeup_sources_read_lock(void)
> {
> return (struct bpf_ws_lock *)(long)(wakeup_sources_read_lock() + 1);
> }
>
> If the underlying wakeup_sources_read_lock() could return a value that,
> when incremented, equals zero, or if there's any error path, BPF programs
> would need to check for NULL before using the lock pointer.
>
> Puranjay Mohan raised this in v2, but it was not addressed in v3 or v4.
>
> Reference: https://lore.kernel.org/all/[email protected]/
>
This is fine, the underlying srcu index return value is guaranteed to
be non-negative, so the return value of bpf_wakeup_sources_read_lock()
will be positive and not need the NULL check.
