Re: [PATCH bpf-next v4 4/6] riscv, bpf: Add RV_TAILCALL_OFFSET macro to format tailcall offset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The tail call offset macro `RV_TAILCALL_OFFSET` incorrectly double-counts the KCFI preamble when `CONFIG_CFI` is enabled, causing tail-called programs to skip their stack pointer adjustment and corrupt the caller's stack frame.
--

commit 655037e3f7e62935bb76d41ae8cffb13d2a38d50
Author: Pu Lehui <pulehui@huawei.com>

riscv, bpf: Add RV_TAILCALL_OFFSET macro to format tailcall offset

Add RV_TAILCALL_OFFSET macro to format tailcall offset, and correct the
relevant comments.

> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 69512d93ca4a7..c239696cca646 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -21,6 +21,8 @@
>  #define RV_KCFI_NINSNS (IS_ENABLED(CONFIG_CFI) ? 1 : 0)
>  /* imm that allows emit_imm to emit max count insns */
>  #define RV_MAX_COUNT_IMM 0x7FFF7FF7FF7FF7FF
> +/* kcfi, fentry and TCC init insns will be skipped on tailcall */
> +#define RV_TAILCALL_OFFSET ((RV_KCFI_NINSNS + RV_FENTRY_NINSNS + 1) * 4)

[Severity: Critical]
This is a pre-existing issue, but does RV_TAILCALL_OFFSET incorrectly
double-count the KCFI preamble when CONFIG_CFI is enabled?

Since prog->bpf_func already skips the 4-byte KCFI hash (because it points
to ctx->ro_insns + cfi_get_offset()), it looks like including RV_KCFI_NINSNS
here overshoots the correct jump target by 4 bytes.

Does this skip the necessary stack pointer adjustment instruction, causing
tail-called programs to execute prologue register saves using the unadjusted
stack pointer of the caller and potentially corrupting the kernel stack?

[ ... ]

> @@ -272,9 +274,7 @@ static void __build_epilogue(bool is_tail_call, struct rv_jit_context *ctx)
>  	if (!is_tail_call)
>  		emit_addiw(RV_REG_A0, RV_REG_A5, 0, ctx);
>  	emit_jalr(RV_REG_ZERO, is_tail_call ? RV_REG_T3 : RV_REG_RA,
> -		  /* kcfi, fentry and TCC init insns will be skipped on tailcall */
> -		  is_tail_call ? (RV_KCFI_NINSNS + RV_FENTRY_NINSNS + 1) * 4 : 0,
> -		  ctx);
> +		  is_tail_call ? RV_TAILCALL_OFFSET : 0, ctx);
>  }

[ ... ]

> @@ -394,7 +394,7 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
>  	off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
>  	emit_branch(BPF_JEQ, RV_REG_T2, RV_REG_ZERO, off, ctx);
>  
> -	/* goto *(prog->bpf_func + 4); */
> +	/* goto *(prog->bpf_func + RV_TAILCALL_OFFSET); */
>  	off = offsetof(struct bpf_prog, bpf_func);
>  	if (is_12b_check(off, insn))
>  		return -1;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629140048.733346-1-pulehui@huaweicloud.com?part=4