[PATCH v1] LoongArch: BPF: Fix off-by-one error for insn_is_cast_user

Tiezhu Yang posted 1 patch 4 weeks, 1 day ago
There is a newer version of this series
arch/loongarch/net/bpf_jit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v1] LoongArch: BPF: Fix off-by-one error for insn_is_cast_user
Posted by Tiezhu Yang 4 weeks, 1 day ago
Fix a severe off-by-one error in the branch offset calculation for the
user pointer cast helper insn_is_cast_user() inside build_insn().

In the current code, an offset of 1 causes the "beq" to target the next
"or" instruction, which means the "or" instruction is always executed,
even if the pointer is NULL. Thus, a native NULL pointer is incorrectly
combined with the arena base address and turns into a non-zero address,
breaking nullable safety guards and causing silent memory corruption.

Fix this by changing the branch offset from 1 to 2, which properly skips
the "or" instruction and jumps directly to the "move_reg" instruction if
the pointer is NULL, ensuring the destination register is safely cleared
to 0.

Fixes: 4fdb5dd8aeba ("LoongArch: BPF: Implement bpf_addr_space_cast instruction")
Cc: stable@vger.kernel.org
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/net/bpf_jit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
index 1eb588e443c9..4da278900938 100644
--- a/arch/loongarch/net/bpf_jit.c
+++ b/arch/loongarch/net/bpf_jit.c
@@ -717,7 +717,7 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool ext
 			move_reg(ctx, t1, src);
 			emit_zext_32(ctx, t1, true);
 			move_imm(ctx, dst, (ctx->user_vm_start >> 32) << 32, false);
-			emit_insn(ctx, beq, t1, LOONGARCH_GPR_ZERO, 1);
+			emit_insn(ctx, beq, t1, LOONGARCH_GPR_ZERO, 2);
 			emit_insn(ctx, or, t1, dst, t1);
 			move_reg(ctx, dst, t1);
 			break;
-- 
2.42.0
Re: [PATCH v1] LoongArch: BPF: Fix off-by-one error for insn_is_cast_user
Posted by bot+bpf-ci@kernel.org 4 weeks, 1 day ago
> LoongArch: BPF: Fix off-by-one error for insn_is_cast_user
>
> Fix a severe off-by-one error in the branch offset calculation for the
> user pointer cast helper insn_is_cast_user() inside build_insn().
>
> In the current code, an offset of 1 causes the "beq" to target the next
> "or" instruction, which means the "or" instruction is always executed,
> even if the pointer is NULL. Thus, a native NULL pointer is incorrectly
> combined with the arena base address and turns into a non-zero address,
> breaking nullable safety guards and causing silent memory corruption.
>
> Fix this by changing the branch offset from 1 to 2, which properly skips
> the "or" instruction and jumps directly to the "move_reg" instruction if
> the pointer is NULL, ensuring the destination register is safely cleared
> to 0.
>
> Fixes: 4fdb5dd8aeba ("LoongArch: BPF: Implement bpf_addr_space_cast instruction")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>

This isn't a bug, but would it read more precisely to say the cast_user
JIT arm in build_insn() rather than "the user pointer cast helper
insn_is_cast_user()"?

insn_is_cast_user() in include/linux/filter.h is only the predicate that
recognises BPF_ALU64|BPF_MOV|BPF_X with off == BPF_ADDR_SPACE_CAST and
imm == 1U << 16; the cast itself is emitted by the JIT arm in build_insn().

Also, would describing the effect as "a zero arena offset no longer casts
to NULL" capture the same facts more concisely than "breaking nullable
safety guards and causing silent memory corruption"?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33153668098