> diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
> index 922ed457db19..8823aceeac8b 100644
> --- a/drivers/base/power/power.h
> +++ b/drivers/base/power/power.h
[ ... ]
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index b8e48a023bf0..80b497de2deb 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
[ ... ]
> @@ -1168,11 +1168,78 @@ static const struct file_operations
> wakeup_sources_stats_fops = {
> .release = seq_release_private,
> };
>
> +#ifdef CONFIG_BPF_SYSCALL
> +#include <linux/btf.h>
> +
> +__bpf_kfunc_start_defs();
[ ... ]
> +/**
> + * 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]/
> +
> +__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]/
> +BTF_ID_FLAGS(func, bpf_wakeup_sources_get_head)
> +BTF_KFUNCS_END(wakeup_source_kfunc_ids)
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25688185901
