[PATCH bpf-next] bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()

[PATCH bpf-next] bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()

[Rahad Bhuiya]

check_cond_jmp_op() propagates PTR_MAYBE_NULL nullness through
mark_ptr_or_null_regs() when it detects a BPF_JEQ / BPF_JNE
comparison against zero.  The existing code handles only the
'forward' operand order:

  dst_reg = nullable pointer,  src_reg = known-zero scalar
  (e.g. 'if r0 == 0')

The symmetric case is left unhandled:

  dst_reg = known-zero scalar,  src_reg = nullable pointer
  (e.g. 'if r6 == r0' where r6 has been proven zero)

When the symmetric case occurs:
 - type_may_be_null(dst_reg->type) is false (dst is scalar zero)
 - The null-check block is skipped entirely
 - mark_ptr_or_null_regs() is never called for src_reg
 - src_reg retains PTR_MAYBE_NULL in both branches

This causes the verifier to either reject a valid program that guards
via the reversed form, or -- if the check is a control dependency that
the verifier is expected to honour -- to silently miss enforcing the
null guard requirement.

Fix: add an 'else if' branch that mirrors the forward logic for the
symmetric operand order.  Like the forward BPF_X case, dst_reg (the
zero scalar) must be marked precise before nullness propagation since
the zero is a property of this execution path.

Commit 6aed0134d3cd ('bpf: Mark the zero register precise for a
register-form NULL check') introduced the precision marking for the
forward case and mentioned it also applies to BPF_X zero-register
comparisons; the symmetric case was overlooked there.

Signed-off-by: Rahad Bhuiya <rahadbhuiya2021@gmail.com>
---
 kernel/bpf/verifier.c                         |  20 +++
 .../selftests/bpf/prog_tests/verifier.c       |   3 +
 .../bpf/progs/verifier_null_ptr_symmetric.c   | 162 ++++++++++++++++++
 3 files changed, 185 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 353bde9ae227..e4ae8f648162 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -18248,6 +18248,26 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 				      opcode == BPF_JNE);
 		mark_ptr_or_null_regs(other_branch, insn->dst_reg,
 				      opcode == BPF_JEQ);
+	} else if (!is_jmp32 && BPF_SRC(insn->code) == BPF_X &&
+		   (opcode == BPF_JEQ || opcode == BPF_JNE) &&
+		   type_may_be_null(src_reg->type) &&
+		   bpf_register_is_null(dst_reg)) {
+		/* Symmetric case: src_reg is the nullable pointer, dst_reg is
+		 * a register whose value is known to be zero.  This arises when
+		 * the programmer writes the comparison in reversed operand order,
+		 * e.g. "if (0 == map_val)" or "if (r0 == map_val)" where r0 has
+		 * been proven zero by the verifier.
+		 *
+		 * The zero is a property of this execution path, so dst_reg
+		 * must be marked precise before we propagate nullness.
+		 */
+		err = mark_chain_precision(env, insn->dst_reg);
+		if (err)
+			return err;
+		mark_ptr_or_null_regs(this_branch, insn->src_reg,
+				      opcode == BPF_JNE);
+		mark_ptr_or_null_regs(other_branch, insn->src_reg,
+				      opcode == BPF_JEQ);
 	} else if (!try_match_pkt_pointers(insn, dst_reg, &regs[insn->src_reg],
 					   this_branch, other_branch) &&
 		   is_pointer_value(env, insn->dst_reg)) {
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 460ad10ddc02..834ee5da9845 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -144,6 +144,7 @@
 #include "irq.skel.h"
 #include "verifier_ctx_ptr_param.skel.h"
 #include "verifier_zext.skel.h"
+#include "verifier_null_ptr_symmetric.skel.h"
 
 #define MAX_ENTRIES 11
 
@@ -377,3 +378,5 @@ void test_verifier_value_ptr_arith(void)
 		      verifier_value_ptr_arith__elf_bytes,
 		      init_value_ptr_arith_maps);
 }
