Re: [RFC PATCH bpf-next 10/12] bpf: support "__uninit" iterator arguments in generic kfuncs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: aliased __uninit and __iter arguments bypass verifier initialization checks
--

commit e08b0892f34d46a76c048cb46111b4956bcddfe8
Author: Justin Suess <utilityemal77@gmail.com>

bpf: support "__uninit" iterator arguments in generic kfuncs

This commit allows generic kfuncs to initialize iterator arguments by
recognizing the __uninit suffix. It updates process_iter_arg() to treat
these marked arguments similarly to how bpf_iter_<type>_new() operates on
uninitialized iterator state.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 066c4b838b858..3294b2a431674 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -8363,8 +8386,16 @@ static int process_iter_arg(struct bpf_verifier_env *env, struct bpf_reg_state *
>  	t = btf_type_by_id(meta->btf, btf_id);
>  	nr_slots = t->size / BPF_REG_SIZE;
>  
> -	if (is_iter_new_kfunc(meta)) {
> -		/* bpf_iter_<type>_new() expects pointer to uninit iter state */
> +	/*
> +	 * Whether this argument is the iterator the call initializes, rather
> +	 * than an initialized one it operates on.
> +	 */
> +	init = is_iter_new_kfunc(meta) ||
> +	       is_kfunc_arg_iter_init(meta, arg,
> +				      &btf_params(meta->func_proto)[arg]);
> +
> +	if (init) {
> +		/* expects a pointer to uninit iter state */

[Severity: Medium]
Is there a risk of bypassing verifier initialization checks if a BPF program
passes aliased registers for these arguments?

When check_func_args() evaluates kfunc arguments sequentially, an __uninit
argument will set init to true here, and process_iter_arg() will immediately
mark the stack slots as initialized via mark_stack_slots_iter().

If a subsequent __iter argument to the same kfunc aliases those exact same
stack slots, its is_iter_reg_valid_init() check will incorrectly pass because
the previous argument just marked them as initialized. This could allow
uninitialized stack memory to be passed where an initialized iterator is
expected.

While this is mitigated in this patch series because the newly introduced
kfunc uses different BTF types for its iterators (causing the bypass to fail
strict BTF ID checks), does this structure need to be fixed to prevent future
issues when kfuncs use matching iterator types?

Should state modifications like mark_stack_slots_iter() be deferred until
all arguments are successfully validated, similar to how
ARG_PTR_TO_UNINIT_MEM is handled?

>  		if (!is_iter_reg_valid_uninit(env, reg, nr_slots)) {
>  			verbose(env, "expected uninitialized iter_%s as %s\n",
>  				iter_type_str(meta->btf, btf_id), reg_arg_name(env, argno));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006002020.2890858-1-utilityemal77@gmail.com?part=10