Commit 8374bfd5 authored by Hao Sun's avatar Hao Sun Committed by Martin KaFai Lau

bpf: fix nullness propagation for reg to reg comparisons

After befae758, the verifier would propagate null information after
JEQ/JNE, e.g., if two pointers, one is maybe_null and the other is not,
the former would be marked as non-null in eq path. However, as comment
"PTR_TO_BTF_ID points to a kernel struct that does not need to be null
checked by the BPF program ... The verifier must keep this in mind and
can make no assumptions about null or non-null when doing branch ...".
If one pointer is maybe_null and the other is PTR_TO_BTF, the former is
incorrectly marked non-null. The following BPF prog can trigger a
null-ptr-deref, also see this report for more details[1]:

	0: (18) r1 = map_fd	        ; R1_w=map_ptr(ks=4, vs=4)
	2: (79) r6 = *(u64 *)(r1 +8)    ; R6_w=bpf_map->inner_map_data
					; R6 is PTR_TO_BTF_ID
					; equals to null at runtime
	3: (bf) r2 = r10
	4: (07) r2 += -4
	5: (62) *(u32 *)(r2 +0) = 0
	6: (85) call bpf_map_lookup_elem#1    ; R0_w=map_value_or_null
	7: (1d) if r6 == r0 goto pc+1
	8: (95) exit
	; from 7 to 9: R0=map_value R6=ptr_bpf_map
	9: (61) r0 = *(u32 *)(r0 +0)          ; null-ptr-deref
	10: (95) exit

So, make the verifier propagate nullness information for reg to reg
comparisons only if neither reg is PTR_TO_BTF_ID.

[1] https://lore.kernel.org/bpf/CACkBjsaFJwjC5oiw-1KXvcazywodwXo4zGYsRHwbr2gSG9WcSw@mail.gmail.com/T/#u

Fixes: befae758 ("bpf: propagate nullness information for reg to reg comparisons")
Signed-off-by: default avatarHao Sun <sunhao.th@gmail.com>
Acked-by: default avatarYonghong Song <yhs@fb.com>
Link: https://lore.kernel.org/r/20221222024414.29539-1-sunhao.th@gmail.comSigned-off-by: default avatarMartin KaFai Lau <martin.lau@kernel.org>
parent 55171f29
...@@ -11822,10 +11822,17 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, ...@@ -11822,10 +11822,17 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
* register B - not null * register B - not null
* for JNE A, B, ... - A is not null in the false branch; * for JNE A, B, ... - A is not null in the false branch;
* for JEQ A, B, ... - A is not null in the true branch. * for JEQ A, B, ... - A is not null in the true branch.
*
* Since PTR_TO_BTF_ID points to a kernel struct that does
* not need to be null checked by the BPF program, i.e.,
* could be null even without PTR_MAYBE_NULL marking, so
* only propagate nullness when neither reg is that type.
*/ */
if (!is_jmp32 && BPF_SRC(insn->code) == BPF_X && if (!is_jmp32 && BPF_SRC(insn->code) == BPF_X &&
__is_pointer_value(false, src_reg) && __is_pointer_value(false, dst_reg) && __is_pointer_value(false, src_reg) && __is_pointer_value(false, dst_reg) &&
type_may_be_null(src_reg->type) != type_may_be_null(dst_reg->type)) { type_may_be_null(src_reg->type) != type_may_be_null(dst_reg->type) &&
base_type(src_reg->type) != PTR_TO_BTF_ID &&
base_type(dst_reg->type) != PTR_TO_BTF_ID) {
eq_branch_regs = NULL; eq_branch_regs = NULL;
switch (opcode) { switch (opcode) {
case BPF_JEQ: case BPF_JEQ:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment