* [PATCH bpf] arm, bpf: Fix offset overflow for BPF_MEM BPF_DW
@ 2020-04-09 22:17 Luke Nelson
2020-04-14 19:43 ` Daniel Borkmann
0 siblings, 1 reply; 2+ messages in thread
From: Luke Nelson @ 2020-04-09 22:17 UTC (permalink / raw)
To: bpf
Cc: Luke Nelson, Xi Wang, Shubham Bansal, Russell King,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, John Fastabend, KP Singh, netdev,
linux-arm-kernel, linux-kernel
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: c5eae692571d6 ("ARM: net: bpf: improve 64-bit store implementation")
Fixes: ec19e02b343db ("ARM: net: bpf: fix LDX instructions")
Co-developed-by: Xi Wang <xi.wang@gmail.com>
Signed-off-by: Xi Wang <xi.wang@gmail.com>
Signed-off-by: Luke Nelson <luke.r.nels@gmail.com>
---
arch/arm/net/bpf_jit_32.c | 40 +++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index d124f78e20ac..bf85d6db4931 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -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);
}
+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 */
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];
- s32 off_max;
s8 rd;
rd = arm_bpf_get_reg32(dst, tmp[1], ctx);
- 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(ARM_ADD_R(tmp[0], tmp[0], rd), ctx);
rd = tmp[0];
@@ -1043,18 +1057,12 @@ static inline void emit_str_r(const s8 dst, const s8 src[],
/* dst = *(size*)(src + off) */
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 *rd = is_stacked(dst_lo) ? tmp : dst;
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(ARM_ADD_R(tmp[0], tmp[0], src), ctx);
rm = tmp[0];
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH bpf] arm, bpf: Fix offset overflow for BPF_MEM BPF_DW
2020-04-09 22:17 [PATCH bpf] arm, bpf: Fix offset overflow for BPF_MEM BPF_DW Luke Nelson
@ 2020-04-14 19:43 ` Daniel Borkmann
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Borkmann @ 2020-04-14 19:43 UTC (permalink / raw)
To: Luke Nelson, bpf
Cc: Luke Nelson, Xi Wang, Shubham Bansal, Russell King,
Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
Andrii Nakryiko, John Fastabend, KP Singh, netdev,
linux-arm-kernel, linux-kernel
On 4/10/20 12:17 AM, Luke Nelson wrote:
> 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: c5eae692571d6 ("ARM: net: bpf: improve 64-bit store implementation")
> Fixes: ec19e02b343db ("ARM: net: bpf: fix LDX instructions")
> Co-developed-by: Xi Wang <xi.wang@gmail.com>
> Signed-off-by: Xi Wang <xi.wang@gmail.com>
> Signed-off-by: Luke Nelson <luke.r.nels@gmail.com>
Applied, thanks!
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-04-14 19:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-09 22:17 [PATCH bpf] arm, bpf: Fix offset overflow for BPF_MEM BPF_DW Luke Nelson
2020-04-14 19:43 ` Daniel Borkmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).