Re: [PATCH bpf-next v3 03/12] bpf: Prepare linked scalars for relations narrower than 64 bits

[Vineet Gupta]

On 10/29/26 3:51 PM, Eduard Zingerman wrote:
> On Tue, 2026-10-20 at 09:18 -0700, Vineet Gupta wrote:
> > Two scalars sharing an ->id are always related by full 64-bit equality
> > today: reg == base + delta over all 64 bits (this is true for
> > BPF_ADD_CONST32 as well, which implies upper bits to be all zero).
> > Sign-extension needs a genuinely low32 equal only relationship kind
> > with bit 31 supplying upper 32-bits.
> >
> > Today the following 2 relationships exist
> >
> >    #define BPF_ADD_CONST64 (1U << 31)
> >    #define BPF_ADD_CONST32 (1U << 30)
> >
> > Model them differently as
> >
> >    enum bpf_reg_link_kind { BPF_REG_LINK_NONE, BPF_REG_LINK_FULL,
> >                             BPF_REG_LINK_ZEXT, BPF_REG_LINK_SEXT };
> >
> > So BPF_ADD_CONST64 becomes BPF_REG_LINK_FULL and BPF_ADD_CONST32 becomes
> > BPF_REG_LINK_ZEXT with the exact same semantics as today. But this paves
> > the way for a future BPF_REG_LINK_SEXT in the same framework.
> >
> > The implementation needs to change as well. Today the above 2 bits are
> > shoe-horned into ->id, needing masking and OR'ing to get ->id and kind
> > bits: with more types and bits needed, move them out into an enum
> > bitfield of its own. To avoid growing the storage (and stay within the
> > 1K allocator bucket), borrow 2 bits out of ->map_uid, 31 -> 29, which is
> > still far more ids than a program can generate.
> >
> > ->id becomes a plain identifier, so check_scalar_ids() loses its "compare
> > the compound id, then the base id" dance in favour of one check_ids().
> >
> > With the kind out of ->id it also leaves the idmap key, so regs_exact()
> > and regsafe()'s explore_alu_limits arm have to compare it explicitly.
> >
> > One case changes behaviour -- old {r2.id=A+delta32} against
> > cur {r2.id=B+delta64} was accepted and is now rejected, correctly, since
> > alu32 and alu64 deltas diverge in sync_linked_regs().
> >
> > Clearing a link is no longer just ->id = 0: any stale ->link_kind needs
> > to be cleared out as well. And given this needs to be done in the newly
> > added kernel/bpf/scev.c, it is moved into bpf_verifier.h as
> > bpf_clear_scalar_id().
> >
> > The helper also zeroes ->delta, which the open-coded clear left behind.
> > Unlike ->id that field is inside the memcmp() prefix state comparison
> > uses, so a leftover value could keep two otherwise-identical states
> > apart; clearing it can only help pruning.
> >
> > Suggested-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> > Signed-off-by: Vineet Gupta <vineet.gupta@xxxxxxxxx>
> > ---
> Nice refactoring.
>
> Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>

Thx.

> ...
>
> > diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
> > index 5432c0fc442b..a6f205653a9e 100644
> > --- a/kernel/bpf/log.c
> > +++ b/kernel/bpf/log.c
> > @@ -651,8 +651,8 @@ static void print_reg_state(struct bpf_verifier_env *env,
> >   		verbose(env, "%s", btf_type_name(reg->btf, reg->btf_id));
> >   	verbose(env, "(");
> >   	if (reg->id)
> > -		verbose_a("id=%d", reg->id & ~BPF_ADD_CONST);
> > -	if (reg->id & BPF_ADD_CONST)
> > +		verbose_a("id=%d", reg->id);
> > +	if (reg->link_kind)
> >   		verbose(env, "%+d", reg->delta);
> Nit: it would be nice to see if the link is full or zext.

Done in v4.

> >   	if (reg->parent_id)
> >   		verbose_a("parent_id=%d", reg->parent_id);
> > diff --git a/kernel/bpf/scev.c b/kernel/bpf/scev.c
> > index 48500d73105a..f0aa314233c8 100644
> > --- a/kernel/bpf/scev.c
> > +++ b/kernel/bpf/scev.c
> > @@ -2285,7 +2285,7 @@ static void scratch_widened_reg_id(struct bpf_verifier_env *env, struct bpf_reg_
> >   {
> >   	switch (base_type(reg->type)) {
> >   	case SCALAR_VALUE:
> > -		reg->id = 0;
> > +		bpf_clear_scalar_id(reg);
> >   		break;
> >   	case PTR_TO_PACKET:
> >   	case PTR_TO_PACKET_META:
> > diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
> > index 953700822fa7..a1b90a31747a 100644
> > --- a/kernel/bpf/states.c
> > +++ b/kernel/bpf/states.c
> ...
>
> > @@ -574,13 +561,14 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
> >   		 * Linked register tracking uses rold->id to detect relationships.
> >   		 * When rold->id == 0, the register is independent and any linking
> >   		 * in rcur only adds constraints. When rold->id != 0, we must verify
> > -		 * id mapping and (for BPF_ADD_CONST) offset consistency.
> > +		 * id mapping and that the link kinds agree, along with any delta
> > +		 * they carry.
> >   		 *
> >   		 * +------------------+-----------+------------------+---------------+
> > -		 * |                  | rold->id  | rold + ADD_CONST | rold->id == 0 |
> > +		 * |                  | rold->id  | rold linked kind | rold->id == 0 |
> >   		 * |------------------+-----------+------------------+---------------|
> >   		 * | rcur->id         | range,ids | false            | range         |
> > -		 * | rcur + ADD_CONST | false     | range,ids,off    | range         |
> > +		 * | rcur linked kind | false     | range,ids,delta  | range         |
> >   		 * | rcur->id == 0    | range,ids | false            | range         |
> >   		 * +------------------+-----------+------------------+---------------+
> Nit: "linked kind" -> "link_kind".
>       also, the table should show when link_kind equality is required.
> ...

Done as well. I'll carry the Ack.

Thx,
-Vineet