+
+void test_verifier_null_ptr_symmetric(void) { RUN(verifier_null_ptr_symmetric); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c b/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c
new file mode 100644
index 000000000000..475aa3dc3bd2
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Verifier selftests for symmetric register-form NULL-pointer checks.
+ *
+ * The BPF verifier's check_cond_jmp_op() handles "if R == 0" NULL checks by
+ * calling mark_ptr_or_null_regs() to mark the nullable pointer as either safe
+ * or unknown in each branch.  Historically only the forward case was covered:
+ *   dst_reg = nullable pointer,  src_reg = known-zero scalar
+ * i.e. "if (map_val == 0)".
+ *
+ * The symmetric case was missing:
+ *   dst_reg = known-zero scalar, src_reg = nullable pointer
+ * i.e. "if (0 == map_val)" or "if (r0 == map_val)" where r0 is proven zero.
+ *
+ * Without the fix the verifier leaves src_reg as PTR_MAYBE_NULL in both
+ * branches, either rejecting a valid program or silently missing a required
+ * null check.  With the fix, nullness is propagated correctly in both
+ * directions.
+ */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(max_entries, 1);
+	__type(key, int);
+	__type(value, int);
+} sym_null_map SEC(".maps");
+
+/* ---- Forward case (already worked before the fix) ---- */
+
+/*
+ * "if (map_val == 0)" -- dst_reg holds the nullable pointer,
+ * src_reg is the literal-zero immediate (BPF_K).  Should be accepted.
+ */
+SEC("socket")
+__description("null check: forward form (dst nullable, imm 0) accepted")
+__success __retval(0)
+__naked void null_check_forward_imm(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 8) = r1;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r1 = %[sym_null_map] ll;			\
+	call %[bpf_map_lookup_elem];			\
+	/* r0 = nullable ptr; forward case: if (r0 == 0) */	\
+	if r0 == 0 goto exit;				\
+	/* r0 is non-null here -- safe to dereference */\
+	r1 = *(u32 *)(r0 + 0);				\
+exit:							\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(sym_null_map)
+	: __clobber_all);
+}
+
+/* ---- Symmetric case (required the new fix) ---- */
+
+/*
+ * "if (0 == map_val)" -- src_reg holds the nullable pointer,
+ * dst_reg is a register that the verifier has proven equals zero.
+ * Should be accepted; before the fix it was rejected because nullness
+ * was never propagated for src_reg.
+ */
+SEC("socket")
+__description("null check: symmetric form (src nullable, dst zero reg) accepted")
+__success __retval(0)
+__naked void null_check_symmetric_reg(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 8) = r1;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r1 = %[sym_null_map] ll;			\
+	call %[bpf_map_lookup_elem];			\
+	/* r0 = nullable ptr, r6 = known 0 */		\
+	r6 = 0;						\
+	/* Symmetric check: if (r6 == r0), i.e. dst=r6 (zero), src=r0 (nullable) */\
+	if r6 == r0 goto exit;				\
+	/* r0 is non-null here -- safe to dereference */\
+	r1 = *(u32 *)(r0 + 0);				\
+exit:							\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(sym_null_map)
+	: __clobber_all);
+}
+
+/*
+ * BPF_JNE symmetric form: "if (r6 != r0) goto use_r0;"
+ * In the fall-through branch r0 == r6 == 0, so it is null.
+ * In the taken branch r0 is non-null and safe to dereference.
+ */
+SEC("socket")
+__description("null check: symmetric JNE form (src nullable, dst zero reg) accepted")
+__success __retval(0)
+__naked void null_check_symmetric_jne(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 8) = r1;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r1 = %[sym_null_map] ll;			\
+	call %[bpf_map_lookup_elem];			\
+	r6 = 0;						\
+	/* if (r6 != r0) goto use_r0 */			\
+	if r6 != r0 goto use_r0;			\
+	/* fall-through: r0 is null here, just exit */	\
+	r0 = 0;						\
+	exit;						\
+use_r0:							\
+	/* r0 is non-null here -- safe to dereference */\
+	r1 = *(u32 *)(r0 + 0);				\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(sym_null_map)
+	: __clobber_all);
+}
+
+/*
+ * Negative test: src_reg is nullable, dst_reg is non-zero scalar.
+ * This must NOT be treated as a null check -- the verifier should
+ * still reject a dereference of r0 without a proper null guard.
+ */
+SEC("socket")
+__description("null check: symmetric form with non-zero dst must not elide null check")
+__failure __msg("R0 invalid mem access 'map_value_or_null'")
+__naked void null_check_symmetric_nonzero_dst_rejected(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 8) = r1;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r1 = %[sym_null_map] ll;			\
+	call %[bpf_map_lookup_elem];			\
+	/* r6 = 1 (non-zero) -- this is NOT a null check */\
+	r6 = 1;						\
+	if r6 == r0 goto skip;				\
+	/* r0 still PTR_MAYBE_NULL here, dereference must fail */\
+	r1 = *(u32 *)(r0 + 0);				\
+skip:							\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(sym_null_map)
+	: __clobber_all);
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.55.0.windows.3

Re: [PATCH bpf-next] bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] bpf: symmetric NULL check fix is unreachable for unprivileged programs
- [Low] bpf: multi-line comments violate subsystem style guidelines
--

commit e76939eba1acc9b00715cb7f09c40f2d50cb02ae
Author: Rahad Bhuiya <rahadbhuiya2021@gmail.com>

bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()
    
