Skip to contentSkip to conversation

ebpf: BPF_ALU32 | BPF_ARSH on BE arches

DISCUSSION Yauheni Kaliuta 2 participants 3 messages Jun 25, 2019 BPF

lore
Share & tools
lore
#0 lore raw

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).

Signature · 2 lines
WBR,
Yauheni Kaliuta
#1 lore raw
Quoted 11 lines · Yauheni Kaliuta

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;
Quoted 5 lines

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

#2 lore raw

Hi, Jiong!

Quoted 26 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Jiong Wang
Quoted 11 lines · Yauheni Kaliuta

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;

Quoted 5 lines

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?

Quoted 6 lines
        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).

Signature · 2 lines
WBR,
Yauheni Kaliuta