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

[email protected]
Newsgroups dev.linux.lists.loongarch,org.kernel.vger.bpf,org.kernel.vger.linux-kernel
Message-ID <18fb7827c1b98f6edeb6d702eba20dded3bb0168fb5939a69ed0eb9a1eade968@mail.kernel.org>
> 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: [email protected]
> Signed-off-by: Tiezhu Yang <[email protected]>

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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.