]> git.baikalelectronics.ru Git - kernel.git/commitdiff
bpf, selftests: Add test case for atomic fetch on spilled pointer
authorDaniel Borkmann <daniel@iogearbox.net>
Tue, 7 Dec 2021 10:07:04 +0000 (10:07 +0000)
committerAlexei Starovoitov <ast@kernel.org>
Wed, 15 Dec 2021 03:33:06 +0000 (19:33 -0800)
Test whether unprivileged would be able to leak the spilled pointer either
by exporting the returned value from the atomic{32,64} operation or by reading
and exporting the value from the stack after the atomic operation took place.

Note that for unprivileged, the below atomic cmpxchg test case named "Dest
pointer in r0 - succeed" is failing. The reason is that in the dst memory
location (r10 -8) there is the spilled register r10:

  0: R1=ctx(id=0,off=0,imm=0) R10=fp0
  0: (bf) r0 = r10
  1: R0_w=fp0 R1=ctx(id=0,off=0,imm=0) R10=fp0
  1: (7b) *(u64 *)(r10 -8) = r0
  2: R0_w=fp0 R1=ctx(id=0,off=0,imm=0) R10=fp0 fp-8_w=fp
  2: (b7) r1 = 0
  3: R0_w=fp0 R1_w=invP0 R10=fp0 fp-8_w=fp
  3: (db) r0 = atomic64_cmpxchg((u64 *)(r10 -8), r0, r1)
  4: R0_w=fp0 R1_w=invP0 R10=fp0 fp-8_w=mmmmmmmm
  4: (79) r1 = *(u64 *)(r0 -8)
  5: R0_w=fp0 R1_w=invP(id=0) R10=fp0 fp-8_w=mmmmmmmm
  5: (b7) r0 = 0
  6: R0_w=invP0 R1_w=invP(id=0) R10=fp0 fp-8_w=mmmmmmmm
  6: (95) exit

However, allowing this case for unprivileged is a bit useless given an
update with a new pointer will fail anyway:

  0: R1=ctx(id=0,off=0,imm=0) R10=fp0
  0: (bf) r0 = r10
  1: R0_w=fp0 R1=ctx(id=0,off=0,imm=0) R10=fp0
  1: (7b) *(u64 *)(r10 -8) = r0
  2: R0_w=fp0 R1=ctx(id=0,off=0,imm=0) R10=fp0 fp-8_w=fp
  2: (db) r0 = atomic64_cmpxchg((u64 *)(r10 -8), r0, r10)
  R10 leaks addr into mem

Acked-by: Brendan Jackman <jackmanb@google.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
tools/testing/selftests/bpf/verifier/atomic_cmpxchg.c
tools/testing/selftests/bpf/verifier/atomic_fetch.c

index c22dc83a41fdc43350285d1db6eb729fe67a7bfd..0ffc69f602af1f42e54447c0dd6aea5391b2f0b2 100644 (file)
                BPF_EXIT_INSN(),
        },
        .result = ACCEPT,
+       .result_unpriv = REJECT,
+       .errstr_unpriv = "leaking pointer from stack off -8",
+},
+{
+       "Dest pointer in r0 - succeed, check 2",
+       .insns = {
+               /* r0 = &val */
+               BPF_MOV64_REG(BPF_REG_0, BPF_REG_10),
+               /* val = r0; */
+               BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_0, -8),
+               /* r5 = &val */
+               BPF_MOV64_REG(BPF_REG_5, BPF_REG_10),
+               /* r0 = atomic_cmpxchg(&val, r0, r5); */
+               BPF_ATOMIC_OP(BPF_DW, BPF_CMPXCHG, BPF_REG_10, BPF_REG_5, -8),
+               /* r1 = *r0 */
+               BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0, -8),
+               /* exit(0); */
+               BPF_MOV64_IMM(BPF_REG_0, 0),
+               BPF_EXIT_INSN(),
+       },
+       .result = ACCEPT,
+       .result_unpriv = REJECT,
+       .errstr_unpriv = "R5 leaks addr into mem",
 },
