Commit 4178417c authored by Luke Nelson's avatar Luke Nelson Committed by Daniel Borkmann

arm, bpf: Fix offset overflow for BPF_MEM BPF_DW

This patch fixes an incorrect check in how immediate memory offsets are
computed for BPF_DW on arm.

For BPF_LDX/ST/STX + BPF_DW, the 32-bit arm JIT breaks down an 8-byte
access into two separate 4-byte accesses using off+0 and off+4. If off
fits in imm12, the JIT emits a ldr/str instruction with the immediate
and avoids the use of a temporary register. While the current check off
<= 0xfff ensures that the first immediate off+0 doesn't overflow imm12,
it's not sufficient for the second immediate off+4, which may cause the
second access of BPF_DW to read/write the wrong address.

This patch fixes the problem by changing the check to
off <= 0xfff - 4 for BPF_DW, ensuring off+4 will never overflow.

A side effect of simplifying the check is that it now allows using
negative immediate offsets in ldr/str. This means that small negative
offsets can also avoid the use of a temporary register.

This patch introduces no new failures in test_verifier or test_bpf.c.

Fixes: c5eae692 ("ARM: net: bpf: improve 64-bit store implementation")
Fixes: ec19e02b ("ARM: net: bpf: fix LDX instructions")
Co-developed-by: default avatarXi Wang <xi.wang@gmail.com>
Signed-off-by: default avatarXi Wang <xi.wang@gmail.com>
Signed-off-by: default avatarLuke Nelson <luke.r.nels@gmail.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20200409221752.28448-1-luke.r.nels@gmail.com
parent e154659b
...@@ -1000,21 +1000,35 @@ static inline void emit_a32_mul_r64(const s8 dst[], const s8 src[], ...@@ -1000,21 +1000,35 @@ static inline void emit_a32_mul_r64(const s8 dst[], const s8 src[],
arm_bpf_put_reg32(dst_hi, rd[0], ctx); arm_bpf_put_reg32(dst_hi, rd[0], ctx);
} }
static bool is_ldst_imm(s16 off, const u8 size)
{
s16 off_max = 0;
switch (size) {
case BPF_B:
case BPF_W:
off_max = 0xfff;
break;
case BPF_H:
off_max = 0xff;
break;
case BPF_DW:
/* Need to make sure off+4 does not overflow. */
off_max = 0xfff - 4;
break;
}
return -off_max <= off && off <= off_max;
}
/* *(size *)(dst + off) = src */ /* *(size *)(dst + off) = src */
static inline void emit_str_r(const s8 dst, const s8 src[], static inline void emit_str_r(const s8 dst, const s8 src[],
s32 off, struct jit_ctx *ctx, const u8 sz){ s16 off, struct jit_ctx *ctx, const u8 sz){
const s8 *tmp = bpf2a32[TMP_REG_1]; const s8 *tmp = bpf2a32[TMP_REG_1];
s32 off_max;
s8 rd; s8 rd;
rd = arm_bpf_get_reg32(dst, tmp[1], ctx); rd = arm_bpf_get_reg32(dst, tmp[1], ctx);
if (sz == BPF_H) if (!is_ldst_imm(off, sz)) {
off_max = 0xff;
else
off_max = 0xfff;
if (off < 0 || off > off_max) {
emit_a32_mov_i(tmp[0], off, ctx); emit_a32_mov_i(tmp[0], off, ctx);
emit(ARM_ADD_R(tmp[0], tmp[0], rd), ctx); emit(ARM_ADD_R(tmp[0], tmp[0], rd), ctx);
rd = tmp[0]; rd = tmp[0];
...@@ -1043,18 +1057,12 @@ static inline void emit_str_r(const s8 dst, const s8 src[], ...@@ -1043,18 +1057,12 @@ static inline void emit_str_r(const s8 dst, const s8 src[],
/* dst = *(size*)(src + off) */ /* dst = *(size*)(src + off) */
static inline void emit_ldx_r(const s8 dst[], const s8 src, static inline void emit_ldx_r(const s8 dst[], const s8 src,
s32 off, struct jit_ctx *ctx, const u8 sz){ s16 off, struct jit_ctx *ctx, const u8 sz){
const s8 *tmp = bpf2a32[TMP_REG_1]; const s8 *tmp = bpf2a32[TMP_REG_1];
const s8 *rd = is_stacked(dst_lo) ? tmp : dst; const s8 *rd = is_stacked(dst_lo) ? tmp : dst;
s8 rm = src; s8 rm = src;
s32 off_max;
if (sz == BPF_H)
off_max = 0xff;
else
off_max = 0xfff;
if (off < 0 || off > off_max) { if (!is_ldst_imm(off, sz)) {
emit_a32_mov_i(tmp[0], off, ctx); emit_a32_mov_i(tmp[0], off, ctx);
emit(ARM_ADD_R(tmp[0], tmp[0], src), ctx); emit(ARM_ADD_R(tmp[0], tmp[0], src), ctx);
rm = tmp[0]; rm = tmp[0];
......
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