On Fri, Sep 01 2023, Puranjay Mohan wrote:
> The problem here is that reg->subreg_def should be set as DEF_NOT_SUBREG for
> registers that are used as destination registers of BPF_LDX |
> BPF_MEMSX. I am seeing
> the same problem on ARM32 and was going to send a patch today.
>
> The problem is that is_reg64() returns false for destination registers
> of BPF_LDX | BPF_MEMSX.
> But BPF_LDX | BPF_MEMSX always loads a 64 bit value because of the
> sign extension so
> is_reg64() should return true.
>
> I have written a patch that I will be sending as a reply to this.
> Please let me know if that makes sense.
>
The check_reg_arg() function will mark reg->subreg_def = DEF_NOT_SUBREG for destination
registers if is_reg64() returns true for these registers. My patch below make is_reg64()
return true for destination registers of BPF_LDX with mod = BPF_MEMSX. I feel this is the
correct way to fix this problem.
Here is my patch:
--- 8< ---
>From cf1bf5282183cf721926ab14d968d3d4097b89b8 Mon Sep 17 00:00:00 2001
From: Puranjay Mohan <puranjay12@xxxxxxxxx>
Date: Fri, 1 Sep 2023 11:18:59 +0000
Subject: [PATCH bpf] bpf: verifier: mark destination of sign-extended load as
64 bit
The verifier can emit instructions to zero-extend destination registers
when the register is being used to keep 32 bit values. This behaviour is
enabled only when the JIT sets bpf_jit_needs_zext() -> true. In the case
of a sign extended load instruction, the destination register always has a
64-bit value, therefore the verifier should not emit zero-extend
instructions for it.
Change is_reg64() to return true if the register under consideration is a
destination register of LDX instruction with mode = BPF_MEMSX.
Fixes: 1f9a1ea821ff ("bpf: Support new sign-extension load insns")
Signed-off-by: Puranjay Mohan <puranjay12@xxxxxxxxx>
---
kernel/bpf/verifier.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index bb78212fa5b2..93f84b868ccc 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3029,7 +3029,7 @@ static bool is_reg64(struct bpf_verifier_env *env, struct bpf_insn *insn,
if (class == BPF_LDX) {
if (t != SRC_OP)
- return BPF_SIZE(code) == BPF_DW;
+ return (BPF_SIZE(code) == BPF_DW || BPF_MODE(code) == BPF_MEMSX);
/* LDX source must be ptr. */
return true;
}
--
2.39.2
