Daniel Borkmann wrote: [...] > >> +Also note that the division and modulo operations are unsigned, > >> +where 'imm' is first sign extended to 64 bits and then converted to > >> +an unsigned 64-bit value. There are no instructions for signed > >> +division or modulo. > > > > Less sure about this part, but it looks to be true at least by looking > > at the interpreter which does: > > > > DST = DST / IMM > > > > where: > > > > DST === (u64) regs[insn->dst_reg] > > IMM === (s32) insn->imm > > > > (and s32 is sign-expanded to u64 according to C rules) > > Yeap, the actual operation is in the target width, so for 32 bit it's casted to > u32, e.g. for modulo (note that the verifier rewrites it into `(src != 0) ? > (dst % src) : dst` form, so here is just the extract of the plain mod insn and it's > similar for div): > > ALU64_MOD_X: > div64_u64_rem(DST, SRC, &AX); > DST = AX; > CONT; > ALU_MOD_X: > AX = (u32) DST; > DST = do_div(AX, (u32) SRC); > CONT; > ALU64_MOD_K: > div64_u64_rem(DST, IMM, &AX); > DST = AX; > CONT; > ALU_MOD_K: > AX = (u32) DST; > DST = do_div(AX, (u32) IMM); > CONT; > > So in above phrasing the middle part needs to be adapted or just removed. The phrasing was based on the earlier discussion on this list (see https://lore.kernel.org/bpf/CAADnVQJ387tWd7WgxqfoB44xMe17bY0RRp_Sng3xMnKsywFpxg@xxxxxxxxxxxxxx/) where Alexei wrote "imm32 is _sign_ extended everywhere", and I cited the div_k tests in lib/test_bpf.c that assume sign extension not zero extension. So I don't know how to reconcile your comments with that thread. If you do, please educate me :) Dave
