Re: [PATCH bpf-next v5 05/14] bpf: Refactor object relationship tracking and fix dynptr UAF bug

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Incorrect indexing of `__state->stack` for outgoing stack arguments in the `bpf_for_each_reg_in_vstate_mask` macro leads to out-of-bounds heap access and potential memory corruption.
--

commit 936dee8aae85e40857f1379da85e7a73536e2261
Author: Amery Hung <ameryhung@gmail.com>

bpf: Refactor object relationship tracking and fix dynptr UAF bug

This commit refactors object relationship tracking in the verifier and fixes
a dynptr use-after-free bug where file/skb dynptrs are not invalidated when
the parent referenced object is freed.

> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 2cb4c7b0eab68..d83b9fbfef7f0 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h

[ ... ]

> @@ -585,7 +591,7 @@ bpf_get_spilled_stack_arg(int slot, struct bpf_func_state *frame)
>  	     iter < frame->out_stack_arg_cnt;                          \
>  	     iter++, reg = bpf_get_spilled_stack_arg(iter, frame))
>  
> -#define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __mask, __expr)   \
> +#define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __stack, __mask, __expr)   \
>  	({                                                               \
>  		struct bpf_verifier_state *___vstate = __vst;            \
>  		int ___i, ___j;                                          \

[ ... ]

> @@ -600,19 +607,26 @@ bpf_get_spilled_stack_arg(int slot, struct bpf_func_state *frame)
>  			bpf_for_each_spilled_reg(___j, __state, __reg, __mask) { \
>  				if (!__reg)                              \
>  					continue;                        \
> +				__stack = &__state->stack[___j];         \
>  				(void)(__expr);                          \
>  			}                                                \
>  			bpf_for_each_spilled_stack_arg(___j, __state, __reg) { \
>  				if (!__reg)                              \
>  					continue;                        \
> +				__stack = &__state->stack[___j];         \

[Severity: Critical]
Is it safe to index the stack array using the out_stack_arg_cnt index here?

In the bpf_for_each_spilled_stack_arg loop, ___j represents the index into
the stack_arg_regs array, which is bounded by out_stack_arg_cnt. However,
this line sets __stack = &__state->stack[___j], which indexes the local
function stack array (bounded by allocated_stack / BPF_REG_SIZE).

Since a BPF program can push stack arguments without allocating a local
stack, ___j might exceed the local stack size, computing an out-of-bounds
pointer.

When release_reference() iterates over these registers, it evaluates
stack->slot_type[BPF_REG_SIZE - 1], which reads out-of-bounds memory.
If this memory coincidentally contains the value for STACK_DYNPTR (2),
could this result in invalidate_dynptr() being called erroneously,
writing STACK_INVALID across multiple slots and corrupting adjacent
kernel memory?

>  				(void)(__expr);                          \
>  			}						 \
>  		}                                                        \
>  	})

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519181314.2731658-1-ameryhung@gmail.com?part=5