Re: Re: [PATCH bpf-next 1/2] bpf: Preserve pointer spill metadata during half-slot cleanup

["Nuoqi Gui" <gnq25@mails.tsinghua.edu.cn>]

> -----Original Messages-----
> From: bot+bpf-ci@kernel.org
> Send time:Monday, 15/06/2026 16:48:49
> To: gnq25@mails.tsinghua.edu.cn, ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, eddyz87@gmail.com, memxor@gmail.com, martin.lau@linux.dev, song@kernel.org, yonghong.song@linux.dev, jolsa@kernel.org, emil@etsalapatis.com, shuah@kernel.org
> Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, gnq25@mails.tsinghua.edu.cn, ast@kernel.org, andrii@kernel.org, daniel@iogearbox.net, martin.lau@kernel.org, eddyz87@gmail.com, yonghong.song@linux.dev, clm@meta.com, ihor.solodrai@linux.dev
> Subject: Re: [PATCH bpf-next 1/2] bpf: Preserve pointer spill metadata during half-slot cleanup
> 
> >     bpf: Preserve pointer spill metadata during half-slot cleanup
> >
> >     __clean_func_state() cleans dead stack slots in 4-byte halves. When the
> >     high half of a STACK_SPILL slot is dead and the low half remains live,
> >     cleanup converts the live low half to STACK_MISC or STACK_ZERO and clears
> >     the saved spilled_ptr metadata.
> >
> >     That conversion is safe only for scalar spills. For a pointer spill, this
> >     metadata clear lets a later 32-bit fill from the still-live half avoid the
> >     normal non-scalar register-fill check and be treated as an ordinary scalar
> >     stack read.
> >
> >     Leave non-scalar spill slots intact in this half-live shape. This is
> >     conservative for pruning and preserves the existing
> >     check_stack_read_fixed_off() rejection path for partial fills from pointer
> >     spills.
> >
> >     Fixes: 2cb27158adb3 ("bpf: poison dead stack slots")
> >     Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
> 
> Is the Fixes: tag pointing at the right commit?
> 
> The commit referenced, 2cb27158adb3 ("bpf: poison dead stack slots"), adds
> STACK_POISON handling in kernel/bpf/verifier.c and does not contain the
> half-slot cleanup logic being corrected here.
> 
> The 4-byte half-slot cleanup in __clean_func_state() that downgrades a
> still-live STACK_SPILL low half to STACK_MISC/STACK_ZERO and clears the
> saved spilled_ptr metadata without checking for SCALAR_VALUE was introduced
> by be23266b4a08 ("bpf: 4-byte precise clean_verifier_state"). That commit
> added the lo_live/hi_live model and the exact block this patch corrects:
> 
> 	if (lo_live && stype == STACK_SPILL) {
> 
> be23266b4a08 predates both 2cb27158adb3 and this fix. Should the tag be:
> 
>   Fixes: be23266b4a08 ("bpf: 4-byte precise clean_verifier_state")
> 
> 
> ---
> 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/27533304560

Yes, be23266b4a08 is the right Fixes target.


The bug is in the half-slot cleanup logic introduced by that commit:
when the high half is dead and the low half remains live, it can downgrade
the still-live STACK_SPILL bytes to STACK_MISC/STACK_ZERO and clear the
spilled_ptr metadata.

2cb27158adb3 only changed the dead-slot marker to STACK_POISON and did not
introduce that half-slot spill cleanup behavior.

I will update the tag to:
Fixes: be23266b4a08 ("bpf: 4-byte precise clean_verifier_state")