On Tue, Jun 25, 2019 at 12:31 PM Jiong Wang <jiong.w...@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.kali...@redhat.com>
> Reviewed-by: Jakub Kicinski <jakub.kicin...@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.mon...@netronome.com>
> Signed-off-by: Jiong Wang <jiong.w...@netronome.com>
Acked-by: Song Liu <songliubrav...@fb.com>
I guess we need:
Cc: <sta...@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
>
