2019-06-25 09:00:48

by Yauheni Kaliuta

Subject: ebpf: BPF_ALU32 | BPF_ARSH on BE arches

Hi!

Looks like the code:

ALU_ARSH_X:
DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
CONT;
ALU_ARSH_K:
DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
CONT;

works incorrectly on BE arches since it must operate on lower
parts of 64bit registers.

See failure of test_verifier test 'arsh32 on imm 2' (#23 on
5.2-rc6).


--
WBR,
Yauheni Kaliuta

2019-06-25 12:16:25

by Jiong Wang

Subject: Re: ebpf: BPF_ALU32 | BPF_ARSH on BE arches

Yauheni Kaliuta writes:

> Hi!
>
> Looks like the code:
>
> ALU_ARSH_X:
> DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
> CONT;
> ALU_ARSH_K:
> DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
> CONT;
>
> works incorrectly on BE arches since it must operate on lower
> parts of 64bit registers.
>
> See failure of test_verifier test 'arsh32 on imm 2' (#23 on
> 5.2-rc6).

Ah, thanks for reporting this.

Should not taken the address directly, does the following fix resolved the
failure?

ALU_ARSH_X:
DST = (u64) (u32) ((s32) DST) >> SRC);
CONT;
ALU_ARSH_K:
DST = (u64) (u32) ((s32) DST) >> IMM);
CONT;

Regards,
Jiong

2019-06-25 12:33:27

by Yauheni Kaliuta

Subject: Re: ebpf: BPF_ALU32 | BPF_ARSH on BE arches

Hi, Jiong!

>>>>> On Tue, 25 Jun 2019 11:20:07 +0100, Jiong Wang wrote:

> Yauheni Kaliuta writes:

>> Hi!
>>
>> Looks like the code:
>>
>> ALU_ARSH_X:
>> DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
>> CONT;
>> ALU_ARSH_K:
>> DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
>> CONT;
>>
>> works incorrectly on BE arches since it must operate on lower
>> parts of 64bit registers.
>>
>> See failure of test_verifier test 'arsh32 on imm 2' (#23 on
>> 5.2-rc6).

> Ah, thanks for reporting this.

> Should not taken the address directly, does the following fix resolved the
> failure?

> ALU_ARSH_X:
> DST = (u64) (u32) ((s32) DST) >> SRC);
> CONT;
> ALU_ARSH_K:
> DST = (u64) (u32) ((s32) DST) >> IMM);
> CONT;

Yes, thanks (just add the missing braces).

--
WBR,
Yauheni Kaliuta