Re: [PATCH bpf-next v2 07/23] bpf: Add precision marking and backtracking for stack argument slots

On 5/9/26 6:05 AM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Incorrect tracking and propagation of stack argument precision at subprogram boundaries in `backtrack_insn()`.
> --
>
> commit 82222fc9756a9a239fae1b810efc4aa155d2973f
> Author: Yonghong Song <[email protected]>
>
>      bpf: Add precision marking and backtracking for stack argument slots
>
> This commit extends the precision marking and backtracking infrastructure to
> support stack argument slots (r11-based accesses). It ensures precision
> demands for scalar values passed through stack arguments are properly
> tracked.
>
>> diff --git a/kernel/bpf/backtrack.c b/kernel/bpf/backtrack.c
>> index 93738a076c888..17f254a02831f 100644
>> --- a/kernel/bpf/backtrack.c
>> +++ b/kernel/bpf/backtrack.c
> [ ... ]
>> @@ -336,6 +351,19 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>>   			return 0;
>>   		bt_clear_reg(bt, load_reg);
>>   
>> +		if (hist && hist->flags & INSN_F_STACK_ARG_ACCESS) {
>> +			spi = hist->spi;
>> +			/*
>> +			 * Stack arg read: callee reads from r11+off, but
>> +			 * the data lives in the caller's stack_arg_regs.
>> +			 * Set the mask in the caller frame so precision
>> +			 * is marked in the caller's slot at the callee
>> +			 * entry checkpoint.
>> +			 */
>> +			bt_set_frame_stack_arg_slot(bt, bt->frame - 1, spi);
>> +			return 0;
>> +		}
>> +
> When backtracking a callee's read of an incoming stack argument via BPF_LDX,
> the logic correctly sets the mask in the caller frame (bt->frame - 1).
>
> [ ... ]
>> @@ -426,6 +465,18 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>>   						bpf_bt_set_frame_reg(bt, bt->frame - 1, i);
>>   					}
>>   				}
>> +				/*
>> +				 * propagate callee's incoming stack arg precision
>> +				 * to caller's outgoing stack arg slots
>> +				 */
>> +				if (bt_stack_arg_mask(bt)) {
>> +					for (i = 0; i < MAX_BPF_FUNC_ARGS - MAX_BPF_FUNC_REG_ARGS; i++) {
>> +						if (!bt_is_frame_stack_arg_slot_set(bt, bt->frame, i))
>> +							continue;
>> +						bt_clear_frame_stack_arg_slot(bt, bt->frame, i);
>> +						bt_set_frame_stack_arg_slot(bt, bt->frame - 1, i);
>> +					}
>> +				}
> Does this propagation logic operate on the correct mask?
>
> Since the callee's BPF_LDX instructions already marked the incoming stack
> arguments directly in the caller's frame (bt->frame - 1),
> bt_stack_arg_mask(bt) for the current frame (bt->frame) shouldn't contain
> any incoming stack arguments here.

You are right here. bt_is_frame_stack_arg_slot_set(bt, bt->frame, i) should be 0 so
the above whole loop is not necessary.

>
> If bt_stack_arg_mask(bt) is non-zero at this point, wouldn't that indicate
> leftover uninitialized outgoing stack arguments from the callee that were
> never matched by a BPF_STX?
>
> Instead of propagating these leftovers to the caller frame, should this
> condition be treated as a verifier_bug(), similar to how leftover stack
> slots are handled just above this code?

Yes, verifier_bug() makes sense to capture potential issues. This will
be similar to potential bugs with register spills.

>