check_cond_jmp_op() accepts "if rA 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 Suggested-by: Nicholas Carlini Signed-off-by: Eduard Zingerman --- 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