Re: [PATCH bpf-next v4 15/18] bpf,x86: Implement JIT support for stack arguments

[Yonghong Song <yonghong.song@linux.dev>]

On 4/11/26 10:43 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index 32864dbc2c4e..ec57b9a6b417 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
> [ ... ]
>
>> @@ -1664,16 +1699,48 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image, u8 *rw_image
> [ ... ]
>
>> +	/*
>> +	 * The verifier guarantees that neither tail_call_reachable nor
>> +	 * priv_stack is set when outgoing stack args exist, so R9 is
>> +	 * always available.
> [ ... ]
>
>> @@ -2243,6 +2377,18 @@ st:			if (is_imm8(insn->off))
>>   		case BPF_LDX | BPF_PROBE_MEMSX | BPF_H:
>>   		case BPF_LDX | BPF_PROBE_MEMSX | BPF_W:
>>   			insn_off = insn->off;
>> +			if (adjust_stack_arg_off) {
>> +				if (insn_off == 8) {
>> +					/* Incoming arg 6: read from r9 */
>> +					EMIT_mov(dst_reg, X86_REG_R9);
>> +					break;
>> +				}
> [ ... ]
>
>> @@ -3757,7 +3907,13 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>   		prog->aux->jit_data = jit_data;
>>   	}
>>   	priv_stack_ptr = prog->aux->priv_stack_ptr;
>> -	if (!priv_stack_ptr && prog->aux->jits_use_priv_stack) {
>> +	/*
>> +	 * x86-64 uses R9 for both private stack frame pointer and
>> +	 * outgoing arg 6, so disable private stack when outgoing
>> +	 * stack args are present.
>> +	 */
>> +	if (!priv_stack_ptr && prog->aux->jits_use_priv_stack &&
>> +	    prog->aux->stack_arg_depth == prog->aux->incoming_stack_arg_depth) {
> This condition disables priv_stack when outgoing stack args are present,
> but R9 is also used for incoming arg 6. Could a subprog with incoming
> stack args (incoming_stack_arg_depth > 0) and no outgoing stack args
> still get priv_stack allocated here?
>
> The condition stack_arg_depth == incoming_stack_arg_depth is true when
> outgoing is zero, regardless of the incoming depth. When priv_stack is
> allocated, emit_priv_frame_ptr() in the prologue overwrites R9:
>
>    emit_mov_imm64(&prog, X86_REG_R9, ...priv_frame_ptr...);
>
> Later, the LDX handler for incoming arg 6 reads R9 expecting the
> caller's 6th argument:
>
>    if (insn_off == 8) {
>        EMIT_mov(dst_reg, X86_REG_R9);
>        break;
>    }
>
> For a subprog with 6+ args and >= 64 bytes of stack (triggering
> PRIV_STACK_ADAPTIVE in check_max_stack_depth_subprog()), R9 would
> contain the private stack frame pointer instead of the incoming arg 6
> value.
>
> The comment also says "outgoing arg 6" but should this also mention
> incoming arg 6, since both use R9?
>
> Would changing the condition to check that stack_arg_depth is zero
> (no stack args at all) be the right fix here?

You are right. Previous approach is to copy *all* 6+ arguments to the
stack for bpf-to-bpf, in which case, R9 will not be used.

But the new approach is to following x86_64 calling convention for
bpf-to-bpf call as well. In this case, indeed, we should disable
private stack if stack_arg_depth is not zero.

>
>>   		/* Allocate actual private stack size with verifier-calculated
>>   		 * stack size plus two memory guards to protect overflow and
>>   		 * underflow.
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24299298635