mirror of https://lore.kernel.org/lkml/
 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 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 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®