On Thu, Mar 21, 2024 at 4:03 PM Puranjay Mohan <puranjay12@xxxxxxxxx> wrote:
>
> The verifier currently converts addr_space_cast from as(1) to as(0) that
> is: BPF_ALU64 | BPF_MOV | BPF_X with off=1 and imm=1
> to
> BPF_ALU | BPF_MOV | BPF_X with imm=1 (32-bit mov)
>
> Because of this imm=1, the JITs that have bpf_jit_needs_zext() == true,
> interpret the converted instruction as BPF_ZEXT_REG(DST) which is a
> special form of mov32, used for doing explicit zero extension on dst.
> These JITs will just zero extend the dst reg and will not move the src to
> dst before the zext.
>
> Fix do_misc_fixups() to set imm=0 when converting addr_space_cast to a
> normal mov32.
>
> The JITs that have bpf_jit_needs_zext() == true rely on the verifier to
> emit zext instructions. Mark dst_reg as subreg when doing cast from
> as(1) to as(0) so the verifier emits a zext instruction after the mov.
>
> Fixes: 6082b6c328b5 ("bpf: Recognize addr_space_cast instruction in the verifier.")
> Signed-off-by: Puranjay Mohan <puranjay12@xxxxxxxxx>
> ---
> kernel/bpf/verifier.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index de7813947981..ee796402ef60 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -14046,8 +14046,11 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
> if (insn->imm) {
> /* off == BPF_ADDR_SPACE_CAST */
> mark_reg_unknown(env, regs, insn->dst_reg);
> - if (insn->imm == 1) /* cast from as(1) to as(0) */
> + if (insn->imm == 1) { /* cast from as(1) to as(0) */
> dst_reg->type = PTR_TO_ARENA;
> + /* PTR_TO_ARENA is 32-bit */
> + dst_reg->subreg_def = env->insn_idx + 1;
> + }
> } else if (insn->off == 0) {
> /* case: R1 = R2
> * copy register state to dest reg
> @@ -19606,8 +19609,9 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
> (((struct bpf_map *)env->prog->aux->arena)->map_flags & BPF_F_NO_USER_CONV)) {
> /* convert to 32-bit mov that clears upper 32-bit */
> insn->code = BPF_ALU | BPF_MOV | BPF_X;
> - /* clear off, so it's a normal 'wX = wY' from JIT pov */
> + /* clear off and imm, so it's a normal 'wX = wY' from JIT pov */
> insn->off = 0;
> + insn->imm = 0;
> } /* cast from as(0) to as(1) should be handled by JIT */
> goto next_insn;
> }
> --
> 2.40.1
>
This did not reach the list somehow.
I will have to resend to trigger the CI.
Sorry for the noise.
Puranjay
