Re: [PATCH bpf-next v1 08/18] bpf: Grow the verifier id scratch on demand

["Kumar Kartikeya Dwivedi" <memxor@gmail.com>]

On Wed Sep 23, 2026 at 9:24 PM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] bpf: OOM in release_reference() triggers WARN_ON_ONCE and is improperly ignored in unmark_stack_slots_iter()
> --
>
> commit bdefbd593b54c82f98de0288f1f6f5e793ee9fbb
> Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>
> bpf: Grow the verifier id scratch on demand
>
> The id map used to compare the ids of two states, which also serves as
> the id stack of release_reference() and as the id set of
> bpf_clear_singular_ids(), is a fixed array embedded in struct
> bpf_verifier_env and sized for the most registers and stack slots a
> state can possibly hold. Turn the map and the set into arrays grown on
> demand, starting at 64 entries and doubling, and free them with the env.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index d8f43f3a89912..8641f1a8d017d 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -10136,8 +10136,9 @@ static int idstack_push(struct bpf_idmap *idmap, u32 id)
>>  		if (idmap->map[i].old == id)
>>  			return 0;
>>
>> -	if (WARN_ON_ONCE(idmap->cnt >= BPF_ID_MAP_SIZE))
>> -		return -EFAULT;
>> +	if (!bpf_id_scratch_reserve((void **)&idmap->map, &idmap->cap, idmap->cnt,
>> +				    sizeof(*idmap->map)))
>> +		return -ENOMEM;
>
> [Severity: Medium]
> With this change, idstack_push() can return -ENOMEM on a normal allocation
> failure, which propagates up through release_reference().
>
> However, in kernel/bpf/verifier.c:unmark_stack_slots_iter(), we have:
>
> 	for (i = 0; i < nr_slots; i++) {
> 		struct bpf_stack_state *slot = bpf_stack_slot(state, spi - i);
> 		struct bpf_reg_state *st = &slot->spilled_ptr;
>
> 		if (i == 0)
> 			WARN_ON_ONCE(release_reference(env, st->id));
>
> Does this mean a simple -ENOMEM during verification will trigger this
> WARN_ON_ONCE splat? Since release_reference() can now legitimately fail under
> memory pressure, should this warning be removed, and should the -ENOMEM error
> be properly propagated here instead of discarded?

This one is legit, probably needs more graceful handling.