On Mon, 2025-05-12 at 19:57 -0700, Kumar Kartikeya Dwivedi wrote:
[...]
In Documentation/bpf/kfuncs.rst there are descriptions for several
suffixes that are supported, "__prog" should be added there.
Do we want to add a separate test case for this feature?
It looks like there are no tests for other suffixes,
so relying on wq tests passing is probably fine.
Implementation lgtm, a few nits below.
> include/linux/bpf_verifier.h | 1 +
> kernel/bpf/helpers.c | 4 ++--
> kernel/bpf/verifier.c | 33 +++++++++++++++++++++++++++------
> 3 files changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 9734544b6957..7dd85ed6059e 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -606,6 +606,7 @@ struct bpf_insn_aux_data {
> bool calls_callback;
> /* registers alive before this instruction. */
> u16 live_regs_before;
> + u16 arg_prog;
Nit: there is a 4-bit hole after `fastcall_spills_num`,
`arg_prog` field could be put there and 2 bytes at the tail
would be remain for future extension.
> };
>
> #define MAX_USED_MAPS 64 /* max number of maps accessed by one eBPF program */
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index fed53da75025..43cbf439b9fb 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -3012,9 +3012,9 @@ __bpf_kfunc int bpf_wq_start(struct bpf_wq *wq, unsigned int flags)
> __bpf_kfunc int bpf_wq_set_callback_impl(struct bpf_wq *wq,
> int (callback_fn)(void *map, int *key, void *value),
> unsigned int flags,
> - void *aux__ign)
> + void *aux__prog)
> {
> - struct bpf_prog_aux *aux = (struct bpf_prog_aux *)aux__ign;
> + struct bpf_prog_aux *aux = (struct bpf_prog_aux *)aux__prog;
> struct bpf_async_kern *async = (struct bpf_async_kern *)wq;
>
> if (flags)
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 28f5a7899bd6..f409a06099f6 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -343,6 +343,7 @@ struct bpf_kfunc_call_arg_meta {
> int uid;
> } map;
> u64 mem_size;
> + u32 arg_prog;
Nit: this is a boolean flag, I'd put it after `bool arg_owning_ref`,
as there is a 3 bytes hole there.
> };
>
> struct btf *btf_vmlinux;
[...]
@@ -12906,6 +12912,17 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
if (is_kfunc_arg_ignore(btf, &args[i]))
continue;
+ if (is_kfunc_arg_prog(btf, &args[i])) {
+ /* Used to reject repeated use of __aux. */
^^^^^^
Nit: should be __prog.
+ if (meta->arg_prog) {
+ verbose(env, "Only 1 prog->aux argument supported per-kfunc\n");
+ return -EFAULT;
+ }
+ meta->arg_prog = regno;
+ cur_aux(env)->arg_prog = regno;
+ continue;
+ }
+
if (btf_type_is_scalar(t)) {
if (reg->type != SCALAR_VALUE) {
verbose(env, "R%d is not a scalar\n", regno);
