All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vineet Gupta <vineet.gupta@linux.dev>
To: bot+bpf-ci@kernel.org, ast@kernel.org, daniel@iogearbox.net,
	andrii@kernel.org, eddyz87@gmail.com, 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,
	martin.lau@kernel.org, mason@kernel.org
Subject: Re: [PATCH bpf-next v3 03/12] bpf: Prepare linked scalars for relations narrower than 64 bits
Date: Mon, 26 Oct 2026 15:52:24 -0700	[thread overview]
Message-ID: <66f1317a-4107-4e31-a3cb-6378fc63d07d@linux.dev> (raw)
In-Reply-To: <443e1351eed4f46cf6c202e5f1130b189e9c5eb09cfcffd24d752c6ec73d450c@mail.kernel.org>

On 10/20/26 10:08 AM, bot+bpf-ci@kernel.org wrote:
>> bpf: Prepare linked scalars for relations narrower than 64 bits
> TL;DR: Commit message only: regs_exact() now also rejects states whose linked
> scalars share a base id in old but not in cur, a stricter infinite-loop check
> the changelog doesn't mention.
>
>> bpf: Prepare linked scalars for relations narrower than 64 bits
>>
>> 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>
> The changelog lists only the delta32/delta64 mismatch as a case that changes
> behaviour.  Is that the only one?
>
> regs_exact() also changes.  It used to run check_ids() on the compound id,
> so old {r2.id=A, r3.id=A|BPF_ADD_CONST64} against cur {r2.id=B,
> r3.id=C|BPF_ADD_CONST64} built two independent idmap entries (A->B and
> A|flag->C|flag) and was accepted as exact.
>
> With the kind moved out of ->id, the same pair now maps A->B and then A->C,
> and check_ids() rejects it:
>
>      return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
>             rold->link_kind == rcur->link_kind &&
>             check_ids(rold->id, rcur->id, idmap) && ...
>
> regs_exact() is reached through regsafe(..., EXACT) from the infinite loop
> check in bpf_is_state_visited():
>
>      states_equal(env, &sl->state, cur, EXACT)
>
> so this is a stricter comparison than before.
>
> Could the changelog also say that regs_exact() now enforces base-id
> consistency for linked scalars, which the old compound-id idmap key did not?

Right. The example you mention is new.

Basically v2 had a patch of its own on this topic and it listed two cases

     old {r1.id=A, r2.id=A+delta}  vs cur {r1.id=B, r2.id=B}
     old {r2.id=A+delta32}         vs cur {r2.id=B+delta64}

Both were same category: one register whose kind differs between old and 
cur, caught by the new
rold->link_kind == rcur->link_kind line.

What you mention here is a different scenario:

     old {r2.id=A, r3.id=A|BPF_ADD_CONST64}
     cur {r2.id=B, r3.id=C|BPF_ADD_CONST64}

kinds match but it is still rejected, by check_ids().

v4 mentions both mechanisms.

Thx,
-Vineet

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


  reply	other threads:[~2026-10-26 22:52 UTC|newest]

Thread overview: 36+ 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 [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 16:39   ` sashiko-bot
2026-10-26 23:06     ` Vineet Gupta
2026-10-20 17:09   ` bot+bpf-ci
2026-10-26 20:30     ` Vineet Gupta
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:57   ` sashiko-bot
2026-10-26 20:46     ` 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 17:20   ` sashiko-bot
2026-10-26 21:28     ` 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:29   ` sashiko-bot
2026-10-20 17:15     ` 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:42   ` sashiko-bot
2026-10-26 21:51     ` 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
2026-10-20 16:32   ` sashiko-bot
2026-10-26 22:01     ` 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=66f1317a-4107-4e31-a3cb-6378fc63d07d@linux.dev \
    --to=vineet.gupta@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bot+bpf-ci@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --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@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=mason@kernel.org \
    --cc=memxor@gmail.com \
    --cc=shuah@kernel.org \
    --cc=song@kernel.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.