mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-10-31 16:38:12 +00:00
39491867ac
As pointed out by Ilya and explained in the new comment, there's a
discrepancy between x86 and BPF CMPXCHG semantics: BPF always loads
the value from memory into r0, while x86 only does so when r0 and the
value in memory are different. The same issue affects s390.
At first this might sound like pure semantics, but it makes a real
difference when the comparison is 32-bit, since the load will
zero-extend r0/rax.
The fix is to explicitly zero-extend rax after doing such a
CMPXCHG. Since this problem affects multiple archs, this is done in
the verifier by patching in a BPF_ZEXT_REG instruction after every
32-bit cmpxchg. Any archs that don't need such manual zero-extension
can do a look-ahead with insn_is_zext to skip the unnecessary mov.
Note this still goes on top of Ilya's patch:
https://lore.kernel.org/bpf/20210301154019.129110-1-iii@linux.ibm.com/T/#u
Differences v5->v6[1]:
- Moved is_cmpxchg_insn and ensured it can be safely re-used. Also renamed it
and removed 'inline' to match the style of the is_*_function helpers.
- Fixed up comments in verifier test (thanks for the careful review, Martin!)
Differences v4->v5[1]:
- Moved the logic entirely into opt_subreg_zext_lo32_rnd_hi32, thanks to Martin
for suggesting this.
Differences v3->v4[1]:
- Moved the optimization against pointless zext into the correct place:
opt_subreg_zext_lo32_rnd_hi32 is called _after_ fixup_bpf_calls.
Differences v2->v3[1]:
- Moved patching into fixup_bpf_calls (patch incoming to rename this function)
- Added extra commentary on bpf_jit_needs_zext
- Added check to avoid adding a pointless zext(r0) if there's already one there.
Difference v1->v2[1]: Now solved centrally in the verifier instead of
specifically for the x86 JIT. Thanks to Ilya and Daniel for the suggestions!
[1] v5: https://lore.kernel.org/bpf/CA+i-1C3ytZz6FjcPmUg5s4L51pMQDxWcZNvM86w4RHZ_o2khwg@mail.gmail.com/T/#t
v4: https://lore.kernel.org/bpf/CA+i-1C3ytZz6FjcPmUg5s4L51pMQDxWcZNvM86w4RHZ_o2khwg@mail.gmail.com/T/#t
v3: https://lore.kernel.org/bpf/08669818-c99d-0d30-e1db-53160c063611@iogearbox.net/T/#t
v2: https://lore.kernel.org/bpf/08669818-c99d-0d30-e1db-53160c063611@iogearbox.net/T/#t
v1: https://lore.kernel.org/bpf/d7ebaefb-bfd6-a441-3ff2-2fdfe699b1d2@iogearbox.net/T/#t
Reported-by: Ilya Leoshkevich <iii@linux.ibm.com>
Fixes: 5ffa25502b ("bpf: Add instructions for atomic_[cmp]xchg")
Signed-off-by: Brendan Jackman <jackmanb@google.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
102 lines
3 KiB
C
102 lines
3 KiB
C
{
|
|
"BPF_ATOMIC OR without fetch",
|
|
.insns = {
|
|
/* val = 0x110; */
|
|
BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0x110),
|
|
/* atomic_or(&val, 0x011); */
|
|
BPF_MOV64_IMM(BPF_REG_1, 0x011),
|
|
BPF_ATOMIC_OP(BPF_DW, BPF_OR, BPF_REG_10, BPF_REG_1, -8),
|
|
/* if (val != 0x111) exit(2); */
|
|
BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8),
|
|
BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0x111, 2),
|
|
BPF_MOV64_IMM(BPF_REG_0, 2),
|
|
BPF_EXIT_INSN(),
|
|
/* r1 should not be clobbered, no BPF_FETCH flag */
|
|
BPF_MOV64_IMM(BPF_REG_0, 0),
|
|
BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0x011, 1),
|
|
BPF_MOV64_IMM(BPF_REG_0, 1),
|
|
BPF_EXIT_INSN(),
|
|
},
|
|
.result = ACCEPT,
|
|
},
|
|
{
|
|
"BPF_ATOMIC OR with fetch",
|
|
.insns = {
|
|
BPF_MOV64_IMM(BPF_REG_0, 123),
|
|
/* val = 0x110; */
|
|
BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0x110),
|
|
/* old = atomic_fetch_or(&val, 0x011); */
|
|
BPF_MOV64_IMM(BPF_REG_1, 0x011),
|
|
BPF_ATOMIC_OP(BPF_DW, BPF_OR | BPF_FETCH, BPF_REG_10, BPF_REG_1, -8),
|
|
/* if (old != 0x110) exit(3); */
|
|
BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0x110, 2),
|
|
BPF_MOV64_IMM(BPF_REG_0, 3),
|
|
BPF_EXIT_INSN(),
|
|
/* if (val != 0x111) exit(2); */
|
|
BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_10, -8),
|
|
BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0x111, 2),
|
|
BPF_MOV64_IMM(BPF_REG_1, 2),
|
|
BPF_EXIT_INSN(),
|
|
/* Check R0 wasn't clobbered (for fear of x86 JIT bug) */
|
|
BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 123, 2),
|
|
BPF_MOV64_IMM(BPF_REG_0, 1),
|
|
BPF_EXIT_INSN(),
|
|
/* exit(0); */
|
|
BPF_MOV64_IMM(BPF_REG_0, 0),
|
|
BPF_EXIT_INSN(),
|
|
},
|
|
.result = ACCEPT,
|
|
},
|
|
{
|
|
"BPF_ATOMIC OR with fetch 32bit",
|
|
.insns = {
|
|
/* r0 = (s64) -1 */
|
|
BPF_MOV64_IMM(BPF_REG_0, 0),
|
|
BPF_ALU64_IMM(BPF_SUB, BPF_REG_0, 1),
|
|
/* val = 0x110; */
|
|
BPF_ST_MEM(BPF_W, BPF_REG_10, -4, 0x110),
|
|
/* old = atomic_fetch_or(&val, 0x011); */
|
|
BPF_MOV32_IMM(BPF_REG_1, 0x011),
|
|
BPF_ATOMIC_OP(BPF_W, BPF_OR | BPF_FETCH, BPF_REG_10, BPF_REG_1, -4),
|
|
/* if (old != 0x110) exit(3); */
|
|
BPF_JMP32_IMM(BPF_JEQ, BPF_REG_1, 0x110, 2),
|
|
BPF_MOV32_IMM(BPF_REG_0, 3),
|
|
BPF_EXIT_INSN(),
|
|
/* if (val != 0x111) exit(2); */
|
|
BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_10, -4),
|
|
BPF_JMP32_IMM(BPF_JEQ, BPF_REG_1, 0x111, 2),
|
|
BPF_MOV32_IMM(BPF_REG_1, 2),
|
|
BPF_EXIT_INSN(),
|
|
/* Check R0 wasn't clobbered (for fear of x86 JIT bug)
|
|
* It should be -1 so add 1 to get exit code.
|
|
*/
|
|
BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 1),
|
|
BPF_EXIT_INSN(),
|
|
},
|
|
.result = ACCEPT,
|
|
},
|
|
{
|
|
"BPF_W atomic_fetch_or should zero top 32 bits",
|
|
.insns = {
|
|
/* r1 = U64_MAX; */
|
|
BPF_MOV64_IMM(BPF_REG_1, 0),
|
|
BPF_ALU64_IMM(BPF_SUB, BPF_REG_1, 1),
|
|
/* u64 val = r1; */
|
|
BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_1, -8),
|
|
/* r1 = (u32)atomic_fetch_or((u32 *)&val, 2); */
|
|
BPF_MOV32_IMM(BPF_REG_1, 2),
|
|
BPF_ATOMIC_OP(BPF_W, BPF_OR | BPF_FETCH, BPF_REG_10, BPF_REG_1, -8),
|
|
/* r2 = 0x00000000FFFFFFFF; */
|
|
BPF_MOV64_IMM(BPF_REG_2, 1),
|
|
BPF_ALU64_IMM(BPF_LSH, BPF_REG_2, 32),
|
|
BPF_ALU64_IMM(BPF_SUB, BPF_REG_2, 1),
|
|
/* if (r2 != r1) exit(1); */
|
|
BPF_JMP_REG(BPF_JEQ, BPF_REG_2, BPF_REG_1, 2),
|
|
BPF_MOV64_REG(BPF_REG_0, BPF_REG_1),
|
|
BPF_EXIT_INSN(),
|
|
/* exit(0); */
|
|
BPF_MOV32_IMM(BPF_REG_0, 0),
|
|
BPF_EXIT_INSN(),
|
|
},
|
|
.result = ACCEPT,
|
|
},
|