[PATCH bpf 1/8] bpf: don't infer non-NULL from a pointer with an unbounded offset

[PATCH bpf 1/8] bpf: don't infer non-NULL from a pointer with an unbounded offset

[Eduard Zingerman]

reg_not_null() decides that a register holds a non-NULL value by
looking at its type alone. For pointer types that allow arithmetic the
type only guarantees a non-NULL base, in case of an unbound offset
the runtime offset value might still add up to NULL.
Consider the followng program:

  r6 = bpf_map_lookup_elem(map, &0);  /* present */
  if (r6 == 0) return 0;
  r7 = bpf_map_lookup_elem(map, &1);  /* absent, NULL at runtime */
  r8 = r7;
  r8 -= r6;     /* pointer - pointer: unknown scalar, -r6 */
  r8 <<= 1;
  r8 >>= 1;     /* any non-negative offset is accepted by */
                /* check_reg_sane_offset_ptr() */
  r6 += r8;     /* verifier: map value;    runtime: zero  */
  if (r7 != r6) return 0;
  *(u8 *)(r7 + 0);  /* r7 is inferred non-NULL, both are zero */

At runtime both registers are zero, the comparison is true and the
load faults with NULL pointer dereference.

Require the offset to be within +-BPF_MAX_VAR_OFF in reg_not_null().

Fixes: cac616db39c2 ("bpf: Verifier track null pointer branch_taken with JNE and JEQ")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 kernel/bpf/verifier.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e64035683795..e53619e2210e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -352,6 +352,13 @@ static bool reg_not_null(struct bpf_verifier_env *env, const struct bpf_reg_stat
 	if (type_may_be_null(type))
 		return false;
 
+	/*
+	 * The types below guarantee a non-NULL base, an unbounded offset can
+	 * still wrap base + offset to zero.
+	 */
+	if (reg_smin(reg) <= -BPF_MAX_VAR_OFF || reg_smax(reg) >= BPF_MAX_VAR_OFF)
+		return false;
+
 	type = base_type(type);
 	return type == PTR_TO_SOCKET ||
 		type == PTR_TO_TCP_SOCK ||
-- 
2.55.0

[PATCH bpf 2/8] selftests/bpf: no non-NULL inference from unbounded offset pointers

[Eduard Zingerman]

Check that a comparison against a pointer whose offset is not bounded
from above does not make the verifier infer that a nullable pointer is
not NULL, and that a bounded offset still does. W/o the previous patch
the first test is accepted.

Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 .../bpf/progs/verifier_jeq_infer_not_null.c   | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c b/tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c
index b412a542ef76..8657e4a0d601 100644
--- a/tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c
+++ b/tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c
@@ -311,6 +311,86 @@ __naked void untrusted_mem_does_not_infer_map_value_non_null(void)
 	: __clobber_all);
 }
 
