Re: [PATCH bpf-next v4 5/6] riscv, bpf: Mixing bpf2bpf and tailcalls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The RISC-V BPF JIT corrupts the Tail Call Counter (TCC) when returning from subprograms (bpf2bpf calls) or trampoline calls, breaking the maximum tail call limit and potentially causing stack overflows.
--

commit 5d936090d578733fdafd16fde93ba7223525701c
Author: Pu Lehui <pulehui@huawei.com>

riscv, bpf: Mixing bpf2bpf and tailcalls

This commit implements mixing bpf2bpf and tailcalls for the RISC-V JIT.
It transitions to passing the Tail Call Counter (TCC) between functions
using a non-callee saved register and explicitly saves it to the stack
across calls to preserve its value.

> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index c239696cca646..384b490c48579 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c

[ ... ]

> @@ -1254,6 +1240,9 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
>  		ret = emit_call((const u64)__bpf_tramp_exit, true, ctx);
>  		if (ret)
>  			goto out;
> +	} else if (flags & BPF_TRAMP_F_TAIL_CALL_CTX) {
> +		/* restore TCC to RV_REG_TCC before calling the orig bpf func */
> +		emit_ld(RV_REG_TCC, -tcc_off, RV_REG_FP, ctx);
>  	}

[Severity: High]
If BPF_TRAMP_F_CALL_ORIG is set, the preceding if block will execute and this
else if branch will be completely skipped.

Does this mean the trampoline skips restoring RV_REG_TCC before returning to
the caller, thereby leaking a clobbered TCC back to the calling function?

[ ... ]

> @@ -1837,10 +1826,18 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>  			}
>  		}
>  
> +		/* restore TCC to RV_REG_TCC before bpf2bpf call */
> +		if (aux->tail_call_reachable && insn->src_reg == BPF_PSEUDO_CALL)
> +			emit_ld(RV_REG_TCC, ctx->tcc_offset, RV_REG_SP, ctx);
> +
>  		ret = emit_call(addr, fixed_addr, ctx);
>  		if (ret)
>  			return ret;
>  
> +		/* store updated TCC back to stack after bpf2bpf call */
> +		if (aux->tail_call_reachable && insn->src_reg == BPF_PSEUDO_CALL)
> +			emit_sd(RV_REG_SP, ctx->tcc_offset, RV_REG_TCC, ctx);
> +
>  		if (insn->src_reg != BPF_PSEUDO_CALL)
>  			emit_mv(bpf_to_rv_reg(BPF_REG_0, ctx), RV_REG_A0, ctx);
>  		break;

[Severity: High]
Since RV_REG_TCC (a6) is a caller-saved register, if the subprogram executes
any BPF helper calls, won't a6 be clobbered?

Looking at __build_epilogue(), there doesn't appear to be any code to restore
RV_REG_TCC from the subprogram's stack slot before it returns to the caller.

If the subprogram returns with a clobbered a6, does this emit_sd() blindly
save that garbage value as the new TCC on the caller's stack? Could this
corrupt the tail call limit counter and potentially lead to infinite tail calls
or stack overflows?

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