Re: [PATCH bpf-next v2 01/13] bpf: move linked-scalar flags out of bpf_reg_state->id [NFC]

[Vineet Gupta <vineet.gupta@linux.dev>]

On 9/10/26 10:30 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Unintentional stricter pruning in explore_alu_limits mode causing state explosion.
> --
>
> commit aa1d987a87f62f32dd84baa98f047ffd7635c40e
> Author: Vineet Gupta <vineet.gupta@linux.dev>
>
> bpf: move linked-scalar flags out of bpf_reg_state->id [NFC]
>
> This commit moves the linked-scalar flags (ADD_CONST32 and ADD_CONST64) out of
> the bpf_reg_state->id field and into a separate add_const bitfield. This
> simplifies ID tracking by removing the need to mask the ID during checks.
>
>> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
>> index 66fb11b6c6a76..d974baad37ee2 100644
>> --- a/kernel/bpf/states.c
>> +++ b/kernel/bpf/states.c
> [ ... ]
>
>> @@ -542,8 +527,7 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
>>   			/* explore_alu_limits disables tnum_in() and range_within()
>>   			 * logic and requires everything to be strict
>>   			 */
>> -			return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
>> -			       check_scalar_ids(rold->id, rcur->id, idmap);
>> +			return regs_exact(rold, rcur, idmap);
>>   		}
> [Severity: Medium]
> Does this unintentionally introduce a stricter pruning check when
> explore_alu_limits is active?
>
> The commit message notes that using regs_exact() amounts to the same as the
> open-coded memcmp plus check_scalar_ids() pair. However, check_scalar_ids()
> allows old_id to be 0 by returning true immediately (ignoring cur_id):
>
> 	if (!old_id)
> 		return true;
>
> On the other hand, regs_exact() calls check_ids(), which strictly enforces
> that both IDs are either zero or non-zero, and fails if they differ:
>
> 	/* either both IDs should be set or both should be zero */
> 	if (!!old_id != !!cur_id)
> 		return false;
>
> When env->explore_alu_limits is true, comparing two equivalent scalar
> registers where one is unlinked (ID == 0) and the other is linked (ID != 0)
> would pass previously but fails now, potentially causing state explosion.

The regs_exact() replacement was suggested in RFC review. And it is true 
it can be tighter.
Since the rest of series changes behavior anyways, I'm inclined towards 
dropping the NFC and document the change. And if we go down this part, 
perhaps patch 2 can also be folded into this one as that was the sole 
reason for breaking it out.

Thx,
-Vineet