This commit addresses an issue in the BPF verifier where symmetric NULL pointer
checks (e.g., if (0 == ptr)) were not propagating the nullness state correctly.
It adds logic to handle the case where the destination register is zero and
the source register is a nullable pointer, updating the branches accordingly.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 353bde9ae2274..e4ae8f6481620 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -18248,6 +18248,26 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
>  				      opcode == BPF_JNE);
>  		mark_ptr_or_null_regs(other_branch, insn->dst_reg,
>  				      opcode == BPF_JEQ);
> +	} else if (!is_jmp32 && BPF_SRC(insn->code) == BPF_X &&
> +		   (opcode == BPF_JEQ || opcode == BPF_JNE) &&
> +		   type_may_be_null(src_reg->type) &&
> +		   bpf_register_is_null(dst_reg)) {

[Severity: Medium]
Will this fix be reachable for unprivileged BPF programs?

In unprivileged mode (env->allow_ptr_leaks == false), an early security check
in check_cond_jmp_op() rejects conditional jumps where src_reg is a pointer,
unless both operands are packet pointers:

    if (!(reg_is_pkt_pointer_any(dst_reg) && reg_is_pkt_pointer_any(src_reg)) &&
        is_pointer_value(env, insn->src_reg)) {
        verbose(env, "R%d pointer comparison prohibited\n", insn->src_reg);
        return -EACCES;
    }

Because this check happens before the newly added block, unprivileged BPF
programs attempting symmetric NULL checks might still be unconditionally
rejected, rendering the fix incomplete for those users.

> +		/* Symmetric case: src_reg is the nullable pointer, dst_reg is
> +		 * a register whose value is known to be zero.  This arises when

[Severity: Low]
This isn't a bug, but does this multi-line comment violate the BPF subsystem
style guidelines?

The BPF guidelines mandate that multi-line comments must have the opening /*
on its own line, with the comment text starting on the subsequent line.

> +		 * the programmer writes the comparison in reversed operand order,
> +		 * e.g. "if (0 == map_val)" or "if (r0 == map_val)" where r0 has
> +		 * been proven zero by the verifier.
> +		 *
> +		 * The zero is a property of this execution path, so dst_reg
> +		 * must be marked precise before we propagate nullness.
> +		 */
> +		err = mark_chain_precision(env, insn->dst_reg);
> +		if (err)
> +			return err;

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c b/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c
> new file mode 100644
> index 0000000000000..475aa3dc3bd25
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Verifier selftests for symmetric register-form NULL-pointer checks.
> + *
> + * The BPF verifier's check_cond_jmp_op() handles "if R == 0" NULL checks by

[Severity: Low]
This isn't a bug, but does this comment also violate the multi-line comment
style for the BPF subsystem?

The opening /* should be on its own line, with the text beginning on the
subsequent line.

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

[PATCH bpf-next v2] bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()

[Rahad Bhuiya]

check_cond_jmp_op() propagates PTR_MAYBE_NULL nullness through
mark_ptr_or_null_regs() when it detects a BPF_JEQ / BPF_JNE
comparison against zero.  The existing code handles only the
'forward' operand order:

  dst_reg = nullable pointer,  src_reg = known-zero scalar
  (e.g. 'if r0 == 0')

The symmetric case is left unhandled:

  dst_reg = known-zero scalar,  src_reg = nullable pointer
  (e.g. 'if r6 == r0' where r6 has been proven zero)

When the symmetric case occurs:
 - type_may_be_null(dst_reg->type) is false (dst is scalar zero)
 - The null-check block is skipped entirely
 - mark_ptr_or_null_regs() is never called for src_reg
 - src_reg retains PTR_MAYBE_NULL in both branches

Additionally, the early unprivileged-mode pointer-comparison guard
(line 18058) rejected any conditional jump where src_reg is a pointer
type, which would have made the new block unreachable for unprivileged
programs.  Comparing a nullable pointer against a register proven to
be zero is not a pointer leak -- it is semantically identical to the
already-permitted forward form 'if (r_nullable == 0)'.  Exempt the
symmetric null check from that guard.

Fix: add an 'else if' branch that mirrors the forward logic for the
symmetric operand order and extend the early guard to permit it.

Commit 6aed0134d3cd ('bpf: Mark the zero register precise for a
register-form NULL check') introduced the precision marking for the
forward case; the symmetric case was overlooked there.

Signed-off-by: Rahad Bhuiya <rahadbhuiya2021@gmail.com>
---
 kernel/bpf/verifier.c                         |  40 ++++-
 .../selftests/bpf/prog_tests/verifier.c       |   3 +
 .../bpf/progs/verifier_null_ptr_symmetric.c   | 163 ++++++++++++++++++
 3 files changed, 203 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 353bde9ae227..68b23d11766d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -18057,9 +18057,22 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 		src_reg = &regs[insn->src_reg];
 		if (!(reg_is_pkt_pointer_any(dst_reg) && reg_is_pkt_pointer_any(src_reg)) &&
 		    is_pointer_value(env, insn->src_reg)) {
-			verbose(env, "R%d pointer comparison prohibited\n",
-				insn->src_reg);
-			return -EACCES;
+			/*
+			 * Symmetric NULL check: "if (r_zero == r_nullable)".
+			 * Comparing a nullable pointer against a register proven
+			 * to be zero is not a pointer leak -- it is the reversed
+			 * form of the already-permitted "if (r_nullable == 0)".
+			 * Permit it; reject every other src_reg pointer comparison.
+			 */
+			if (BPF_CLASS(insn->code) == BPF_JMP32 ||
+			    (BPF_OP(insn->code) != BPF_JEQ &&
+			     BPF_OP(insn->code) != BPF_JNE) ||
+			    !type_may_be_null(src_reg->type) ||
+			    !bpf_register_is_null(dst_reg)) {
+				verbose(env, "R%d pointer comparison prohibited\n",
+					insn->src_reg);
+				return -EACCES;
+			}
 		}
 
 		if (src_reg->type == PTR_TO_STACK)
@@ -18248,6 +18261,27 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 				      opcode == BPF_JNE);
 		mark_ptr_or_null_regs(other_branch, insn->dst_reg,
 				      opcode == BPF_JEQ);
+	} else if (!is_jmp32 && BPF_SRC(insn->code) == BPF_X &&
+		   (opcode == BPF_JEQ || opcode == BPF_JNE) &&
+		   type_may_be_null(src_reg->type) &&
+		   bpf_register_is_null(dst_reg)) {
+		/*
+		 * Symmetric case: src_reg is the nullable pointer, dst_reg is
+		 * a register whose value is known to be zero.  This arises when
+		 * the programmer writes the comparison in reversed operand order,
+		 * e.g. "if (0 == map_val)" or "if (r0 == map_val)" where r0 has
+		 * been proven zero by the verifier.
+		 *
+		 * The zero is a property of this execution path, so dst_reg
+		 * must be marked precise before we propagate nullness.
+		 */
+		err = mark_chain_precision(env, insn->dst_reg);
+		if (err)
+			return err;
+		mark_ptr_or_null_regs(this_branch, insn->src_reg,
+				      opcode == BPF_JNE);
+		mark_ptr_or_null_regs(other_branch, insn->src_reg,
+				      opcode == BPF_JEQ);
 	} else if (!try_match_pkt_pointers(insn, dst_reg, &regs[insn->src_reg],
 					   this_branch, other_branch) &&
 		   is_pointer_value(env, insn->dst_reg)) {
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 460ad10ddc02..834ee5da9845 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -144,6 +144,7 @@
 #include "irq.skel.h"
 #include "verifier_ctx_ptr_param.skel.h"
 #include "verifier_zext.skel.h"
+#include "verifier_null_ptr_symmetric.skel.h"
 
 #define MAX_ENTRIES 11
 
@@ -377,3 +378,5 @@ void test_verifier_value_ptr_arith(void)
 		      verifier_value_ptr_arith__elf_bytes,
 		      init_value_ptr_arith_maps);
 }
