bpf: Fix extable address check.

The verifier checks that PTR_TO_BTF_ID pointer is either valid or NULL,
but it cannot distinguish IS_ERR pointer from valid one.

When offset is added to IS_ERR pointer it may become small positive
value which is a user address that is not handled by extable logic
and has to be checked for at the runtime.

Tighten BPF_PROBE_MEM pointer check code to prevent this case.

Fixes: 643d483d7b91 ("bpf: Emit explicit NULL pointer checks for PROBE_LDX instructions.")
Reported-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index fa58681db45e4d7bda6eb95081fe64b36efc650a..bafe36e69227d81e30d81ef2680ace5b8d8da39b 100644 (file)
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1252,19 +1252,54 @@ st:                     if (is_imm8(insn->off))
                case BPF_LDX | BPF_MEM | BPF_DW:
                case BPF_LDX | BPF_PROBE_MEM | BPF_DW:
                        if (BPF_MODE(insn->code) == BPF_PROBE_MEM) {
-                               /* test src_reg, src_reg */
-                               maybe_emit_mod(&prog, src_reg, src_reg, true); /* always 1 byte */
-                               EMIT2(0x85, add_2reg(0xC0, src_reg, src_reg));
-                               /* jne start_of_ldx */
-                               EMIT2(X86_JNE, 0);
+                               /* Though the verifier prevents negative insn->off in BPF_PROBE_MEM
+                                * add abs(insn->off) to the limit to make sure that negative
+                                * offset won't be an issue.
+                                * insn->off is s16, so it won't affect valid pointers.
+                                */
+                               u64 limit = TASK_SIZE_MAX + PAGE_SIZE + abs(insn->off);
+                               u8 *end_of_jmp1, *end_of_jmp2;
+
+                               /* Conservatively check that src_reg + insn->off is a kernel address:
+                                * 1. src_reg + insn->off >= limit
+                                * 2. src_reg + insn->off doesn't become small positive.
+                                * Cannot do src_reg + insn->off >= limit in one branch,
+                                * since it needs two spare registers, but JIT has only one.
+                                */
+
+                               /* movabsq r11, limit */
+                               EMIT2(add_1mod(0x48, AUX_REG), add_1reg(0xB8, AUX_REG));
+                               EMIT((u32)limit, 4);
+                               EMIT(limit >> 32, 4);
+                               /* cmp src_reg, r11 */
+                               maybe_emit_mod(&prog, src_reg, AUX_REG, true);
+                               EMIT2(0x39, add_2reg(0xC0, src_reg, AUX_REG));
+                               /* if unsigned '<' goto end_of_jmp2 */
+                               EMIT2(X86_JB, 0);
+                               end_of_jmp1 = prog;
+
+                               /* mov r11, src_reg */
+                               emit_mov_reg(&prog, true, AUX_REG, src_reg);
+                               /* add r11, insn->off */
+                               maybe_emit_1mod(&prog, AUX_REG, true);
+                               EMIT2_off32(0x81, add_1reg(0xC0, AUX_REG), insn->off);
+                               /* jmp if not carry to start_of_ldx
+                                * Otherwise ERR_PTR(-EINVAL) + 128 will be the user addr
+                                * that has to be rejected.
+                                */
+                               EMIT2(0x73 /* JNC */, 0);
+                               end_of_jmp2 = prog;
+
                                /* xor dst_reg, dst_reg */
                                emit_mov_imm32(&prog, false, dst_reg, 0);
                                /* jmp byte_after_ldx */
                                EMIT2(0xEB, 0);
 
-                               /* populate jmp_offset for JNE above */
-                               temp[4] = prog - temp - 5 /* sizeof(test + jne) */;
+                               /* populate jmp_offset for JB above to jump to xor dst_reg */
+                               end_of_jmp1[-1] = end_of_jmp2 - end_of_jmp1;
+                               /* populate jmp_offset for JNC above to jump to start_of_ldx */
                                start_of_ldx = prog;
+                               end_of_jmp2[-1] = start_of_ldx - end_of_jmp2;
                        }
                        emit_ldx(&prog, BPF_SIZE(insn->code), dst_reg, src_reg, insn->off);
                        if (BPF_MODE(insn->code) == BPF_PROBE_MEM) {