mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: Vineet Gupta <vineet.gupta@linux.dev>,
	ast@kernel.org, daniel@iogearbox.net, 	andrii@kernel.org,
	memxor@gmail.com
Cc: martin.lau@linux.dev, song@kernel.org, yonghong.song@linux.dev,
	 jolsa@kernel.org, emil@etsalapatis.com, ihor.solodrai@linux.dev,
	 john.fastabend@gmail.com, shuah@kernel.org, bpf@vger.kernel.org,
	 linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH bpf-next v3 03/12] bpf: Prepare linked scalars for relations narrower than 64 bits
Date: Thu, 29 Oct 2026 15:51:50 -0700	[thread overview]
Message-ID: <49019042b63d5718a9419e8fac035cf9104ae8bf.camel@gmail.com> (raw)
In-Reply-To: <20261020161907.2115379-4-vineet.gupta@linux.dev>

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@gmail.com>
> Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
> ---

Nice refactoring.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

...

> 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.

>  	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.

...

  parent reply	other threads:[~2026-10-29 22:51 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20 16:18 [PATCH bpf-next v3 00/12] bpf: track scalar equality across the low 32 bits Vineet Gupta
2026-10-20 16:18 ` [PATCH bpf-next v3 01/12] bpf: Keep the 32-bit range across a sign extension that cannot change it Vineet Gupta
2026-10-29 22:30   ` Eduard Zingerman
2026-10-20 16:18 ` [PATCH bpf-next v3 02/12] selftests/bpf: Cover sign extensions that cannot change the range Vineet Gupta
2026-10-20 16:18 ` [PATCH bpf-next v3 03/12] bpf: Prepare linked scalars for relations narrower than 64 bits Vineet Gupta
2026-10-20 17:08   ` bot+bpf-ci
2026-10-26 22:52     ` Vineet Gupta
2026-10-29 22:51   ` Eduard Zingerman [this message]
2026-10-20 16:18 ` [PATCH bpf-next v3 04/12] bpf: Track low-32 scalar equality across zero-extending movs Vineet Gupta
2026-10-20 17:09   ` bot+bpf-ci
2026-10-26 20:30     ` Vineet Gupta
2026-10-29 23:59   ` Eduard Zingerman
2026-10-20 16:19 ` [PATCH bpf-next v3 05/12] selftests/bpf: Cover the low-32 link for " Vineet Gupta
2026-10-20 16:53   ` bot+bpf-ci
2026-10-26 18:36     ` Vineet Gupta
2026-10-20 16:19 ` [PATCH bpf-next v3 06/12] bpf: Track low-32 scalar equality across sign-extending movs Vineet Gupta
2026-10-20 16:19 ` [PATCH bpf-next v3 07/12] selftests/bpf: Cover the low-32 link for " Vineet Gupta
2026-10-20 16:19 ` [PATCH bpf-next v3 08/12] bpf: Recover the low 32 bits on the reverse link direction Vineet Gupta
2026-10-20 16:53   ` bot+bpf-ci
2026-10-26 19:57     ` Vineet Gupta
2026-10-20 16:19 ` [PATCH bpf-next v3 09/12] bpf: Track low-32 scalar equality across narrowing stack fills Vineet Gupta
2026-10-20 16:19 ` [PATCH bpf-next v3 10/12] selftests/bpf: Cover the low-32 link for " Vineet Gupta
2026-10-20 16:52   ` bot+bpf-ci
2026-10-26 20:21     ` Vineet Gupta
2026-10-20 16:19 ` [PATCH bpf-next v3 11/12] [do not merge] bpf: Track low-32 scalar equality across narrowing stack spills Vineet Gupta
2026-10-20 16:19 ` [PATCH bpf-next v3 12/12] [do not merge] selftests/bpf: Cover the low-32 link for " Vineet Gupta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49019042b63d5718a9419e8fac035cf9104ae8bf.camel@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=emil@etsalapatis.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=shuah@kernel.org \
    --cc=song@kernel.org \
    --cc=vineet.gupta@linux.dev \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®