+
+void test_verifier_null_ptr_symmetric(void) { RUN(verifier_null_ptr_symmetric); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c b/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c
new file mode 100644
index 000000000000..3adf28402d73
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c
@@ -0,0 +1,163 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Verifier selftests for symmetric register-form NULL-pointer checks.
+ *
+ * The BPF verifier's check_cond_jmp_op() handles "if R == 0" NULL checks by
+ * calling mark_ptr_or_null_regs() to mark the nullable pointer as either safe
+ * or unknown in each branch.  Historically only the forward case was covered:
+ *   dst_reg = nullable pointer,  src_reg = known-zero scalar
+ * i.e. "if (map_val == 0)".
+ *
+ * The symmetric case was missing:
+ *   dst_reg = known-zero scalar, src_reg = nullable pointer
+ * i.e. "if (0 == map_val)" or "if (r0 == map_val)" where r0 is proven zero.
+ *
+ * Without the fix the verifier leaves src_reg as PTR_MAYBE_NULL in both
+ * branches, either rejecting a valid program or silently missing a required
+ * null check.  With the fix, nullness is propagated correctly in both
+ * directions.
+ */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(max_entries, 1);
+	__type(key, int);
+	__type(value, int);
+} sym_null_map SEC(".maps");
+
+/* ---- Forward case (already worked before the fix) ---- */
+
+/*
+ * "if (map_val == 0)" -- dst_reg holds the nullable pointer,
+ * src_reg is the literal-zero immediate (BPF_K).  Should be accepted.
+ */
+SEC("socket")
+__description("null check: forward form (dst nullable, imm 0) accepted")
+__success __retval(0)
+__naked void null_check_forward_imm(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 8) = r1;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r1 = %[sym_null_map] ll;			\
+	call %[bpf_map_lookup_elem];			\
+	/* r0 = nullable ptr; forward case: if (r0 == 0) */	\
+	if r0 == 0 goto exit;				\
+	/* r0 is non-null here -- safe to dereference */\
+	r1 = *(u32 *)(r0 + 0);				\
+exit:							\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(sym_null_map)
+	: __clobber_all);
+}
+
+/* ---- Symmetric case (required the new fix) ---- */
+
+/*
+ * "if (0 == map_val)" -- src_reg holds the nullable pointer,
+ * dst_reg is a register that the verifier has proven equals zero.
+ * Should be accepted; before the fix it was rejected because nullness
+ * was never propagated for src_reg.
+ */
+SEC("socket")
+__description("null check: symmetric form (src nullable, dst zero reg) accepted")
+__success __retval(0)
+__naked void null_check_symmetric_reg(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 8) = r1;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r1 = %[sym_null_map] ll;			\
+	call %[bpf_map_lookup_elem];			\
+	/* r0 = nullable ptr, r6 = known 0 */		\
+	r6 = 0;						\
+	/* Symmetric check: if (r6 == r0), i.e. dst=r6 (zero), src=r0 (nullable) */\
+	if r6 == r0 goto exit;				\
+	/* r0 is non-null here -- safe to dereference */\
+	r1 = *(u32 *)(r0 + 0);				\
+exit:							\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(sym_null_map)
+	: __clobber_all);
+}
+
+/*
+ * BPF_JNE symmetric form: "if (r6 != r0) goto use_r0;"
+ * In the fall-through branch r0 == r6 == 0, so it is null.
+ * In the taken branch r0 is non-null and safe to dereference.
+ */
+SEC("socket")
+__description("null check: symmetric JNE form (src nullable, dst zero reg) accepted")
+__success __retval(0)
+__naked void null_check_symmetric_jne(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 8) = r1;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r1 = %[sym_null_map] ll;			\
+	call %[bpf_map_lookup_elem];			\
+	r6 = 0;						\
+	/* if (r6 != r0) goto use_r0 */			\
+	if r6 != r0 goto use_r0;			\
+	/* fall-through: r0 is null here, just exit */	\
+	r0 = 0;						\
+	exit;						\
+use_r0:							\
+	/* r0 is non-null here -- safe to dereference */\
+	r1 = *(u32 *)(r0 + 0);				\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(sym_null_map)
+	: __clobber_all);
+}
+
+/*
+ * Negative test: src_reg is nullable, dst_reg is non-zero scalar.
+ * This must NOT be treated as a null check -- the verifier should
+ * still reject a dereference of r0 without a proper null guard.
+ */
+SEC("socket")
+__description("null check: symmetric form with non-zero dst must not elide null check")
+__failure __msg("R0 invalid mem access 'map_value_or_null'")
+__naked void null_check_symmetric_nonzero_dst_rejected(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 8) = r1;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r1 = %[sym_null_map] ll;			\
+	call %[bpf_map_lookup_elem];			\
+	/* r6 = 1 (non-zero) -- this is NOT a null check */\
+	r6 = 1;						\
+	if r6 == r0 goto skip;				\
+	/* r0 still PTR_MAYBE_NULL here, dereference must fail */\
+	r1 = *(u32 *)(r0 + 0);				\
+skip:							\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(sym_null_map)
+	: __clobber_all);
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.55.0.windows.3

[PATCH bpf-next v3] bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()

[Rahad Bhuiya]

check_cond_jmp_op() propagates PTR_MAYBE_NULL nullness through
mark_ptr_or_null_regs() when it detects a BPF_JEQ / BPF_JNE
comparison against zero.  The existing code handles only the
'forward' operand order:

  dst_reg = nullable pointer,  src_reg = known-zero scalar
  (e.g. 'if r0 == 0')

The symmetric case is left unhandled:

  dst_reg = known-zero scalar,  src_reg = nullable pointer
  (e.g. 'if r6 == r0' where r6 has been proven zero)

When the symmetric case occurs:
 - type_may_be_null(dst_reg->type) is false (dst is scalar zero)
 - The null-check block is skipped entirely
 - mark_ptr_or_null_regs() is never called for src_reg
 - src_reg retains PTR_MAYBE_NULL in both branches

Additionally, the early unprivileged-mode pointer-comparison guard
(line 18058) rejected any conditional jump where src_reg is a pointer
type, which would have made the new block unreachable for unprivileged
programs.  Comparing a nullable pointer against a register proven to
be zero is not a pointer leak -- it is semantically identical to the
already-permitted forward form 'if (r_nullable == 0)'.  Exempt the
symmetric null check from that guard.

Fix: add an 'else if' branch that mirrors the forward logic for the
symmetric operand order and extend the early guard to permit it.