+/*
+ * A pointer with an offset that is not bounded from above may be null at
+ * runtime, hence it is not a witness for the pointer it is compared with.
+ */
+SEC("socket")
+__failure
+__msg("error: invalid dereference of R7 (a nullable map value pointer)")
+__naked void unbounded_offset_does_not_infer_map_value_non_null(void)
+{
+	asm volatile ("					\
+	/* r6 = bpf_map_lookup_elem(map_hash, &0); */	\
+	*(u64 *)(r10 - 8) = 0;				\
+	r1 = %[map_hash] ll;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	call %[bpf_map_lookup_elem];			\
+	if r0 == 0 goto 1f;				\
+	r6 = r0;					\
+	/* r7 = bpf_map_lookup_elem(map_hash, &1); */	\
+	*(u64 *)(r10 - 8) = 1;				\
+	r1 = %[map_hash] ll;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	call %[bpf_map_lookup_elem];			\
+	r7 = r0;					\
+	/* pointer - pointer is an unknown scalar */	\
+	r8 = r7;					\
+	r8 -= r6;					\
+	/* r8 is in [0, S64_MAX] */			\
+	r8 <<= 1;					\
+	r8 >>= 1;					\
+	/* r6 may wrap to zero at runtime */		\
+	r6 += r8;					\
+	if r7 != r6 goto 1f;				\
+	r0 = *(u8 *)(r7 + 0);				\
+1:	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(map_hash)
+	: __clobber_all);
+}
+
+/* Same, but the offset is bounded, so the inference is still done. */
+SEC("socket")
+__success
+__naked void bounded_offset_infers_map_value_non_null(void)
+{
+	asm volatile ("					\
+	/* r6 = bpf_map_lookup_elem(map_hash, &0); */	\
+	*(u64 *)(r10 - 8) = 0;				\
+	r1 = %[map_hash] ll;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	call %[bpf_map_lookup_elem];			\
+	if r0 == 0 goto 1f;				\
+	r6 = r0;					\
+	/* r7 = bpf_map_lookup_elem(map_hash, &1); */	\
+	*(u64 *)(r10 - 8) = 1;				\
+	r1 = %[map_hash] ll;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	call %[bpf_map_lookup_elem];			\
+	r7 = r0;					\
+	/* pointer - pointer is an unknown scalar */	\
+	r8 = r7;					\
+	r8 -= r6;					\
+	/* r8 is in [0, 3] */				\
+	r8 &= 3;					\
+	r6 += r8;					\
+	if r7 != r6 goto 1f;				\
+	r0 = *(u8 *)(r7 + 0);				\
+1:	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(map_hash)
+	: __clobber_all);
+}
+
 void kfunc_root(void)
 {
 	bpf_rdonly_cast(0, 0);
-- 
2.55.0

[PATCH bpf 3/8] bpf: don't resurrect a scalar id dropped by collect_linked_regs()

[Eduard Zingerman]

check_cond_jmp_op() copies the compared registers into
env->{false,true}_reg{1,2} before collect_linked_regs() runs and copies
those snapshots back into both branch states afterwards.

collect_linked_regs() records at most LINKED_REGS_MAX members of a
linked registers group in the jump history and calls clear_scalar_id()
for every member that does not fit. The compared register is not exempt
from that.

As a consequence, sync_linked_regs() might adjust ranges for more
registers than bpf_bt_sync_linked_regs() can propagate precision to.

Collect the linked registers before the snapshots are taken instead.
This might lead to some unnecessary clear_scalar_id's, but from
previous testing situations with many linked registers are
extremely rare.

Fixes: ec1d77cb0ee9 ("bpf: Use bpf_verifier_env buffers for reg_set_min_max")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 kernel/bpf/verifier.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e53619e2210e..d16dd1fb08b6 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -16870,6 +16870,16 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 			return err;
 	}
 
+	/*
+	 * Collect the linked registers before env->{true,false}_reg{1,2} setup,
+	 * otherwise ids dropped by collect_linked_regs() would be resurrected
+	 * when env->{true,false}_reg{1,2} are copied back.
+	 */
+	if (BPF_SRC(insn->code) == BPF_X && src_reg->type == SCALAR_VALUE && src_reg->id)
+		collect_linked_regs(env, this_branch, src_reg->id, &linked_regs);
+	if (dst_reg->type == SCALAR_VALUE && dst_reg->id)
+		collect_linked_regs(env, this_branch, dst_reg->id, &linked_regs);
+
 	is_jmp32 = BPF_CLASS(insn->code) == BPF_JMP32;
 	env->false_reg1 = *dst_reg;
 	env->false_reg2 = *src_reg;
@@ -16924,10 +16934,6 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 	 * 'this_branch' and 'other_branch' share this history
 	 * if parent state is created.
 	 */
