Re: [RFC PATCH bpf-next 3/6] bpf: Reject UHMUL/SHMUL in unsupported JITs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The PowerPC JIT backends check the first instruction of the program instead of the current instruction when rejecting UHMUL/SHMUL.
--

commit 1bfedfff9f6c73dc718b4f4866c0696e836592cb
Author: Yazhou Tang <tangyazhou518@outlook.com>

bpf: Reject UHMUL/SHMUL in unsupported JITs

CPU JIT backends currently lower BPF_MUL based on the opcode without
examining insn->off. Once the verifier accepts UHMUL and SHMUL, a backend
without explicit support would therefore lower either variant as the
existing low-half multiplication and silently produce the wrong result.

> diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
> index bfdc50740da8e..b578b19017b22 100644
> --- a/arch/powerpc/net/bpf_jit_comp32.c
> +++ b/arch/powerpc/net/bpf_jit_comp32.c
> @@ -349,6 +349,9 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, u32 *fimage, struct code
>  		u32 true_cond;
>  		u32 tmp_idx;
>  
> +		if (bpf_insn_is_hmul(insn))
> +			return -EOPNOTSUPP;
> +

[Severity: High]
Does this code check the correct instruction? In bpf_jit_build_body(),
insn points to the first instruction of the program (fp->insnsi), and
the loop uses the index i. By calling bpf_insn_is_hmul(insn) instead of
bpf_insn_is_hmul(&insn[i]), the check only ever looks at the 0-th
instruction of the program.

If an unsupported UHMUL or SHMUL instruction occurs anywhere other than the
very first instruction, it will not be rejected and will instead be compiled
silently as a standard lower-half multiplication.

>  		if (i && (BPF_CLASS(code) == BPF_ALU64 || BPF_CLASS(code) == BPF_ALU) &&
>  		    (BPF_CLASS(prevcode) == BPF_ALU64 || BPF_CLASS(prevcode) == BPF_ALU) &&
>  		    BPF_OP(prevcode) == BPF_MOV && BPF_SRC(prevcode) == BPF_X &&

[ ... ]

> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index fc9db691e8203..4ef0f3cffe848 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -992,6 +992,9 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, u32 *fimage, struct code
>  		u32 tmp_idx;
>  		u32 jmp_off;
>  
> +		if (bpf_insn_is_hmul(insn))
> +			return -EOPNOTSUPP;
> +

[Severity: High]
Similarly, does this code check the correct instruction for the 64-bit JIT?
It appears to check the first instruction instead of &insn[i], which will
cause the same silent incorrect compilation for unsupported UHMUL/SHMUL
instructions not located at the first position in the program.

>  		/*
>  		 * addrs[] maps a BPF bytecode address into a real offset from
>  		 * the start of the body code.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902070534.2141812-1-tangyazhou@zju.edu.cn?part=3