All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: Kumar Kartikeya Dwivedi <memxor@gmail.com>, bpf@vger.kernel.org
Cc: Nicholas Carlini <npc@anthropic.com>,
	Alexei Starovoitov <ast@kernel.org>,
	 Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Emil Tsalapatis <emil@etsalapatis.com>,
	 kkd@meta.com, kernel-team@meta.com
Subject: Re: [PATCH bpf v2 2/7] bpf: Preserve packet pointer class displacement in regsafe()
Date: Sat, 05 Sep 2026 23:40:53 -0700	[thread overview]
Message-ID: <83e2ad1e23774de204372c7e3d3bddc8f570c099.camel@gmail.com> (raw)
In-Reply-To: <20260905083418.3723623-3-memxor@gmail.com>

On Sat, 2026-09-05 at 10:34 +0200, Kumar Kartikeya Dwivedi wrote:
> regsafe() maps packet pointer IDs between states and checks that each
> current register range is a subset of the corresponding explored
> register range. It does not, however, preserve the displacement between
> registers that share a packet pointer ID.
> 
> This is unsound because packet range is shared by ID. A bounds check on
> one class member updates every member, and a later access can consume the
> range through another member. Commit 022ac0750883 ("bpf: use reg->var_off
> instead of reg->off for pointers") folded the fixed pointer offset into
> r64 and removed the old off equality check, so two individually narrower
> registers can prune even when their displacement has changed. The
> explored path can then license an out-of-bounds packet access on the
> pruned path.
> 
> Requiring equal r64 bases would prevent the bug, but would also reject a
> safe uniform translation of the whole class. Instead, record the base
> translation seen for the first packet pointer in each ID mapping and
> require every subsequent member to have the same translation. This keeps
> the relative displacement invariant while retaining pruning for uniformly
> translated classes. Packet pointers without an ID remain unaffected.
> 
> Fixes: 022ac0750883 ("bpf: use reg->var_off instead of reg->off for pointers")
> Reported-by: Nicholas Carlini <npc@anthropic.com>
> Suggested-by: Nicholas Carlini <npc@anthropic.com>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---

So, essentially this is the same problem as `id + ADD_CONST` tracking
for scalar values, where the actual difference is stored in `delta` field.
In other words, that would be an undo of 022ac0750883 for packet pointers.

What about the original suggested fix:

            /* id relations must be preserved */
            if (!check_ids(rold->id, rcur->id, idmap))
                    return false;
    +       if (rold->id && rold->r64.base != rcur->r64.base)
    +               return false;
            /* new val must satisfy old val knowledge */
            return range_within(rold, rcur) &&
                   tnum_in(rold->var_off, rcur->var_off);

Is it that bad?
Given this logic in adjust_ptr_min_max_vals:

    case BPF_ADD:
        ...
		if (reg_is_pkt_pointer(ptr_reg))
			if (!known)
				dst_reg->id = ++env->id_gen;

I'd expect it to be okay.

...

  parent reply	other threads:[~2026-09-06  6:40 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-05  8:34 [PATCH bpf v2 0/7] Misc bug fixes - part 5 Kumar Kartikeya Dwivedi
2026-09-05  8:34 ` [PATCH bpf v2 1/7] bpf: Make post-verification instruction rewrites killable Kumar Kartikeya Dwivedi
2026-09-11 22:56   ` Eduard Zingerman
2026-09-05  8:34 ` [PATCH bpf v2 2/7] bpf: Preserve packet pointer class displacement in regsafe() Kumar Kartikeya Dwivedi
2026-09-05  9:25   ` bot+bpf-ci
2026-09-05 20:46   ` Alexei Starovoitov
2026-09-06  6:40   ` Eduard Zingerman [this message]
2026-09-06  7:04     ` Eduard Zingerman
2026-09-06 15:11       ` Alexei Starovoitov
2026-09-05  8:34 ` [PATCH bpf v2 3/7] selftests/bpf: Test packet pointer class displacement pruning Kumar Kartikeya Dwivedi
2026-09-05  9:10   ` bot+bpf-ci
2026-09-05 20:48   ` Alexei Starovoitov
2026-09-05  8:34 ` [PATCH bpf v2 4/7] bpf: Reject fall-through across subprogram boundaries Kumar Kartikeya Dwivedi
2026-09-05 20:29   ` Alexei Starovoitov
2026-09-05  8:34 ` [PATCH bpf v2 5/7] selftests/bpf: Test poisoned subprogram terminator Kumar Kartikeya Dwivedi
2026-09-05  8:34 ` [PATCH bpf v2 6/7] bpf: Assign lock identity to callback map values Kumar Kartikeya Dwivedi
2026-09-05  9:25   ` bot+bpf-ci
2026-09-12  0:23   ` Eduard Zingerman
2026-09-05  8:34 ` [PATCH bpf v2 7/7] selftests/bpf: Check callback map value lock identity Kumar Kartikeya Dwivedi
2026-09-05  9:10   ` bot+bpf-ci

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=83e2ad1e23774de204372c7e3d3bddc8f570c099.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=kernel-team@meta.com \
    --cc=kkd@meta.com \
    --cc=memxor@gmail.com \
    --cc=npc@anthropic.com \
    /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.