Re: [PATCH v3 1/4] LoongArch: BPF: Optimize redundant TCC loads in epilogue

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The macro BPF_TAIL_CALL_CNT_PTR_STACK_OFF does not account for the BPF Arena register, leading to an incorrect stack offset calculation and a deterministic kernel panic on tail calls when arena is used.
--

--- Patch [1]: [PATCH v3 1/4] LoongArch: BPF: Optimize redundant TCC loads in epilogue ---
commit eabfba52164a1855119336bb138d22f9f54aedfe
Author: Tiezhu Yang <yangtiezhu@loongson.cn>

    LoongArch: BPF: Optimize redundant TCC loads in epilogue
    
    The legacy epilogue implementation pops the tail call counter (TCC)
    context via a redundant double-load pattern. It first decrements the
    load_offset by 2 slots to fetch 'tcc_ptr', and then immediately bumps
    it back up by 1 slot to load the original 'tcc' value into REG_TCC,
    unnecessarily overwriting the register.
    
    Optimize this sequence by adjusting the load_offset by only 1 slot.
    This aligns the offset directly with the higher stack slot containing
    the entry TCC counter (or caller state), allowing us to restore the
    REG_TCC register safely with a single load.

> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index 2738b4db11653..93a1813122564 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -246,14 +246,8 @@ static void __build_epilogue(struct jit_ctx *ctx, bool is_tail_call)
>  		emit_insn(ctx, ldd, REG_ARENA, LOONGARCH_GPR_SP, load_offset);
>  	}
>  
> -	/*
> -	 * When push into the stack, follow the order of tcc then tcc_ptr.
> -	 * When pop from the stack, first pop tcc_ptr then followed by tcc.
> -	 */
> -	load_offset -= 2 * sizeof(long);
> -	emit_insn(ctx, ldd, REG_TCC, LOONGARCH_GPR_SP, load_offset);
> -
> -	load_offset += sizeof(long);
> +	/* Only restore the TCC state into REG_TCC from the higher slot */
> +	load_offset -= sizeof(long);
>  	emit_insn(ctx, ldd, REG_TCC, LOONGARCH_GPR_SP, load_offset);

[Severity: High]
This isn't a bug introduced by this patch, but while looking at the dynamic
stack offset handling for REG_ARENA here, I noticed an issue in how the stack
offset is calculated for the tail call counter pointer.

While the prologue and epilogue dynamically adjust load_offset for REG_ARENA
(shifting the offsets down by 8 bytes when ctx->arena_vm_start is true),
the macro BPF_TAIL_CALL_CNT_PTR_STACK_OFF uses a hardcoded subtraction:

arch/loongarch/net/bpf_jit.c:
#define BPF_TAIL_CALL_CNT_PTR_STACK_OFF(stack) (round_up(stack, 16) - 80)

When a BPF program using the BPF arena makes a tail call, emit_bpf_tail_call()
uses this macro to calculate tcc_ptr_off:

arch/loongarch/net/bpf_jit.c:emit_bpf_tail_call() {
    ...
    int tcc_ptr_off = BPF_TAIL_CALL_CNT_PTR_STACK_OFF(ctx->stack_size);
    ...
    emit_insn(ctx, ldd, REG_TCC, LOONGARCH_GPR_SP, tcc_ptr_off);
    emit_insn(ctx, ldd, t3, REG_TCC, 0);
    ...
}

Because the hardcoded macro doesn't account for the 8-byte shift from
REG_ARENA, it will evaluate to the offset of the integer tail call count (tcc)
instead of tcc_ptr.

Could this lead to the JIT loading the integer count (often 0) into REG_TCC
and dereferencing it as a pointer, resulting in a kernel panic?

>  
>  	emit_insn(ctx, addid, LOONGARCH_GPR_SP, LOONGARCH_GPR_SP, stack_adjust);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707101508.32283-1-yangtiezhu@loongson.cn?part=1