Commit 6aed0134d3cd ('bpf: Mark the zero register precise for a
register-form NULL check') introduced the precision marking for the
forward case; the symmetric case was overlooked there.

Signed-off-by: Rahad Bhuiya <rahadbhuiya2021@gmail.com>
---
 kernel/bpf/verifier.c                         |  40 ++++-
 .../selftests/bpf/prog_tests/verifier.c       |   3 +
 .../bpf/progs/verifier_null_ptr_symmetric.c   | 163 ++++++++++++++++++
 3 files changed, 203 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 353bde9ae227..68b23d11766d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -18057,9 +18057,22 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 		src_reg = &regs[insn->src_reg];
 		if (!(reg_is_pkt_pointer_any(dst_reg) && reg_is_pkt_pointer_any(src_reg)) &&
 		    is_pointer_value(env, insn->src_reg)) {
-			verbose(env, "R%d pointer comparison prohibited\n",
-				insn->src_reg);
-			return -EACCES;
+			/*
+			 * Symmetric NULL check: "if (r_zero == r_nullable)".
+			 * Comparing a nullable pointer against a register proven
+			 * to be zero is not a pointer leak -- it is the reversed
+			 * form of the already-permitted "if (r_nullable == 0)".
+			 * Permit it; reject every other src_reg pointer comparison.
+			 */
+			if (BPF_CLASS(insn->code) == BPF_JMP32 ||
+			    (BPF_OP(insn->code) != BPF_JEQ &&
+			     BPF_OP(insn->code) != BPF_JNE) ||
+			    !type_may_be_null(src_reg->type) ||
+			    !bpf_register_is_null(dst_reg)) {
+				verbose(env, "R%d pointer comparison prohibited\n",
+					insn->src_reg);
+				return -EACCES;
+			}
 		}
 
 		if (src_reg->type == PTR_TO_STACK)
@@ -18248,6 +18261,27 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 				      opcode == BPF_JNE);
 		mark_ptr_or_null_regs(other_branch, insn->dst_reg,
 				      opcode == BPF_JEQ);
+	} else if (!is_jmp32 && BPF_SRC(insn->code) == BPF_X &&
+		   (opcode == BPF_JEQ || opcode == BPF_JNE) &&
+		   type_may_be_null(src_reg->type) &&
+		   bpf_register_is_null(dst_reg)) {
+		/*
+		 * Symmetric case: src_reg is the nullable pointer, dst_reg is
+		 * a register whose value is known to be zero.  This arises when
+		 * the programmer writes the comparison in reversed operand order,
+		 * e.g. "if (0 == map_val)" or "if (r0 == map_val)" where r0 has
+		 * been proven zero by the verifier.
+		 *
+		 * The zero is a property of this execution path, so dst_reg
+		 * must be marked precise before we propagate nullness.
+		 */
+		err = mark_chain_precision(env, insn->dst_reg);
+		if (err)
+			return err;
+		mark_ptr_or_null_regs(this_branch, insn->src_reg,
+				      opcode == BPF_JNE);
+		mark_ptr_or_null_regs(other_branch, insn->src_reg,
+				      opcode == BPF_JEQ);
 	} else if (!try_match_pkt_pointers(insn, dst_reg, &regs[insn->src_reg],
 					   this_branch, other_branch) &&
 		   is_pointer_value(env, insn->dst_reg)) {
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 460ad10ddc02..834ee5da9845 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -144,6 +144,7 @@
 #include "irq.skel.h"
 #include "verifier_ctx_ptr_param.skel.h"
 #include "verifier_zext.skel.h"
+#include "verifier_null_ptr_symmetric.skel.h"
 
 #define MAX_ENTRIES 11
 
@@ -377,3 +378,5 @@ void test_verifier_value_ptr_arith(void)
 		      verifier_value_ptr_arith__elf_bytes,
 		      init_value_ptr_arith_maps);
 }
