* [PATCH bpf] bpf: fix BPF_ALU32 | BPF_ARSH on BE arches
@ 2019-06-25 16:41 Jiong Wang
2019-06-25 20:15 ` Song Liu
2019-06-26 14:31 ` Daniel Borkmann
0 siblings, 2 replies; 3+ messages in thread
From: Jiong Wang @ 2019-06-25 16:41 UTC (permalink / raw)
To: alexei.starovoitov, daniel
Cc: yauheni.kaliuta, bpf, netdev, oss-drivers, Jiong Wang
Yauheni reported the following code do not work correctly on BE arches:
ALU_ARSH_X:
DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
CONT;
ALU_ARSH_K:
DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
CONT;
and are causing failure of test_verifier test 'arsh32 on imm 2' on BE
arches.
The code is taking address and interpreting memory directly, so is not
endianness neutral. We should instead perform standard C type casting on
the variable. A u64 to s32 conversion will drop the high 32-bit and reserve
the low 32-bit as signed integer, this is all we want.
Fixes: 2dc6b100f928 ("bpf: interpreter support BPF_ALU | BPF_ARSH")
Reported-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
kernel/bpf/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 080e2bb..f2148db 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1364,10 +1364,10 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u64 *stack)
insn++;
CONT;
ALU_ARSH_X:
- DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
+ DST = (u64) (u32) (((s32) DST) >> SRC);
CONT;
ALU_ARSH_K:
- DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
+ DST = (u64) (u32) (((s32) DST) >> IMM);
CONT;
ALU64_ARSH_X:
(*(s64 *) &DST) >>= SRC;
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf] bpf: fix BPF_ALU32 | BPF_ARSH on BE arches
2019-06-25 16:41 [PATCH bpf] bpf: fix BPF_ALU32 | BPF_ARSH on BE arches Jiong Wang
@ 2019-06-25 20:15 ` Song Liu
2019-06-26 14:31 ` Daniel Borkmann
1 sibling, 0 replies; 3+ messages in thread
From: Song Liu @ 2019-06-25 20:15 UTC (permalink / raw)
To: Jiong Wang
Cc: Alexei Starovoitov, Daniel Borkmann, yauheni.kaliuta, bpf,
Networking, oss-drivers
On Tue, Jun 25, 2019 at 12:31 PM Jiong Wang <jiong.wang@netronome.com> wrote:
>
> Yauheni reported the following code do not work correctly on BE arches:
>
> ALU_ARSH_X:
> DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
> CONT;
> ALU_ARSH_K:
> DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
> CONT;
>
> and are causing failure of test_verifier test 'arsh32 on imm 2' on BE
> arches.
>
> The code is taking address and interpreting memory directly, so is not
> endianness neutral. We should instead perform standard C type casting on
> the variable. A u64 to s32 conversion will drop the high 32-bit and reserve
> the low 32-bit as signed integer, this is all we want.
>
> Fixes: 2dc6b100f928 ("bpf: interpreter support BPF_ALU | BPF_ARSH")
> Reported-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Acked-by: Song Liu <songliubraving@fb.com>
I guess we need:
Cc: <stable@vger.kernel.org> #v5.0+
> ---
> kernel/bpf/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 080e2bb..f2148db 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -1364,10 +1364,10 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u64 *stack)
> insn++;
> CONT;
> ALU_ARSH_X:
> - DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
> + DST = (u64) (u32) (((s32) DST) >> SRC);
> CONT;
> ALU_ARSH_K:
> - DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
> + DST = (u64) (u32) (((s32) DST) >> IMM);
> CONT;
> ALU64_ARSH_X:
> (*(s64 *) &DST) >>= SRC;
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf] bpf: fix BPF_ALU32 | BPF_ARSH on BE arches
2019-06-25 16:41 [PATCH bpf] bpf: fix BPF_ALU32 | BPF_ARSH on BE arches Jiong Wang
2019-06-25 20:15 ` Song Liu
@ 2019-06-26 14:31 ` Daniel Borkmann
1 sibling, 0 replies; 3+ messages in thread
From: Daniel Borkmann @ 2019-06-26 14:31 UTC (permalink / raw)
To: Jiong Wang, alexei.starovoitov; +Cc: yauheni.kaliuta, bpf, netdev, oss-drivers
On 06/25/2019 06:41 PM, Jiong Wang wrote:
> Yauheni reported the following code do not work correctly on BE arches:
>
> ALU_ARSH_X:
> DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
> CONT;
> ALU_ARSH_K:
> DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
> CONT;
>
> and are causing failure of test_verifier test 'arsh32 on imm 2' on BE
> arches.
>
> The code is taking address and interpreting memory directly, so is not
> endianness neutral. We should instead perform standard C type casting on
> the variable. A u64 to s32 conversion will drop the high 32-bit and reserve
> the low 32-bit as signed integer, this is all we want.
>
> Fixes: 2dc6b100f928 ("bpf: interpreter support BPF_ALU | BPF_ARSH")
> Reported-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Applied, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-06-26 14:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-25 16:41 [PATCH bpf] bpf: fix BPF_ALU32 | BPF_ARSH on BE arches Jiong Wang
2019-06-25 20:15 ` Song Liu
2019-06-26 14:31 ` 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).