index 3bc9ff7a860b7fa4c85ab8b79fe9b0bb19e84e88..5bf03fb4fa2b6ad6fb450f6993baeb243f853cf0 100644 (file)
@@ -1,3 +1,97 @@
+{
+       "atomic dw/fetch and address leakage of (map ptr & -1) via stack slot",
+       .insns = {
+               BPF_LD_IMM64(BPF_REG_1, -1),
+               BPF_LD_MAP_FD(BPF_REG_8, 0),
+               BPF_LD_MAP_FD(BPF_REG_9, 0),
+               BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+               BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+               BPF_STX_MEM(BPF_DW, BPF_REG_2, BPF_REG_9, 0),
+               BPF_ATOMIC_OP(BPF_DW, BPF_AND | BPF_FETCH, BPF_REG_2, BPF_REG_1, 0),
+               BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_2, 0),
+               BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+               BPF_MOV64_REG(BPF_REG_1, BPF_REG_8),
+               BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+               BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
+               BPF_STX_MEM(BPF_DW, BPF_REG_0, BPF_REG_9, 0),
+               BPF_MOV64_IMM(BPF_REG_0, 0),
+               BPF_EXIT_INSN(),
+       },
+       .fixup_map_array_48b = { 2, 4 },
+       .result = ACCEPT,
+       .result_unpriv = REJECT,
+       .errstr_unpriv = "leaking pointer from stack off -8",
+},
+{
+       "atomic dw/fetch and address leakage of (map ptr & -1) via returned value",
+       .insns = {
+               BPF_LD_IMM64(BPF_REG_1, -1),
+               BPF_LD_MAP_FD(BPF_REG_8, 0),
+               BPF_LD_MAP_FD(BPF_REG_9, 0),
+               BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+               BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+               BPF_STX_MEM(BPF_DW, BPF_REG_2, BPF_REG_9, 0),
+               BPF_ATOMIC_OP(BPF_DW, BPF_AND | BPF_FETCH, BPF_REG_2, BPF_REG_1, 0),
+               BPF_MOV64_REG(BPF_REG_9, BPF_REG_1),
+               BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+               BPF_MOV64_REG(BPF_REG_1, BPF_REG_8),
+               BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+               BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
+               BPF_STX_MEM(BPF_DW, BPF_REG_0, BPF_REG_9, 0),
+               BPF_MOV64_IMM(BPF_REG_0, 0),
+               BPF_EXIT_INSN(),
+       },
+       .fixup_map_array_48b = { 2, 4 },
+       .result = ACCEPT,
+       .result_unpriv = REJECT,
+       .errstr_unpriv = "leaking pointer from stack off -8",
+},
+{
+       "atomic w/fetch and address leakage of (map ptr & -1) via stack slot",
+       .insns = {
+               BPF_LD_IMM64(BPF_REG_1, -1),
+               BPF_LD_MAP_FD(BPF_REG_8, 0),
+               BPF_LD_MAP_FD(BPF_REG_9, 0),
+               BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+               BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+               BPF_STX_MEM(BPF_DW, BPF_REG_2, BPF_REG_9, 0),
+               BPF_ATOMIC_OP(BPF_W, BPF_AND | BPF_FETCH, BPF_REG_2, BPF_REG_1, 0),
+               BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_2, 0),
+               BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+               BPF_MOV64_REG(BPF_REG_1, BPF_REG_8),
+               BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+               BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
+               BPF_STX_MEM(BPF_DW, BPF_REG_0, BPF_REG_9, 0),
+               BPF_MOV64_IMM(BPF_REG_0, 0),
+               BPF_EXIT_INSN(),
+       },
+       .fixup_map_array_48b = { 2, 4 },
+       .result = REJECT,
+       .errstr = "invalid size of register fill",
+},
+{
+       "atomic w/fetch and address leakage of (map ptr & -1) via returned value",
+       .insns = {
+               BPF_LD_IMM64(BPF_REG_1, -1),
+               BPF_LD_MAP_FD(BPF_REG_8, 0),
+               BPF_LD_MAP_FD(BPF_REG_9, 0),
+               BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+               BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+               BPF_STX_MEM(BPF_DW, BPF_REG_2, BPF_REG_9, 0),
+               BPF_ATOMIC_OP(BPF_W, BPF_AND | BPF_FETCH, BPF_REG_2, BPF_REG_1, 0),
+               BPF_MOV64_REG(BPF_REG_9, BPF_REG_1),
+               BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+               BPF_MOV64_REG(BPF_REG_1, BPF_REG_8),
+               BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+               BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
+               BPF_STX_MEM(BPF_DW, BPF_REG_0, BPF_REG_9, 0),
+               BPF_MOV64_IMM(BPF_REG_0, 0),
+               BPF_EXIT_INSN(),
+       },
+       .fixup_map_array_48b = { 2, 4 },
+       .result = REJECT,
+       .errstr = "invalid size of register fill",
+},
 #define __ATOMIC_FETCH_OP_TEST(src_reg, dst_reg, operand1, op, operand2, expect) \
        {                                                               \
                "atomic fetch " #op ", src=" #dst_reg " dst=" #dst_reg, \