+
+void test_verifier_null_ptr_symmetric(void) { RUN(verifier_null_ptr_symmetric); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c b/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c
new file mode 100644
index 000000000000..e49e9918a651
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c
@@ -0,0 +1,163 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Verifier selftests for symmetric register-form NULL-pointer checks.
+ *
+ * The BPF verifier's check_cond_jmp_op() handles "if R == 0" NULL checks by
+ * calling mark_ptr_or_null_regs() to mark the nullable pointer as either safe
+ * or unknown in each branch.  Historically only the forward case was covered:
+ *   dst_reg = nullable pointer,  src_reg = known-zero scalar
+ * i.e. "if (map_val == 0)".
+ *
+ * The symmetric case was missing:
+ *   dst_reg = known-zero scalar, src_reg = nullable pointer
+ * i.e. "if (0 == map_val)" or "if (r0 == map_val)" where r0 is proven zero.
+ *
+ * Without the fix the verifier leaves src_reg as PTR_MAYBE_NULL in both
+ * branches, either rejecting a valid program or silently missing a required
+ * null check.  With the fix, nullness is propagated correctly in both
+ * directions.
+ */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(max_entries, 1);
+	__type(key, int);
+	__type(value, int);
+} sym_null_map SEC(".maps");
+
+/* ---- Forward case (already worked before the fix) ---- */
+
+/*
+ * "if (map_val == 0)" -- dst_reg holds the nullable pointer,
+ * src_reg is the literal-zero immediate (BPF_K).  Should be accepted.
+ */
+SEC("socket")
+__description("null check: forward form (dst nullable, imm 0) accepted")
+__success __retval(0)
+__naked void null_check_forward_imm(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 8) = r1;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r1 = %[sym_null_map] ll;			\
+	call %[bpf_map_lookup_elem];			\
+	/* r0 = nullable ptr; forward case: if (r0 == 0) */	\
+	if r0 == 0 goto exit_fwd;			\
+	/* r0 is non-null here -- safe to dereference */\
+	r1 = *(u32 *)(r0 + 0);				\
+exit_fwd:						\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(sym_null_map)
+	: __clobber_all);
+}
+
+/* ---- Symmetric case (required the new fix) ---- */
+
+/*
+ * "if (0 == map_val)" -- src_reg holds the nullable pointer,
+ * dst_reg is a register that the verifier has proven equals zero.
+ * Should be accepted; before the fix it was rejected because nullness
+ * was never propagated for src_reg.
+ */
+SEC("socket")
+__description("null check: symmetric form (src nullable, dst zero reg) accepted")
+__success __retval(0)
+__naked void null_check_symmetric_reg(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 8) = r1;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r1 = %[sym_null_map] ll;			\
+	call %[bpf_map_lookup_elem];			\
+	/* r0 = nullable ptr, r6 = known 0 */		\
+	r6 = 0;						\
+	/* Symmetric check: if (r6 == r0), i.e. dst=r6 (zero), src=r0 (nullable) */\
+	if r6 == r0 goto exit_sym;			\
+	/* r0 is non-null here -- safe to dereference */\
+	r1 = *(u32 *)(r0 + 0);				\
+exit_sym:						\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(sym_null_map)
+	: __clobber_all);
+}
+
+/*
+ * BPF_JNE symmetric form: "if (r6 != r0) goto use_r0;"
+ * In the fall-through branch r0 == r6 == 0, so it is null.
+ * In the taken branch r0 is non-null and safe to dereference.
+ */
+SEC("socket")
+__description("null check: symmetric JNE form (src nullable, dst zero reg) accepted")
+__success __retval(0)
+__naked void null_check_symmetric_jne(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 8) = r1;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r1 = %[sym_null_map] ll;			\
+	call %[bpf_map_lookup_elem];			\
+	r6 = 0;						\
+	/* if (r6 != r0) goto use_r0 */			\
+	if r6 != r0 goto use_r0;			\
+	/* fall-through: r0 is null here, just exit */	\
+	r0 = 0;						\
+	exit;						\
+use_r0:							\
+	/* r0 is non-null here -- safe to dereference */\
+	r1 = *(u32 *)(r0 + 0);				\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(sym_null_map)
+	: __clobber_all);
+}
+
+/*
+ * Negative test: src_reg is nullable, dst_reg is non-zero scalar.
+ * This must NOT be treated as a null check -- the verifier should
+ * still reject a dereference of r0 without a proper null guard.
+ */
+SEC("socket")
+__description("null check: symmetric form with non-zero dst must not elide null check")
+__failure __msg("R0 invalid mem access 'map_value_or_null'")
+__naked void null_check_symmetric_nonzero_dst_rejected(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 8) = r1;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r1 = %[sym_null_map] ll;			\
+	call %[bpf_map_lookup_elem];			\
+	/* r6 = 1 (non-zero) -- this is NOT a null check */\
+	r6 = 1;						\
+	if r6 == r0 goto skip;				\
+	/* r0 still PTR_MAYBE_NULL here, dereference must fail */\
+	r1 = *(u32 *)(r0 + 0);				\
+skip:							\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(sym_null_map)
+	: __clobber_all);
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.55.0.windows.3

Re: [PATCH bpf-next v3] bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3330 bytes --]

> bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()

TL;DR:
- Test issue: new selftest file may belong in verifier_jeq_infer_not_null.c,
  which already covers this path
- No unpriv test tags, so the relaxed unprivileged pointer-comparison guard
  may go untested

> diff --git a/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c b/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c
> new file mode 100644
> index 000000000000..e49e9918a651
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_null_ptr_symmetric.c
> @@ -0,0 +1,163 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Verifier selftests for symmetric register-form NULL-pointer checks.
> + *
> + * The BPF verifier's check_cond_jmp_op() handles "if R == 0" NULL checks by

[ ... ]

> +/* ---- Forward case (already worked before the fix) ---- */
> +
> +/*
> + * "if (map_val == 0)" -- dst_reg holds the nullable pointer,
> + * src_reg is the literal-zero immediate (BPF_K).  Should be accepted.
> + */
> +SEC("socket")
> +__description("null check: forward form (dst nullable, imm 0) accepted")
> +__success __retval(0)
> +__naked void null_check_forward_imm(void)

[ ... ]

> +/* ---- Symmetric case (required the new fix) ---- */
> +
> +/*
> + * "if (0 == map_val)" -- src_reg holds the nullable pointer,
> + * dst_reg is a register that the verifier has proven equals zero.
> + * Should be accepted; before the fix it was rejected because nullness
> + * was never propagated for src_reg.
> + */
> +SEC("socket")
> +__description("null check: symmetric form (src nullable, dst zero reg) accepted")
> +__success __retval(0)
> +__naked void null_check_symmetric_reg(void)

[ ... ]

A subsystem pattern flags this as potentially concerning, and it is not a
bug in the verifier change itself:  should these cases be added to an existing
test file instead of a new standalone file?

