Skip to contentSkip to conversation

bpf: don't infer non-NULL from a pointer with an unbounded offset

PATCH Applied Eduard Zingerman 4 participants 11 messages Sep 4

Subsystems (4)
lore
Share & tools
lore

Eduard Zingerman

author maintainer
#0 lore raw

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

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")
---
 kernel/bpf/verifier.c | 7 +++++++
 1 file changed, 7 insertions(+)
kernel/bpf/verifier.c +7 -0
index e64035683795..e53619e2210e 100644
@@ -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 ||
Signature · 1 line
2.55.0
author maintainer
#1 lore raw

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

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.

---
 .../bpf/progs/verifier_jeq_infer_not_null.c   | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)
tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c +80 -0
index b412a542ef76..8657e4a0d601 100644
@@ -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);
Signature · 1 line
2.55.0
author maintainer
#2 lore raw

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

In reply to 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")
---
 kernel/bpf/verifier.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
kernel/bpf/verifier.c +10 -4
index e53619e2210e..d16dd1fb08b6 100644
@@ -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)
Signature · 1 line
2.55.0
author maintainer
#3 lore raw

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

In reply to 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,...)".

---
 .../selftests/bpf/progs/verifier_scalar_ids.c       | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
tools/testing/selftests/bpf/progs/verifier_scalar_ids.c +7 -6
index 663d15fc5fd2..256547048cc4 100644
@@ -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;"
Signature · 1 line
2.55.0
author maintainer
#4 lore raw

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

In reply to 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")
---
 kernel/bpf/verifier.c | 7 +++++++
 1 file changed, 7 insertions(+)
kernel/bpf/verifier.c +7 -0
index d16dd1fb08b6..bb8e9efcfbad 100644
@@ -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);
Signature · 1 line
2.55.0
author maintainer
#5 lore raw

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

In reply to 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.

---
 .../bpf/progs/verifier_jeq_infer_not_null.c   | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)
tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c +27 -0
index 8657e4a0d601..410acbf658c7 100644
@@ -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);
Signature · 1 line
2.55.0
author maintainer
#6 lore raw

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

In reply to 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.")
---
 kernel/bpf/verifier.c | 9 +++++++++
 1 file changed, 9 insertions(+)
kernel/bpf/verifier.c +9 -0
index bb8e9efcfbad..709b4793e8eb 100644
@@ -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.
 		 */
Signature · 1 line
2.55.0
#7 lore raw

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

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

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 <[email protected]>

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.

Quoted 7 lines
kernel/bpf/verifier.c +0 -0
index 303368460ec10..fde5d046b6e32 100644
@@ -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) ||

kernel/bpf/verifier.c static int check_cond_jmp_op(struct bpf_verifier_env *env, · line 17063

Quoted 10 lines
kernel/bpf/verifier.c +9 -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?

Quoted 3 lines
kernel/bpf/verifier.c +0 -0
 		/* Mark all identical registers in each branch as either
 		 * safe or unknown depending R == 0 or R != 0 conditional.
 		 */
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
#8 lore raw

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

Quoted 43 lines · 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 <[email protected]>

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.

Quoted 7 lines
kernel/bpf/verifier.c +0 -0
index 303368460ec10..fde5d046b6e32 100644
@@ -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) ||

kernel/bpf/verifier.c static int check_cond_jmp_op(struct bpf_verifier_env *env, · line 17063

Quoted 10 lines
kernel/bpf/verifier.c +9 -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:

Quoted 6 lines
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/[email protected].

Quoted 24 lines

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;
    ...
}
Quoted 7 lines
kernel/bpf/verifier.c:check_mem_reg() {
    ...
    if (bpf_register_is_null(reg))
        return 0;
    ...
}

The rest need investigation...

Quoted 11 lines

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?

Quoted 3 lines
kernel/bpf/verifier.c +0 -0
 		/* Mark all identical registers in each branch as either
 		 * safe or unknown depending R == 0 or R != 0 conditional.
 		 */
author maintainer
#9 lore raw

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

In reply to 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.

---
 .../bpf/progs/verifier_jeq_infer_not_null.c   | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)
tools/testing/selftests/bpf/progs/verifier_jeq_infer_not_null.c +34 -0
index 410acbf658c7..3c789c565b18 100644
@@ -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);
Signature · 1 line
2.55.0
#10 lore raw
In reply to Eduard Zingerman
This series was applied to bpf/bpf.git (master) by Kumar Kartikeya Dwivedi <[email protected]>:

Hello:

This series was applied to bpf/bpf.git (master)
by Kumar Kartikeya Dwivedi <[email protected]>:

Quoted 18 lines · 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 */
Quoted 1 line

[...]

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!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html