Skip to content

Commit b599d7d

Browse files
puranjaymohanAlexei Starovoitov
authored andcommitted
bpf, x86: Fix PROBE_MEM runtime load check
When a load is marked PROBE_MEM - e.g. due to PTR_UNTRUSTED access - the address being loaded from is not necessarily valid. The BPF jit sets up exception handlers for each such load which catch page faults and 0 out the destination register. If the address for the load is outside kernel address space, the load will escape the exception handling and crash the kernel. To prevent this from happening, the emits some instruction to verify that addr is > end of userspace addresses. x86 has a legacy vsyscall ABI where a page at address 0xffffffffff600000 is mapped with user accessible permissions. The addresses in this page are considered userspace addresses by the fault handler. Therefore, a BPF program accessing this page will crash the kernel. This patch fixes the runtime checks to also check that the PROBE_MEM address is below VSYSCALL_ADDR. Example BPF program: SEC("fentry/tcp_v4_connect") int BPF_PROG(fentry_tcp_v4_connect, struct sock *sk) { *(volatile unsigned long *)&sk->sk_tsq_flags; return 0; } BPF Assembly: 0: (79) r1 = *(u64 *)(r1 +0) 1: (79) r1 = *(u64 *)(r1 +344) 2: (b7) r0 = 0 3: (95) exit x86-64 JIT ========== BEFORE AFTER ------ ----- 0: nopl 0x0(%rax,%rax,1) 0: nopl 0x0(%rax,%rax,1) 5: xchg %ax,%ax 5: xchg %ax,%ax 7: push %rbp 7: push %rbp 8: mov %rsp,%rbp 8: mov %rsp,%rbp b: mov 0x0(%rdi),%rdi b: mov 0x0(%rdi),%rdi ------------------------------------------------------------------------------- f: movabs $0x100000000000000,%r11 f: movabs $0xffffffffff600000,%r10 19: add $0x2a0,%rdi 19: mov %rdi,%r11 20: cmp %r11,%rdi 1c: add $0x2a0,%r11 23: jae 0x0000000000000029 23: sub %r10,%r11 25: xor %edi,%edi 26: movabs $0x100000000a00000,%r10 27: jmp 0x000000000000002d 30: cmp %r10,%r11 29: mov 0x0(%rdi),%rdi 33: ja 0x0000000000000039 --------------------------------\ 35: xor %edi,%edi 2d: xor %eax,%eax \ 37: jmp 0x0000000000000040 2f: leave \ 39: mov 0x2a0(%rdi),%rdi 30: ret \-------------------------------------------- 40: xor %eax,%eax 42: leave 43: ret Signed-off-by: Puranjay Mohan <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
1 parent 66e13b6 commit b599d7d

File tree

1 file changed

+25
-32
lines changed

1 file changed

+25
-32
lines changed

arch/x86/net/bpf_jit_comp.c

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1807,36 +1807,41 @@ st: if (is_imm8(insn->off))
18071807
if (BPF_MODE(insn->code) == BPF_PROBE_MEM ||
18081808
BPF_MODE(insn->code) == BPF_PROBE_MEMSX) {
18091809
/* Conservatively check that src_reg + insn->off is a kernel address:
1810-
* src_reg + insn->off >= TASK_SIZE_MAX + PAGE_SIZE
1811-
* src_reg is used as scratch for src_reg += insn->off and restored
1812-
* after emit_ldx if necessary
1810+
* src_reg + insn->off > TASK_SIZE_MAX + PAGE_SIZE
1811+
* and
1812+
* src_reg + insn->off < VSYSCALL_ADDR
18131813
*/
18141814

1815-
u64 limit = TASK_SIZE_MAX + PAGE_SIZE;
1815+
u64 limit = TASK_SIZE_MAX + PAGE_SIZE - VSYSCALL_ADDR;
18161816
u8 *end_of_jmp;
18171817

1818-
/* At end of these emitted checks, insn->off will have been added
1819-
* to src_reg, so no need to do relative load with insn->off offset
1820-
*/
1821-
insn_off = 0;
1818+
/* movabsq r10, VSYSCALL_ADDR */
1819+
emit_mov_imm64(&prog, BPF_REG_AX, (long)VSYSCALL_ADDR >> 32,
1820+
(u32)(long)VSYSCALL_ADDR);
18221821

1823-
/* movabsq r11, limit */
1824-
EMIT2(add_1mod(0x48, AUX_REG), add_1reg(0xB8, AUX_REG));
1825-
EMIT((u32)limit, 4);
1826-
EMIT(limit >> 32, 4);
1822+
/* mov src_reg, r11 */
1823+
EMIT_mov(AUX_REG, src_reg);
18271824

18281825
if (insn->off) {
1829-
/* add src_reg, insn->off */
1830-
maybe_emit_1mod(&prog, src_reg, true);
1831-
EMIT2_off32(0x81, add_1reg(0xC0, src_reg), insn->off);
1826+
/* add r11, insn->off */
1827+
maybe_emit_1mod(&prog, AUX_REG, true);
1828+
EMIT2_off32(0x81, add_1reg(0xC0, AUX_REG), insn->off);
18321829
}
18331830

1834-
/* cmp src_reg, r11 */
1835-
maybe_emit_mod(&prog, src_reg, AUX_REG, true);
1836-
EMIT2(0x39, add_2reg(0xC0, src_reg, AUX_REG));
1831+
/* sub r11, r10 */
1832+
maybe_emit_mod(&prog, AUX_REG, BPF_REG_AX, true);
1833+
EMIT2(0x29, add_2reg(0xC0, AUX_REG, BPF_REG_AX));
1834+
1835+
/* movabsq r10, limit */
1836+
emit_mov_imm64(&prog, BPF_REG_AX, (long)limit >> 32,
1837+
(u32)(long)limit);
1838+
1839+
/* cmp r10, r11 */
1840+
maybe_emit_mod(&prog, AUX_REG, BPF_REG_AX, true);
1841+
EMIT2(0x39, add_2reg(0xC0, AUX_REG, BPF_REG_AX));
18371842

1838-
/* if unsigned '>=', goto load */
1839-
EMIT2(X86_JAE, 0);
1843+
/* if unsigned '>', goto load */
1844+
EMIT2(X86_JA, 0);
18401845
end_of_jmp = prog;
18411846

18421847
/* xor dst_reg, dst_reg */
@@ -1862,18 +1867,6 @@ st: if (is_imm8(insn->off))
18621867
/* populate jmp_offset for JMP above */
18631868
start_of_ldx[-1] = prog - start_of_ldx;
18641869

1865-
if (insn->off && src_reg != dst_reg) {
1866-
/* sub src_reg, insn->off
1867-
* Restore src_reg after "add src_reg, insn->off" in prev
1868-
* if statement. But if src_reg == dst_reg, emit_ldx
1869-
* above already clobbered src_reg, so no need to restore.
1870-
* If add src_reg, insn->off was unnecessary, no need to
1871-
* restore either.
1872-
*/
1873-
maybe_emit_1mod(&prog, src_reg, true);
1874-
EMIT2_off32(0x81, add_1reg(0xE8, src_reg), insn->off);
1875-
}
1876-
18771870
if (!bpf_prog->aux->extable)
18781871
break;
18791872

0 commit comments

Comments
 (0)