Re: [RFC bpf-next 2/6] bpf: move the linked-scalar flags into bpf_reg_state->flags [NFC]

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

On 8/15/26 5:04 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Missing BPF_FLAG_ADD_CONST check in the explore_alu_limits verification path allows incorrect pruning of incompatible linked scalars.
> - [Medium] Narrowing spills of scalar registers fail to clear BPF_FLAG_ADD_CONST linkage flags, leading to stale flags on unlinked registers and subsequent pruning failures in regsafe().
> --
>
> --- Patch [2]: [RFC bpf-next 2/6] bpf: move the linked-scalar flags into bpf_reg_state->flags [NFC] ---
> commit 19636caada7b86a42b8056f18241af3d2e662845
> Author: Vineet Gupta <vineet.gupta@linux.dev>
> Subject: bpf: move the linked-scalar flags into bpf_reg_state->flags [NFC]
>
> This patch moves the linked-scalar flags from the bpf_reg_state->id field
> into a dedicated bpf_reg_state->flags field. It also updates the verification
> logic to explicitly check these new flags using the link_flags_match helper
> where appropriate.
>
>> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
>> index f7a0314fa1064..d3105b9a9965f 100644
>> --- a/kernel/bpf/states.c
>> +++ b/kernel/bpf/states.c
>> @@ -488,11 +480,32 @@ static int clean_verifier_state(struct bpf_verifier_env *env,
> [ ... ]
>
>> +static bool link_flags_match(const struct bpf_reg_state *rold,
>> +			     const struct bpf_reg_state *rcur)
>> +{
>> +	if (!rold->id)
>> +		return true;
>> +
>> +	return (rold->flags & BPF_FLAG_ADD_CONST) == (rcur->flags & BPF_FLAG_ADD_CONST);
>> +}
>> +
>>   static bool regs_exact(const struct bpf_reg_state *rold,
>>   		       const struct bpf_reg_state *rcur,
>>   		       struct bpf_idmap *idmap)
>>   {
>>   	return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
>> +	       link_flags_match(rold, rcur) &&
>>   	       check_ids(rold->id, rcur->id, idmap) &&
>>   	       check_ids(rold->parent_id, rcur->parent_id, idmap);
>>   }
> [Severity: High]
> Does the explore_alu_limits verification path also need a similar update?
>
> In regsafe(), when env->explore_alu_limits is true, the verifier uses a
> strict comparison:
>
> regsafe() {
>          if (env->explore_alu_limits) {
>                  return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
>                         check_scalar_ids(rold->id, rcur->id, idmap);
>          }
> }
>
> Since memcmp() stops at the id field, it misses the newly added flags
> field. Without calling link_flags_match(), states with the same base
> scalar IDs but different linkage flags (such as BPF_FLAG_ADD_CONST32 vs
> BPF_FLAG_ADD_CONST64) might be incorrectly deemed equivalent, bypassing
> the strict bounds checking intended by this mode.

Agree.

> [Severity: Medium]
> Can narrowing spills leave stale flags on unlinked registers?
>
> When a linked scalar register is spilled with a smaller size,
> check_stack_write_fixed_off() copies the register and manually breaks
> the relation by setting the id to 0:
>
> check_stack_write_fixed_off() {
>          save_register_state(env, state, spi, reg, size);
>          if (!reg_value_fits)
>                  state->stack[spi].spilled_ptr.id = 0;
> }
>
> Since the linkage flags were moved out of the id field, setting id to 0
> no longer clears the BPF_FLAG_ADD_CONST bits.
>
> If this state is later compared in regsafe(), link_flags_match() will see
> rold->id != 0, but rcur->id == 0 with a stale BPF_FLAG_ADD_CONST flag.
> Because the rold->id check does not short-circuit for rcur, the helper
> will evaluate to false, incorrectly rejecting a state that is safe to
> prune.
>
> Would it be safer to use clear_scalar_id() or otherwise clear the flags
> when breaking the relation?

Agree.

Thx,
-Vineet