Re: [PATCH bpf-next v2 09/23] bpf: Extend liveness analysis to track stack argument slots

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] record_load_store_access() mistakenly uses the tracking state of stack argument slot 0 as the base pointer for R11-based memory accesses, leading to spurious liveness marks.
- [Low] Missing verifier logging for stack argument liveness state transitions.
--

commit 00a5fce117daa13f26c61ec5ffa3622241608fc1
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Extend liveness analysis to track stack argument slots

This commit extends liveness analysis to handle stack argument slots, preventing
pointers passed through stack args from incorrectly poisoning the pointed-to
stack slots. It explicitly handles BPF_REG_PARAMS (R11) to avoid out-of-bounds
accesses and extends tracking arrays.

> diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> index de0b4c6263ff2..eed7260f2bb0a 100644
> --- a/kernel/bpf/liveness.c
> +++ b/kernel/bpf/liveness.c
> @@ -610,6 +610,24 @@ enum arg_track_state {
>  /* Track callee stack slots fp-8 through fp-512 (64 slots of 8 bytes each) */
>  #define MAX_ARG_SPILL_SLOTS 64
>  
> +/* Track stack arg slots: outgoing starts at -(i+1)*8, incoming at +(i+1)*8 */
> +#define MAX_STACK_ARG_SLOTS (MAX_BPF_FUNC_ARGS - MAX_BPF_FUNC_REG_ARGS)
> +
> +/*
> + * Combined register + stack arg tracking: R0-R10 at indices 0-10,
> + * outgoing stack arg slots at indices MAX_BPF_REG..MAX_BPF_REG+6.
> + */
> +#define MAX_AT_TRACK_REGS (MAX_BPF_REG + MAX_STACK_ARG_SLOTS)

Could this repurposed index cause a spurious liveness mark in
record_load_store_access()?

Because at[MAX_BPF_REG] is repurposed to track the value stored in outgoing
stack argument slot 0, what happens when record_load_store_access() processes
stack argument instructions like is_stack_arg_stx()?

record_load_store_access() does this:

	case BPF_STX:
		if (BPF_MODE(insn->code) == BPF_ATOMIC) {
			...
		} else {
			ptr = &at[insn->dst_reg];
			sz = -sz;
		}

If the instruction is a stack argument access, insn->dst_reg is BPF_REG_PARAMS
(which equals MAX_BPF_REG). It will fetch &at[MAX_BPF_REG]. If a BPF program
previously stored a frame pointer (FP) into slot 0, at[MAX_BPF_REG] will reflect
that FP.

Does this mean record_load_store_access() will spuriously treat this FP as the
base pointer and incorrectly mark the resulting local stack frame slot as
accessed, corrupting the local stack liveness mask?

[ ... ]
> @@ -1616,7 +1663,8 @@ static int compute_subprog_args(struct bpf_verifier_env *env,
>  		memcpy(at_out, at_in[i], sizeof(at_out));
>  		memcpy(at_stack_out, at_stack_in[i], MAX_ARG_SPILL_SLOTS * sizeof(*at_stack_out));
>  
> -		arg_track_xfer(env, insn, idx, at_out, at_stack_out, instance, callsites);
> +		arg_track_xfer(env, insn, idx, at_out, at_stack_out,
> +			       at_stack_arg_entry, instance, callsites);
>  		arg_track_log(env, insn, idx, at_in[i], at_stack_in[i], at_out, at_stack_out);

Are the state transitions for stack arguments logged here?

In arg_track_log(), the loop for logging register transitions is strictly
bounded by MAX_BPF_REG:

	for (i = 0; i < MAX_BPF_REG; i++) {
		if (arg_track_eq(&at_out[i], &at_in[i]))
			continue;

Since the tracking arrays at_in and at_out were extended to MAX_AT_TRACK_REGS to
include state for outgoing stack arguments at indices MAX_BPF_REG through
MAX_AT_TRACK_REGS - 1, does arg_track_log() silently omit logging their state
transitions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507212942.1122000-1-yonghong.song@linux.dev?part=9