The same directory already has verifier_jeq_infer_not_null.c, which covers
this code path.  It contains jeq_reg_reg_null_check() ("r1 = 0; if r0 == r1
goto 1f;") and jne_reg_reg_null_check().  It also holds the precision
regression test from 6b31560c6bc1 ("selftests/bpf: No non-NULL inference from
an imprecise zero register") for the 6aed0134d3cd logic that this patch
mirrors.

The symmetric JEQ, JNE and negative cases would fit naturally as extra cases
in verifier_jeq_infer_not_null.c, next to their forward counterparts.  The
null_check_forward_imm case ("if r0 == 0") only re-tests long-standing
behavior that many existing tests already cover.

On its own terms the new file looks functionally correct.  The three
__success programs load in privileged mode, __retval(0) holds because the
empty hash map returns NULL, and the negative case still matches the "R0
invalid mem access 'map_value_or_null'" text.

Separately, none of the new cases has a __success_unpriv or __failure_unpriv
tag.  Does that mean test_loader only runs them in privileged mode, so the
relaxed unprivileged "pointer comparison prohibited" guard that this patch
also changes is not exercised?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/37828996894

[PATCH bpf-next v4] bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()

[Rahad Bhuiya]

check_cond_jmp_op() propagates PTR_MAYBE_NULL nullness through
mark_ptr_or_null_regs() when it detects a BPF_JEQ / BPF_JNE
comparison against zero.  The existing code handles only the
'forward' operand order:

  dst_reg = nullable pointer,  src_reg = known-zero scalar
  (e.g. 'if r0 == 0')

The symmetric case is left unhandled:

  dst_reg = known-zero scalar,  src_reg = nullable pointer
  (e.g. 'if r6 == r0' where r6 has been proven zero)

When the symmetric case occurs:
 - type_may_be_null(dst_reg->type) is false (dst is scalar zero)
 - The null-check block is skipped entirely
 - mark_ptr_or_null_regs() is never called for src_reg
 - src_reg retains PTR_MAYBE_NULL in both branches

Additionally, the early unprivileged-mode pointer-comparison guard
(line 18058) rejected any conditional jump where src_reg is a pointer
type, which would have made the new block unreachable for unprivileged
programs.  Comparing a nullable pointer against a register proven to
be zero is not a pointer leak -- it is semantically identical to the
already-permitted forward form 'if (r_nullable == 0)'.  Exempt the
symmetric null check from that guard.

Fix: add an 'else if' branch that mirrors the forward logic for the
symmetric operand order and extend the early guard to permit it.
Add corresponding symmetric JEQ, JNE, and negative non-zero dst test
cases into verifier_jeq_infer_not_null.c with __success_unpriv to
exercise both privileged and unprivileged paths.

Commit 6aed0134d3cd ('bpf: Mark the zero register precise for a
register-form NULL check') introduced the precision marking for the
forward case; the symmetric case was overlooked there.

Signed-off-by: Rahad Bhuiya <rahadbhuiya2021@gmail.com>
---
 kernel/bpf/verifier.c                         | 40 ++++++++-
 .../bpf/progs/verifier_jeq_infer_not_null.c   | 82 +++++++++++++++++++
 2 files changed, 119 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 353bde9ae227..68b23d11766d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -18057,9 +18057,22 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 		src_reg = &regs[insn->src_reg];
 		if (!(reg_is_pkt_pointer_any(dst_reg) && reg_is_pkt_pointer_any(src_reg)) &&
 		    is_pointer_value(env, insn->src_reg)) {
-			verbose(env, "R%d pointer comparison prohibited\n",
-				insn->src_reg);
-			return -EACCES;
+			/*
+			 * Symmetric NULL check: "if (r_zero == r_nullable)".
+			 * Comparing a nullable pointer against a register proven
+			 * to be zero is not a pointer leak -- it is the reversed
+			 * form of the already-permitted "if (r_nullable == 0)".
+			 * Permit it; reject every other src_reg pointer comparison.
+			 */
+			if (BPF_CLASS(insn->code) == BPF_JMP32 ||
+			    (BPF_OP(insn->code) != BPF_JEQ &&
+			     BPF_OP(insn->code) != BPF_JNE) ||
+			    !type_may_be_null(src_reg->type) ||
+			    !bpf_register_is_null(dst_reg)) {
+				verbose(env, "R%d pointer comparison prohibited\n",
+					insn->src_reg);
+				return -EACCES;
+			}
 		}
 
 		if (src_reg->type == PTR_TO_STACK)
@@ -18248,6 +18261,27 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 				      opcode == BPF_JNE);
 		mark_ptr_or_null_regs(other_branch, insn->dst_reg,
 				      opcode == BPF_JEQ);
+	} else if (!is_jmp32 && BPF_SRC(insn->code) == BPF_X &&
+		   (opcode == BPF_JEQ || opcode == BPF_JNE) &&
+		   type_may_be_null(src_reg->type) &&
+		   bpf_register_is_null(dst_reg)) {
+		/*
+		 * Symmetric case: src_reg is the nullable pointer, dst_reg is
+		 * a register whose value is known to be zero.  This arises when
+		 * the programmer writes the comparison in reversed operand order,
+		 * e.g. "if (0 == map_val)" or "if (r0 == map_val)" where r0 has
+		 * been proven zero by the verifier.
+		 *
+		 * The zero is a property of this execution path, so dst_reg
+		 * must be marked precise before we propagate nullness.
+		 */
+		err = mark_chain_precision(env, insn->dst_reg);
+		if (err)
+			return err;
+		mark_ptr_or_null_regs(this_branch, insn->src_reg,
+				      opcode == BPF_JNE);
+		mark_ptr_or_null_regs(other_branch, insn->src_reg,
+				      opcode == BPF_JEQ);
 	} else if (!try_match_pkt_pointers(insn, dst_reg, &regs[insn->src_reg],
 					   this_branch, other_branch) &&
 		   is_pointer_value(env, insn->dst_reg)) {
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 3c789c565b18..ee2e9374969d 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
@@ -273,6 +273,88 @@ __naked void jne_reg_reg_null_check(void)
         : __clobber_all);
 }
 