-	if (BPF_SRC(insn->code) == BPF_X && src_reg->type == SCALAR_VALUE && src_reg->id)
-		collect_linked_regs(env, this_branch, src_reg->id, &linked_regs);
-	if (dst_reg->type == SCALAR_VALUE && dst_reg->id)
-		collect_linked_regs(env, this_branch, dst_reg->id, &linked_regs);
 	if (linked_regs.cnt > 1) {
 		err = bpf_push_jmp_history(env, this_branch, 0, 0, 0, linked_regs_pack(&linked_regs));
 		if (err)
-- 
2.55.0

[PATCH bpf 4/8] selftests/bpf: check the linked regs cap for the compared register

[Eduard Zingerman]

linked_regs_too_many_regs checks that collect_linked_regs() ties at most
LINKED_REGS_MAX registers for a single jump. Compare r5 instead of r0,
so that the register the jump compares is itself the member that does
not fit, and check that it comes out of the jump unlinked.

W/o the previous patch env->{false,true}_reg{1,2} bring r5's id back and
insn 7 is logged as "R5=scalar(id=1,...)".

Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 .../selftests/bpf/progs/verifier_scalar_ids.c       | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/verifier_scalar_ids.c b/tools/testing/selftests/bpf/progs/verifier_scalar_ids.c
index 663d15fc5fd2..256547048cc4 100644
--- a/tools/testing/selftests/bpf/progs/verifier_scalar_ids.c
+++ b/tools/testing/selftests/bpf/progs/verifier_scalar_ids.c
@@ -380,13 +380,14 @@ SEC("socket")
 __success __log_level(2)
 __flag(BPF_F_TEST_STATE_FREQ)
 /*
- * check that r0 and r5 have different IDs after 'if',
- * collect_linked_regs() can't tie more than 5 registers for a single insn.
+ * check that r5 is unlinked after 'if', collect_linked_regs() can't tie
+ * more than 5 registers for a single insn and the register compared by
+ * the jump is not exempt from that.
  */
-__msg("7: (25) if r0 > 0x7 goto pc+0         ; R0=scalar(id=1")
+__msg("7: (25) if r5 > 0x7 goto pc+0         ; R5=scalar(smin=")
 __msg("12: (bf) r5 = r5                      ; R5=scalar(id=2")
 /* check that r{0-4} are marked precise after 'if' */
-__msg("frame0: regs=r0 stack= before 7: (25) if r0 > 0x7 goto pc+0")
+__msg("frame0: regs=r0 stack= before 7: (25) if r5 > 0x7 goto pc+0")
 __msg("frame0: parent state regs=r0,r1,r2,r3,r4 stack=:")
 __naked void linked_regs_too_many_regs(void)
 {
@@ -400,8 +401,8 @@ __naked void linked_regs_too_many_regs(void)
 	"r3 = r0;"
 	"r4 = r0;"
 	"r5 = r0;"
-	/* propagate range for r{0-5} */
-	"if r0 > 7 goto +0;"
+	/* r{0-4} fill the record, r5 does not fit and is unlinked */
+	"if r5 > 7 goto +0;"
 	/* keep r{1-4} live */
 	"r1 = r1;"
 	"r2 = r2;"
-- 
2.55.0

[PATCH bpf 5/8] bpf: don't predict JMP32 pointer vs zero comparisons

[Eduard Zingerman]

Consider the following program:

        r1 = map_value;      /* low 32 bits are zero at runtime */
        r6 = 0xdead000000000000;
        if w1 != 0 goto l1;
    l0: r1 += r6;
        r2 = *(u64 *)(r1 + 0);
        exit;
    l1: r6 = 0;
        goto l0;

At the moment is_branch_taken() reports the jump as always taken,
because it does not distinguish between BPF_JMP and BPF_JMP32
comparisons when processing 'if w1 != 0 ...'.

Fixes: cac616db39c2 ("bpf: Verifier track null pointer branch_taken with JNE and JEQ")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 kernel/bpf/verifier.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d16dd1fb08b6..bb8e9efcfbad 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -16309,6 +16309,13 @@ static int is_branch_taken(struct bpf_verifier_env *env, struct bpf_reg_state *r
 	if (__is_pointer_value(false, reg1) || __is_pointer_value(false, reg2)) {
 		u64 val;
 
+		/*
+		 * The low 32 bits of a valid pointer may well be zero, hence
+		 * nothing below applies to a 32-bit comparison.
+		 */
+		if (is_jmp32)
+			return -1;
+
 		/* arrange that reg2 is a scalar, and reg1 is a pointer */
 		if (!is_reg_const(reg2, is_jmp32)) {
 			opcode = flip_opcode(opcode);
-- 
2.55.0

[PATCH bpf 6/8] selftests/bpf: check that JMP32 pointer vs zero jumps are not predicted

[Eduard Zingerman]

Add jmp32_ptr_vs_zero_jne: the fall-through of the 32-bit compare, which
the verifier used to skip, contains an out of bounds map value access,
hence w/o the previous patch the program is accepted. See previous patch
for detailed description.

Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 .../bpf/progs/verifier_jeq_infer_not_null.c   | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c b/tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c
index 8657e4a0d601..410acbf658c7 100644
--- a/tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c
+++ b/tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c
@@ -391,6 +391,33 @@ __naked void bounded_offset_infers_map_value_non_null(void)
 	: __clobber_all);
 }
 
+/*
+ * The low 32 bits of a map value pointer may be zero, hence a 32-bit
+ * compare with zero cannot be predicted from the pointer being non-NULL
+ * and both successors of such a jump have to be verified.
+ */
+SEC("socket")
+__failure __msg("invalid access to map value, value_size=4 off=32 size=4")
+__naked void jmp32_ptr_vs_zero_jne(void)
+{
+	asm volatile ("					\
+	/* r0 = bpf_map_lookup_elem(map_hash, &key); */	\
+	*(u64 *)(r10 - 8) = 0;				\
+	r1 = %[map_hash] ll;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	call %[bpf_map_lookup_elem];			\
+	if r0 == 0 goto 1f;				\
+	if w0 != 0 goto 1f;				\
+	r0 = *(u32 *)(r0 + 32);				\
+1:	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(map_hash)
+	: __clobber_all);
+}
+
 void kfunc_root(void)
 {
 	bpf_rdonly_cast(0, 0);
-- 
2.55.0

[PATCH bpf 7/8] bpf: mark the zero register precise for a register-form NULL check

[Eduard Zingerman]

check_cond_jmp_op() accepts "if rA <op> rB" as a NULL check for a
nullable pointer rA when rB is a scalar known to be zero,
lifts PTR_MAYBE_NULL from rA in the corresponding branch and does not
mark rB precise. Consider the following program:

  r0 = bpf_get_prandom_u32();
  r6 = 1;                  /* the r6 == 0 path is explored first */
  if (r0 == 0) goto 1f;
  r6 = 0;
1:
  r0 = bpf_map_lookup_elem(map, &0);  /* absent, NULL at runtime */
  if (r0 == r6) goto 2f;   /* taken as a NULL check for r0       */
  *(u8 *)(r0 + 0);         /* verifier: map value; runtime: zero */
2:
  return 0;

The r6 == 0 path is explored first and the dereference is accepted.
The r6 == 1 path is pruned at the checkpoint recorded for (1),
so the comparison is never verified with a non-zero r6. At runtime a
failed lookup returns NULL, NULL != 1 takes the non-NULL edge and the
program dereferences a pointer that is zero.

Fixes: 2f4cb53eed44 ("bpf: detect non null pointer with register operand in JEQ/JNE.")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 kernel/bpf/verifier.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index bb8e9efcfbad..709b4793e8eb 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -17024,6 +17024,15 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 	    type_may_be_null(dst_reg->type) &&
 	    ((BPF_SRC(insn->code) == BPF_K && insn->imm == 0) ||
 	     (BPF_SRC(insn->code) == BPF_X && bpf_register_is_null(src_reg)))) {
+		/*
+		 * For BPF_X the zero is a property of this execution path,
+		 * hence src_reg has to be precise.
+		 */
+		if (BPF_SRC(insn->code) == BPF_X) {
+			err = mark_chain_precision(env, insn->src_reg);
+			if (err)
+				return err;
+		}
 		/* Mark all identical registers in each branch as either
 		 * safe or unknown depending R == 0 or R != 0 conditional.
 		 */
-- 
2.55.0

[PATCH bpf 8/8] selftests/bpf: no non-NULL inference from an imprecise zero register

[Eduard Zingerman]

Check that a register-form NULL check does not lift PTR_MAYBE_NULL on
a path where the compared register is non-zero. W/o the previous patch
the program is accepted.

Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 .../bpf/progs/verifier_jeq_infer_not_null.c   | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c b/tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c
index 410acbf658c7..3c789c565b18 100644
--- a/tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c
+++ b/tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c
@@ -418,6 +418,40 @@ __naked void jmp32_ptr_vs_zero_jne(void)
 	: __clobber_all);
 }
 
+/*
+ * The below program is explored in two paths: r6 == 0 and r6 == 1.
+ * On the first path comparison "if r0 == r6 goto 2f" should mark r6 as precise,
+ * otherwise unsafe path with r6 == 1 would be incorrectly pruned.
+ */
+SEC("socket")
+__failure
+__flag(BPF_F_TEST_STATE_FREQ)
+__msg("error: invalid dereference of R0 (a nullable map value pointer)")
+__naked void imprecise_zero_does_not_infer_map_value_non_null(void)
+{
+	asm volatile ("					\
+	call %[bpf_get_prandom_u32];			\
+	/* r6 is 0 on the path explored first, 1 on the other */\
+	r6 = 1;						\
+	if r0 == 0 goto 1f;				\
+	r6 = 0;						\
+	/* r0 = bpf_map_lookup_elem(map_hash, &0); */	\
+1:	*(u64 *)(r10 - 8) = 0;				\
+	r1 = %[map_hash] ll;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	call %[bpf_map_lookup_elem];			\
+	if r0 == r6 goto 2f;				\
+	r0 = *(u8 *)(r0 + 0);				\
+2:	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_get_prandom_u32),
+	  __imm(bpf_map_lookup_elem),
+	  __imm_addr(map_hash)
+	: __clobber_all);
+}
+
 void kfunc_root(void)
 {
 	bpf_rdonly_cast(0, 0);
-- 
2.55.0

Re: [PATCH bpf 7/8] bpf: mark the zero register precise for a register-form NULL check

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The verifier fails to mark scalar registers precise when `bpf_register_is_null()` is used to satisfy a nullable pointer argument or kptr store.
--

commit c721af04f2965a6636b3260c058c2531da5526e0
Author: Eduard Zingerman <eddyz87@gmail.com>

bpf: mark the zero register precise for a register-form NULL check

This patch fixes a bug in check_cond_jmp_op() where comparing a nullable
pointer to a scalar known to be zero fails to mark the scalar register
precise. This prevents malicious state pruning where an imprecise scalar
could bypass safety checks and lead to a forged pointer dereference.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 303368460ec10..fde5d046b6e32 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -17052,6 +17052,15 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
>  	    type_may_be_null(dst_reg->type) &&
>  	    ((BPF_SRC(insn->code) == BPF_K && insn->imm == 0) ||
>  	     (BPF_SRC(insn->code) == BPF_X && bpf_register_is_null(src_reg)))) {
> +		/*
> +		 * For BPF_X the zero is a property of this execution path,
> +		 * hence src_reg has to be precise.
> +		 */
> +		if (BPF_SRC(insn->code) == BPF_X) {
> +			err = mark_chain_precision(env, insn->src_reg);
> +			if (err)
> +				return err;
> +		}

[Severity: Critical]
This is a pre-existing issue and not a bug introduced by this patch, but
since this change addresses missing precision tracking for zero-valued
scalar registers, do we need similar mark_chain_precision() calls in
other places that use bpf_register_is_null() to satisfy pointer checks?

For example, in check_map_kptr_access(), we bypass type checking when the
register is a scalar zero:

kernel/bpf/verifier.c:check_map_kptr_access() {
    ...
    if (!bpf_register_is_null(val_reg) &&
        map_kptr_match_type(env, kptr_field, val_reg, value_regno))
    ...
}

Similar bypasses without precision marking appear in several other argument
checkers:

kernel/bpf/verifier.c:check_func_arg() {
    ...
    if (bpf_register_is_null(reg) && type_may_be_null(arg_type))
        /* A NULL register has a SCALAR_VALUE type, so skip
         * type checking.
         */
        goto skip_type_check;
    ...
    if (type_may_be_null(arg_type) && bpf_register_is_null(reg))
        return 0;
    ...
}

kernel/bpf/verifier.c:check_kfunc_args() {
    ...
    if (bpf_register_is_null(reg) && type_may_be_null(kf_arg_type))
        continue;
    ...
}

kernel/bpf/verifier.c:check_mem_reg() {
    ...
    if (bpf_register_is_null(reg))
        return 0;
    ...
}

Could an attacker use these paths in a similar way to the vulnerability
described in your commit message? 

If a branch assigns a scalar 0, passes it to one of these functions
(leaving it imprecise), and another branch merges with an arbitrary kernel
address, wouldn't states_equal() incorrectly prune the malicious branch
because regsafe() returns true for imprecise scalars?

>  		/* Mark all identical registers in each branch as either
>  		 * safe or unknown depending R == 0 or R != 0 conditional.
>  		 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904083325.2083493-1-eddyz87@gmail.com?part=7

Re: [PATCH bpf 1/8] bpf: don't infer non-NULL from a pointer with an unbounded offset

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf.git (master)
by Kumar Kartikeya Dwivedi <memxor@gmail.com>:

On Fri,  4 Sep 2026 01:33:18 -0700 you wrote:
> reg_not_null() decides that a register holds a non-NULL value by
> looking at its type alone. For pointer types that allow arithmetic the
> type only guarantees a non-NULL base, in case of an unbound offset
> the runtime offset value might still add up to NULL.
> Consider the followng program:
> 
>   r6 = bpf_map_lookup_elem(map, &0);  /* present */
>   if (r6 == 0) return 0;
>   r7 = bpf_map_lookup_elem(map, &1);  /* absent, NULL at runtime */
>   r8 = r7;
>   r8 -= r6;     /* pointer - pointer: unknown scalar, -r6 */
>   r8 <<= 1;
>   r8 >>= 1;     /* any non-negative offset is accepted by */
>                 /* check_reg_sane_offset_ptr() */
>   r6 += r8;     /* verifier: map value;    runtime: zero  */
>   if (r7 != r6) return 0;
>   *(u8 *)(r7 + 0);  /* r7 is inferred non-NULL, both are zero */
> 
> [...]

Here is the summary with links:
  - [bpf,1/8] bpf: don't infer non-NULL from a pointer with an unbounded offset
    https://git.kernel.org/bpf/bpf/c/67b529f521a6
  - [bpf,2/8] selftests/bpf: no non-NULL inference from unbounded offset pointers
    https://git.kernel.org/bpf/bpf/c/6752b90ccfb3
  - [bpf,3/8] bpf: don't resurrect a scalar id dropped by collect_linked_regs()
    https://git.kernel.org/bpf/bpf/c/73a98f96811e
  - [bpf,4/8] selftests/bpf: check the linked regs cap for the compared register
    https://git.kernel.org/bpf/bpf/c/bc412b3fb185
  - [bpf,5/8] bpf: don't predict JMP32 pointer vs zero comparisons
    https://git.kernel.org/bpf/bpf/c/e51179a4e098
  - [bpf,6/8] selftests/bpf: check that JMP32 pointer vs zero jumps are not predicted
    https://git.kernel.org/bpf/bpf/c/836b2fe544a5
  - [bpf,7/8] bpf: mark the zero register precise for a register-form NULL check
    https://git.kernel.org/bpf/bpf/c/6aed0134d3cd
  - [bpf,8/8] selftests/bpf: no non-NULL inference from an imprecise zero register
    https://git.kernel.org/bpf/bpf/c/6b31560c6bc1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH bpf 7/8] bpf: mark the zero register precise for a register-form NULL check

[Kumar Kartikeya Dwivedi]

On Fri Sep 4, 2026 at 10:57 AM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] The verifier fails to mark scalar registers precise when `bpf_register_is_null()` is used to satisfy a nullable pointer argument or kptr store.
> --
>
> commit c721af04f2965a6636b3260c058c2531da5526e0
> Author: Eduard Zingerman <eddyz87@gmail.com>
>
> bpf: mark the zero register precise for a register-form NULL check
>
> This patch fixes a bug in check_cond_jmp_op() where comparing a nullable
> pointer to a scalar known to be zero fails to mark the scalar register
> precise. This prevents malicious state pruning where an imprecise scalar
> could bypass safety checks and lead to a forged pointer dereference.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 303368460ec10..fde5d046b6e32 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -17052,6 +17052,15 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
>>  	    type_may_be_null(dst_reg->type) &&
>>  	    ((BPF_SRC(insn->code) == BPF_K && insn->imm == 0) ||
>>  	     (BPF_SRC(insn->code) == BPF_X && bpf_register_is_null(src_reg)))) {
>> +		/*
>> +		 * For BPF_X the zero is a property of this execution path,
>> +		 * hence src_reg has to be precise.
>> +		 */
>> +		if (BPF_SRC(insn->code) == BPF_X) {
>> +			err = mark_chain_precision(env, insn->src_reg);
>> +			if (err)
>> +				return err;
>> +		}
>
> [Severity: Critical]
> This is a pre-existing issue and not a bug introduced by this patch, but
> since this change addresses missing precision tracking for zero-valued
> scalar registers, do we need similar mark_chain_precision() calls in
> other places that use bpf_register_is_null() to satisfy pointer checks?
>
> For example, in check_map_kptr_access(), we bypass type checking when the
> register is a scalar zero:
>
> kernel/bpf/verifier.c:check_map_kptr_access() {
>     ...
>     if (!bpf_register_is_null(val_reg) &&
>         map_kptr_match_type(env, kptr_field, val_reg, value_regno))
>     ...
> }

This should be addressed by the following fix:
https://lore.kernel.org/bpf/20260904104203.345917-6-memxor@gmail.com.

>
> Similar bypasses without precision marking appear in several other argument
> checkers:
>
> kernel/bpf/verifier.c:check_func_arg() {
>     ...
>     if (bpf_register_is_null(reg) && type_may_be_null(arg_type))
>         /* A NULL register has a SCALAR_VALUE type, so skip
>          * type checking.
>          */
>         goto skip_type_check;
>     ...
>     if (type_may_be_null(arg_type) && bpf_register_is_null(reg))
>         return 0;
>     ...
> }
>
> kernel/bpf/verifier.c:check_kfunc_args() {
>     ...
>     if (bpf_register_is_null(reg) && type_may_be_null(kf_arg_type))
>         continue;
>     ...
> }
>
> kernel/bpf/verifier.c:check_mem_reg() {
>     ...
>     if (bpf_register_is_null(reg))
>         return 0;
>     ...
> }
>

The rest need investigation...

> Could an attacker use these paths in a similar way to the vulnerability
> described in your commit message?
>
> If a branch assigns a scalar 0, passes it to one of these functions
> (leaving it imprecise), and another branch merges with an arbitrary kernel
> address, wouldn't states_equal() incorrectly prune the malicious branch
> because regsafe() returns true for imprecise scalars?
>
>>  		/* Mark all identical registers in each branch as either
>>  		 * safe or unknown depending R == 0 or R != 0 conditional.
>>  		 */