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

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

On Thu Sep 24, 2026 at 11:13 AM CEST, bot+bpf-ci wrote:
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index b314b9425b86..3ff1d4f753d3 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>
> [ ... ]
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 8b7f2c283e5a..aebe2e6b2a3d 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>
> [ ... ]
>
>> @@ -18898,12 +18903,13 @@ static void idset_cnt_inc(struct bpf_idset *idset, u32 id)
>>  			return;
>>  		}
>>  	}
>> -	/* New id */
>> -	if (idset->num_ids < BPF_ID_MAP_SIZE) {
>> -		idset->entries[idset->num_ids].id = id;
>> -		idset->entries[idset->num_ids].cnt = 1;
>> -		idset->num_ids++;
>> -	}
>> +	/* New id; one that cannot be recorded counts as shared and is kept */
>> +	if (!bpf_id_scratch_reserve((void **)&idset->entries, &idset->cap, idset->num_ids,
>> +				    sizeof(*idset->entries)))
>> +		return;
>> +	idset->entries[idset->num_ids].id = id;
>> +	idset->entries[idset->num_ids].cnt = 1;
>> +	idset->num_ids++;
>>  }
>
> The comment says "one that cannot be recorded counts as shared and is kept".
> With the old fixed array, once num_ids reached BPF_ID_MAP_SIZE, that was
> permanent -- every subsequent new id failed to be recorded. But
> krealloc_array() failure is transient, and bpf_id_scratch_reserve() leaves
> cap unchanged after a failure, so the next call retries the same allocation
> and can succeed.
>
> Consider a scalar id X held by exactly two registers, with num_ids == cap
> when X first appears in the idset_cnt_inc() pass of bpf_clear_singular_ids():
>
>   1st occurrence of X: reserve fails -> X not recorded
>   (a later new id Y: reserve succeeds -> cap doubles)
>   2nd occurrence of X: not found -> reserve succeeds -> entries[] = {X, cnt = 1}
>
> X now has cnt == 1 even though two registers share it. The second pass then
> runs clear_scalar_id() on both registers. This is sound, because the cleared
> state is cur, which keeps being explored, and forgetting a relation only
> widens the abstraction.
>
> It does silently drop a real scalar link, though: sync_linked_regs() no
> longer refines the second register, so a valid program can be rejected with
> an ordinary verifier error instead of failing with -ENOMEM. The outcome also
> depends on allocation timing.
>
> Would it be better to remember that a reservation failed during this pass
> (for example a flag in struct bpf_idset) and then skip the clearing pass
> entirely? That would make "is kept" true in every case.
>

Fixed for v3: when the id set cannot record an id its counts are incomplete, so
bpf_clear_singular_ids() now keeps every id instead of clearing any.

>
> ---
> 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/35976322553