On Thu, 2023-09-28 at 21:35 +0000, Dave Thaler wrote:
> In re-reading the instruction-set.rst changes for sign extensions, there is one ambiguity
> regarding BPF_ALU | BPF_MOVSX with offset = 32.
>
> Is it:
> a) Undefined (not a permitted instruction), or
> b) Defined as being synonymous with BPF_ALU | BPF_MOV?
>
> The table implies (b) when it says:
> > BPF_MOVSX 0xb0 8/16/32 dst = (s8,s16,s32)src
>
> But the following text could be interpreted as ():
> > ``BPF_ALU | BPF_MOVSX`` :term:`sign extends<Sign Extend>` 8-bit and 16-bit operands into 32
> > bit operands, and zeroes the remaining upper 32 bits.
Hi Dave,
I checked current verifier implementation and it goes with option (a):
static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
{
...
} else if (opcode == BPF_MOV) {
if (BPF_SRC(insn->code) == BPF_X) {
...
if (BPF_CLASS(insn->code) == BPF_ALU) {
if (insn->off != 0 && insn->off != 8 && insn->off != 16) {
verbose(env, "BPF_MOV uses reserved fields\n");
return -EINVAL;
}
} ...
...
} ...
...
}
For 32-bit move it reports error if insn->off == 32.
LLVM backend also uses option (a) as it only defines MOVSX_rr_32_8 and
MOVSX_rr_32_16, thus hypothetical MOVSX_rr_32_32 would be rejected by
disassembler.
> There's no reason I can think of to use it, given it's synonymous but if given a BPF program that
> uses it, should it be rejected by a verifier/disassembler/etc.? Or treated as valid?
Atleast this is what happens now.
Thanks,
Eduard.
