netdev - Re: [PATCH bpf] riscv, bpf: Fix kfunc parameters incompatibility between bpf and riscv abi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <mb61pfrwf7afx.fsf@gmail.com>
Date: Sun, 24 Mar 2024 18:26:42 +0000
From: Puranjay Mohan <puranjay12@...il.com>
To: Pu Lehui <pulehui@...weicloud.com>, bpf@...r.kernel.org,
 linux-riscv@...ts.infradead.org, netdev@...r.kernel.org
Cc: Björn Töpel <bjorn@...nel.org>, Alexei Starovoitov
 <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko
 <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, Eduard
 Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, Yonghong Song
 <yhs@...com>, John Fastabend <john.fastabend@...il.com>, KP Singh
 <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo
 <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, Palmer Dabbelt
 <palmer@...belt.com>, Luke Nelson <luke.r.nels@...il.com>, Pu Lehui
 <pulehui@...wei.com>, Pu Lehui <pulehui@...weicloud.com>
Subject: Re: [PATCH bpf] riscv, bpf: Fix kfunc parameters incompatibility
 between bpf and riscv abi

Pu Lehui <pulehui@...weicloud.com> writes:

> From: Pu Lehui <pulehui@...wei.com>
>
> We encountered a failing case when running selftest in no_alu32 mode:
>
> The failure case is `kfunc_call/kfunc_call_test4` and its source code is
> like bellow:
> ```
> long bpf_kfunc_call_test4(signed char a, short b, int c, long d) __ksym;
> int kfunc_call_test4(struct __sk_buff *skb)
> {
> 	...
> 	tmp = bpf_kfunc_call_test4(-3, -30, -200, -1000);
> 	...
> }
> ```
>
> And its corresponding asm code is:
> ```
> 0: r1 = -3
> 1: r2 = -30
> 2: r3 = 0xffffff38 # opcode: 18 03 00 00 38 ff ff ff 00 00 00 00 00 00 00 00
> 4: r4 = -1000
> 5: call bpf_kfunc_call_test4
> ```
>
> insn 2 is parsed to ld_imm64 insn to emit 0x00000000ffffff38 imm, and
> converted to int type and then send to bpf_kfunc_call_test4. But since
> it is zero-extended in the bpf calling convention, riscv jit will
> directly treat it as an unsigned 32-bit int value, and then fails with
> the message "actual 4294966063 != expected -1234".
>
> The reason is the incompatibility between bpf and riscv abi, that is,
> bpf will do zero-extension on uint, but riscv64 requires sign-extension
> on int or uint. We can solve this problem by sign extending the 32-bit
> parameters in kfunc.
>
> The issue is related to [0], and thanks to Yonghong and Alexei.
>
> Link: https://github.com/llvm/llvm-project/pull/84874 [0]
> Fixes: d40c3847b485 ("riscv, bpf: Add kfunc support for RV64")
> Signed-off-by: Pu Lehui <pulehui@...wei.com>
> ---
>  arch/riscv/net/bpf_jit_comp64.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 869e4282a2c4..e3fc39370f7d 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -1454,6 +1454,22 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>  		if (ret < 0)
>  			return ret;
>  
> +		if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
> +			const struct btf_func_model *fm;
> +			int idx;
> +
> +			fm = bpf_jit_find_kfunc_model(ctx->prog, insn);
> +			if (!fm)
> +				return -EINVAL;
> +
> +			for (idx = 0; idx < fm->nr_args; idx++) {
> +				u8 reg = bpf_to_rv_reg(BPF_REG_1 + idx, ctx);
> +
> +				if (fm->arg_size[idx] == sizeof(int))
> +					emit_sextw(reg, reg, ctx);
> +			}
> +		}
> +
>  		ret = emit_call(addr, fixed_addr, ctx);
>  		if (ret)
>  			return ret;
> -- 
> 2.34.1

Thanks for doing this, it fixes the issue I was seeing with arena_htab
selftest after enabling arena on RISCV.

Tested-by: Puranjay Mohan <puranjay12@...il.com>
Reviewed-by: Puranjay Mohan <puranjay12@...il.com>