All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>
To: <sashiko-reviews@lists.linux.dev>, <bpf@vger.kernel.org>
Subject: Re: [PATCH v2 bpf 14/19] bpf: Reset the range of a packet pointer that scalar evolution clamps
Date: Tue, 27 Oct 2026 20:09:38 +0000	[thread overview]
Message-ID: <DMFW5YZ93QUU.1AGG5BSIGD0JW@gmail.com> (raw)
In-Reply-To: <sashiko-outbox-187068@kernel.org>

On Tue, Oct 27, 2026 at 12:30 PM sashiko-bot@kernel.org <sashiko-bot@kernel.org> wrote:
>> +		if (type_is_pkt_pointer(reg->type))
>> +			reg->range = max(entry_reg->range, 0);
>
> [Severity: Medium]
> Should bpf_widen_scev_regs() also clamp reg->range with max(reg->range, 0)
> when widening a packet pointer at loop entry?

[...]

> If the loop has a negative slope that moves the pointer backwards into the
> packet on subsequent iterations, is_pkt_ptr_branch_taken() in
> kernel/bpf/verifier.c will still fold pkt_end comparisons on the widened
> register using the stale negative range marker.
>
> In addition, when the backedge reaches the loop header, bpf_clamp_scev_regs()
> sets rcur->range to 0 via max(entry_reg->range, 0) while the widened header
> state still has rold->range < 0, causing regsafe() in kernel/bpf/states.c to
> reject state pruning at the loop header:

Not a bug.
The negative range is correct in the first iteration. The next iterations
come from the clamp with range 0. regsafe() doesn't prune 0 against -2,
so the verifier verifies the loop again with range 0 and takes both
branches of the compare.
The cost is one more pass over the loop. It's the same before this patch:
adjust_ptr_min_max_vals() clears a negative range in r6 -= 8.

>>  		err = bpf_set_reg_range(env, reg, bounds.r64, bounds.r32,
>>  					bounds.base, bounds.step);
>
> [Severity: Critical]
> Can preserving entry_reg->range across bpf_set_reg_range() here (and in
> bpf_widen_scev_regs()) allow out-of-bounds packet reads and writes past
> pkt_end when bounds.step tightens reg_umax(reg)?

[...]

> Here bounds.step becomes 8. Calling bpf_set_reg_range() -> reg_bounds_sync()
> -> __update_bounds_from_step() -> cnum64_intersect_linear() then rounds
> reg_umax(reg) down from 58 to 56 while reg->range remains 58 (and similarly
> rounds [0, 50] down to [0, 48] with range = 50 for a negative slope r6 -= 8).

yeah. It's an old bug, not from this patch.
bpf_widen_scev_regs() has it since commit 6ccf49f79327: it keeps the range
and lowers umax in the same way. The clamp is not necessary to hit it.

I have a fix ready and will send in follow up after this lands.

  reply	other threads:[~2026-10-27 20:09 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-27 19:02 [PATCH v2 bpf 00/19] bpf: Remove BPF_F_ARENA_SCALAR, fix scalar evolution and scalar forks Alexei Starovoitov
2026-10-27 19:02 ` [PATCH v2 bpf 01/19] selftests/bpf: Remove tests for global data in arena Alexei Starovoitov
2026-10-27 19:02 ` [PATCH v2 bpf 02/19] libbpf: Remove .arena.data Alexei Starovoitov
2026-10-27 19:02 ` [PATCH v2 bpf 03/19] libbpf: Keep format strings of bpf_printk() in .rodata Alexei Starovoitov
2026-10-27 19:02 ` [PATCH v2 bpf 04/19] selftests/bpf: Remove tests for arena access through numbers Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 05/19] bpf: Remove BPF_F_ARENA_SCALAR Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 06/19] bpf, x86: Remove r0_on_stack from fetching atomics in arena Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 07/19] bpf: Clear delta of a register that scalar evolution widens Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 08/19] selftests/bpf: Test that a widened register is not linked Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 09/19] selftests/bpf: Wait for the last maps of map_kptr to be freed Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 10/19] selftests/bpf: Avoid a page fault in rcu_reentry while the handler is armed Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 11/19] bpf: Don't widen a pointer to a buffer in scalar evolution Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 12/19] selftests/bpf: Test a loop that moves a pointer to a buffer Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 13/19] bpf: Don't clamp a register whose type changed in the loop Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 14/19] bpf: Reset the range of a packet pointer that scalar evolution clamps Alexei Starovoitov
2026-10-27 19:30   ` sashiko-bot
2026-10-27 20:09     ` Alexei Starovoitov [this message]
2026-10-27 19:03 ` [PATCH v2 bpf 15/19] selftests/bpf: Test the clamp of pointers in scalar evolution Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 16/19] bpf: Finish the insn in the state that maybe_fork_scalars() splits off Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 17/19] selftests/bpf: Test maybe_fork_scalars() at the head of a counted loop Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 18/19] bpf: Keep stack arguments in arch_bpf_timed_may_goto() Alexei Starovoitov
2026-10-27 19:48   ` bot+bpf-ci
2026-10-28  4:53     ` Alexei Starovoitov
2026-10-27 19:03 ` [PATCH v2 bpf 19/19] selftests/bpf: Test stack arguments across may_goto Alexei Starovoitov
2026-10-28  4:10 ` [PATCH v2 bpf 00/19] bpf: Remove BPF_F_ARENA_SCALAR, fix scalar evolution and scalar forks patchwork-bot+netdevbpf

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=DMFW5YZ93QUU.1AGG5BSIGD0JW@gmail.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko-reviews@lists.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.