[RESEND,bpf] bpf: verifier: fix addr_space_cast from as(1) to as(0)

Commit Message

Puranjay Mohan March 21, 2024, 3:39 p.m. UTC

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 <[e-mail]>
---
 kernel/bpf/verifier.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

[e-mail] March 23, 2024, 3:30 a.m. UTC | #1

Hello:

This patch was applied to bpf/bpf.git (master)
by Alexei Starovoitov <[e-mail]>:

On Thu, 21 Mar 2024 15:39:39 +0000 you 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.
> 
> [...]

Here is the summary with links:
  - [RESEND,bpf] bpf: verifier: fix addr_space_cast from as(1) to as(0)
    https://git.kernel.org/bpf/bpf/c/c49cdf5b88aa

You are awesome, thank you!

Patch

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;
 		}