All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kumar Kartikeya Dwivedi" <memxor@gmail.com>
To: "Hao Sun" <sunhao.th@gmail.com>, <bpf@vger.kernel.org>
Cc: <ast@kernel.org>, <daniel@iogearbox.net>, <andrii@kernel.org>,
	<eddyz87@gmail.com>, <john.fastabend@gmail.com>,
	<martin.lau@linux.dev>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] bpf: Fix uninit read for non-fetch atomics on partially spilled slots
Date: Fri, 25 Sep 2026 04:23:10 +0200	[thread overview]
Message-ID: <DLO1FZU14J3C.1SLTCMVTS8DD5@gmail.com> (raw)
In-Reply-To: <20260924131342.934290-1-sunhao.th@gmail.com>

On Thu Sep 24, 2026 at 3:13 PM CEST, Hao Sun wrote:
> check_stack_read_fixed_off() skips partial spill checks for non-fetch
> atomics; the following prog can be loaded:
>
> 0: (b7) r1 = 1                        ; R1=1
> 1: (63) *(u32 *)(r10 -8) = r1         ; R1=1 R10=fp0 fp-8=????1
> 2: (db) lock *(u64 *)(r10 -8) += r1   ; R1=1 R10=fp0 fp-8=mmmmmmmm
> 3: (79) r0 = *(u64 *)(r10 -8)         ; R0=scalar() R10=fp0 fp-8=mmmmmmmm
> 4: (77) r0 >>= 32                     ; R0=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
> 5: (95) exit
>
> When test run:
> 	retval=4294967295
>
> Note fp-8 is ????1 at #1, yet it becomes fp-8=mmmmmmmm after the
> non-fetching atomic add at #2; hence the high 32 bits are leaked.
>
> Fix by applying the partial load check; after the patch, the
> prog is rejected:
>
> Verification failed: Memory Safety: Uninitialized stack read
>
> Reason:
>   This rejected read uses 8 bytes at stack offset -8, but byte 4 in that range is uninitialized on
>   this path. Programs loaded with CAP_PERFMON can be allowed to read uninitialized stack bytes, but
>   this program is being rejected without that allowance.
>
> At:
>   ...
>       0 | (b7) r1 = 1
>       1 | (63) *(u32 *)(r10 -8) = r1
>   >>> 2 | (db) lock *(u64 *)(r10 -8) += r1
>       3 | (79) r0 = *(u64 *)(r10 -8)
>       4 | (77) r0 >>= 32
>
> This affects CAP_BPF only.
>
> Fixes: 354e8f1970f8 ("bpf: Support <8-byte scalar spill and refill")
> Signed-off-by: Hao Sun <sunhao.th@gmail.com>
>
> ---

The same root cause is also reachable through check_stack_range_initialized().
It takes the spilled register path for every byte of a slot holding a spilled
scalar, without looking at the byte's own slot_type, so a helper or kfunc memory
argument spanning a narrow spill still reads the uninitialized half without
CAP_PERFMON:

  r1 = 1;
  *(u32 *)(r10 - 8) = r1;
  r1 = map_ringbuf ll;
  r2 = r10;
  r2 += -8;
  r3 = 8;
  r4 = 0;
  call bpf_ringbuf_output;

loads with CAP_BPF alone and copies four uninitialized bytes of kernel stack to
user space. The fix is to only take that path when *stype == STACK_SPILL, so the
rest of the slot goes through the usual MISC/ZERO/INVALID checks like any other
slot. Could you add that to v2 with a matching test (same Fixes: tag), since
it's the same bug on the helper path?

Locally I tried something like this and it addressed the problem.

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 200ec0f71617..9166dccef95a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7481,7 +7481,12 @@ static int check_stack_range_initialized(
                        goto mark;
                }

-               if (bpf_is_spilled_reg(ss) &&
+               /*
+                * Only the bytes marked STACK_SPILL hold the spilled register.
+                * The rest of a narrowly spilled slot keeps its previous type
+                * and must be initialized on its own.
+                */
+               if (*stype == STACK_SPILL &&
                    (ss->spilled_ptr.type == SCALAR_VALUE ||
                     env->allow_ptr_leaks)) {
                        if (clobber) {

Please also target bpf-next in next revision.

pw-bot: cr

  parent reply	other threads:[~2026-09-25  2:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-24 13:13 [PATCH 1/2] bpf: Fix uninit read for non-fetch atomics on partially spilled slots Hao Sun
2026-09-24 13:13 ` [PATCH 2/2] selftests/bpf: Test non-fetch atomic on a narrow stack spill Hao Sun
2026-09-25  2:23 ` Kumar Kartikeya Dwivedi [this message]
2026-09-25  2:37   ` [PATCH 1/2] bpf: Fix uninit read for non-fetch atomics on partially spilled slots Kumar Kartikeya Dwivedi

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=DLO1FZU14J3C.1SLTCMVTS8DD5@gmail.com \
    --to=memxor@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=john.fastabend@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=sunhao.th@gmail.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.