+/* Symmetric case: dst_reg is known 0, src_reg is nullable pointer.
+ * "if (r1 == r0) goto 1f;" (reversed operand order).
+ * Verify that non-nullness is propagated to src_reg in the non-null branch.
+ */
+SEC("socket")
+__description("null check: symmetric form (dst 0 reg, src nullable) JEQ")
+__success __success_unpriv
+__retval(0)
+__naked void jeq_reg_reg_null_check_symmetric(void)
+{
+	asm volatile ("					\
+	*(u32*)(r10 - 8) = 0;				\
+	r1 = %[map_hash] ll;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	call %[bpf_map_lookup_elem];			\
+	r1 = 0;						\
+	/* r1 is known 0, r0 is nullable ptr */		\
+	if r1 == r0 goto 1f;				\
+	r0 = *(u32*)(r0 + 0);				\
+1:	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(map_hash)
+	: __clobber_all);
+}
+
+/* Symmetric case with JNE: "if (r1 != r0) goto 1f;"
+ * Taken branch: r0 is non-null. Fall-through: r0 is null.
+ */
+SEC("socket")
+__description("null check: symmetric form (dst 0 reg, src nullable) JNE")
+__success __success_unpriv
+__retval(0)
+__naked void jne_reg_reg_null_check_symmetric(void)
+{
+	asm volatile ("					\
+	*(u32*)(r10 - 8) = 0;				\
+	r1 = %[map_hash] ll;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	call %[bpf_map_lookup_elem];			\
+	r1 = 0;						\
+	if r1 != r0 goto 1f;				\
+	goto 2f;					\
+1:	r0 = *(u32*)(r0 + 0);				\
+2:	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(map_hash)
+	: __clobber_all);
+}
+
+/* Negative test: dst_reg is a non-zero scalar (r1 = 1).
+ * "if (r1 == r0)" must NOT be treated as a null check.
+ * Dereference must fail because r0 remains nullable.
+ */
+SEC("socket")
+__description("null check: symmetric form with non-zero dst must not elide null check")
+__failure
+__msg("R0 invalid mem access 'map_value_or_null'")
+__naked void null_check_symmetric_nonzero_dst_rejected(void)
+{
+	asm volatile ("					\
+	*(u32*)(r10 - 8) = 0;				\
+	r1 = %[map_hash] ll;				\
+	r2 = r10;					\
+	r2 += -8;					\
+	call %[bpf_map_lookup_elem];			\
+	r1 = 1;						\
+	if r1 == r0 goto 1f;				\
+	r0 = *(u32*)(r0 + 0);				\
+1:	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(map_hash)
+	: __clobber_all);
+}
+
 /*
  * A comparison between PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED and
  * PTR_TO_MAP_VALUE_OR_NULL should not infer that map pointer is not null.
-- 
2.55.0.windows.3

Re: [PATCH bpf-next v4] bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3125 bytes --]

> bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()

TL;DR: Commit message only: it doesn't name a real program or compiler output
that produces the zero-in-dst register NULL check, which clang doesn't seem to
emit, plus two minor wording nits.

> bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()
>
> check_cond_jmp_op() propagates PTR_MAYBE_NULL nullness through
> mark_ptr_or_null_regs() when it detects a BPF_JEQ / BPF_JNE
> comparison against zero.  The existing code handles only the
> 'forward' operand order:
>
>   dst_reg = nullable pointer,  src_reg = known-zero scalar
>   (e.g. 'if r0 == 0')
>
> The symmetric case is left unhandled:
>
>   dst_reg = known-zero scalar,  src_reg = nullable pointer
>   (e.g. 'if r6 == r0' where r6 has been proven zero)
>
> When the symmetric case occurs:
>  - type_may_be_null(dst_reg->type) is false (dst is scalar zero)
>  - The null-check block is skipped entirely
>  - mark_ptr_or_null_regs() is never called for src_reg
>  - src_reg retains PTR_MAYBE_NULL in both branches
>
> Additionally, the early unprivileged-mode pointer-comparison guard
> (line 18058) rejected any conditional jump where src_reg is a pointer
> type, which would have made the new block unreachable for unprivileged
> programs.  Comparing a nullable pointer against a register proven to
> be zero is not a pointer leak -- it is semantically identical to the
> already-permitted forward form 'if (r_nullable == 0)'.  Exempt the
> symmetric null check from that guard.
>
> Fix: add an 'else if' branch that mirrors the forward logic for the
> symmetric operand order and extend the early guard to permit it.
> Add corresponding symmetric JEQ, JNE, and negative non-zero dst test
> cases into verifier_jeq_infer_not_null.c with __success_unpriv to
> exercise both privileged and unprivileged paths.
>
> Commit 6aed0134d3cd ('bpf: Mark the zero register precise for a
> register-form NULL check') introduced the precision marking for the
> forward case; the symmetric case was overlooked there.
>
> Signed-off-by: Rahad Bhuiya <rahadbhuiya2021@gmail.com>

This isn't a bug, but the changelog mostly walks through the code paths and
doesn't name a program or compiler output that produces the symmetric form.

Clang emits 'if rX == 0' for both 'p == 0' and '0 == p', so the zero-in-dst
register compare doesn't appear to come from ordinary C source.

Could the changelog say what real-world program or toolchain output hits
the 'if rZero == rNullable' form?

Two smaller points on the same text.  The '(line ...)' reference in the
paragraph about the early guard is a source line number that will go stale,
so could it be dropped?

Also, the forward-case example 'if r0 == 0' is the BPF_K form rather than
the register-form check this message is about.  Would an example using
'if r0 == rN' read more clearly there?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/37887809131

Re: [PATCH bpf-next v4] bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()

[Rahad Bhuiya]

Thanks for the review.

Regarding where the symmetric form arises in practice:

While standard Clang compiling high-level C source typically canonicalizes
comparisons against immediate zero into BPF_K ('if rX == 0'), the register-form
BPF_X comparison with zero in dst occurs in:

1. JIT compilers, bytecode generators, and BPF linkers/assemblers (e.g. custom
   assemblers, LLVM register-pressure spilling/copying, or runtimes like
   Aya/ebpf-go where registers are mapped dynamically).
2. Alternative compiler backends such as GCC BPF, where instruction selection
   does not guarantee commutative canonicalization for register-register
   conditional jumps.
3. Verifier consistency and completeness: commit 6aed0134d3cd explicitly
   hardened the verifier for register-register NULL checks ('if rNullable == rZero'),
   but handled only the forward operand order, leaving an asymmetrical gap
   where valid programs were rejected solely based on operand ordering.

The points regarding dropping the stale source line reference and clarifying
the register-register example ('if r0 == r1') in the commit message are well taken.
Happy to refresh the commit message in a v5 if maintainers prefer, or keep it
as-is if no further code changes are requested.

Re: [PATCH bpf-next v4] bpf: fix symmetric register-form NULL-pointer check in check_cond_jmp_op()

[Alexei Starovoitov]

On Fri, Oct 09, 2026 at 10:38 AM Rahad Bhuiya <rahadbhuiya2021@gmail.com> wrote:

Four versions in 22 hours. Not a single changelog.
v2 answered sashiko, v3 the CI build failure, v4 the bpf-ci AI review.
Pls slow down.

> The symmetric case is left unhandled:
>
>   dst_reg = known-zero scalar,  src_reg = nullable pointer
>   (e.g. 'if r6 == r0' where r6 has been proven zero)

Which compiler generates 'if r6 == r0' with zero in dst_reg?

I don't think we need this patch.


pw-bot: cr