Re: [PATCH bpf-next v6 1/2] bpf: Destroy callee-local dynptrs on subprog return

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 10/2/26 4:29 AM, Alexei Starovoitov wrote:
> On Mon Sep 28, 2026 at 4:48 AM UTC, Amery Hung wrote:
>> On Sun, Sep 27, 2026 at 5:04 AM Xu Yunxiang <xyx2021@mail.ustc.edu.cn> wrote:
>>>
>>> A data slice is valid only while the specific dynptr that produced it
>>> remains valid. prepare_func_exit() copies a subprogram return value into
>>> the caller and then frees the callee state without applying the normal
>>> destruction semantics to dynptrs in the callee stack.
>>>
>>> A callee can therefore derive a slice from a local dynptr, return or spill
>>> the slice into its caller, and then let the dynptr disappear with the
>>> callee frame. The slice retains the id of a dynptr that is no longer in the
>>> verifier state, so the verifier continues to accept accesses through it.
>>>
>>> Before the object relationship refactor, bpf_dynptr_data() slices from
>>> referenced dynptrs also carried the shared reference id, allowing a later
>>> release to catch some of these escaped slices. The refactor made those
>>> slices precise children of their source dynptr and exposed the missing
>>> teardown as an accepted stale access even after the shared resource is
>>> released.
>>>
>>> Add destroy_dynptrs_in_stack_slots() to apply the existing dynptr teardown
>>> to an inclusive range of stack slots. Reuse it for dynptr initialization,
>>> variable-offset stack writes, and the entire callee stack before freeing
>>> the frame. The teardown rejects losing the last dynptr for a referenced
>>> resource and invalidates the dynptr and all its descendants, including
>>> slices held in the caller. Propagate errors through prepare_func_exit().
>>>
>>> Reuse the existing teardown API and diagnostic. Update the affected
>>> callback test expectation in this change so the commit remains test-clean.
>>>
>>> Fixes: 308c7a0ae885 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
>>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>>> Closes: https://lore.kernel.org/r/20260829040712.B1B511F000E9@smtp.kernel.org
>>> Suggested-by: Amery Hung <ameryhung@gmail.com>
>>> Link: https://lore.kernel.org/r/CAMB2axOmdHwSrHihSRskDywkCmEGQOX+6dZPN_A9u-HhxY3UDA@mail.gmail.com
>>> Link: https://lore.kernel.org/r/CAMB2axMmrTb+UZ84UD48MwMtXbk1s9bWtreU3AOfjzU0fPT0BA@mail.gmail.com
>>> Assisted-by: LLM
>>> Signed-off-by: Xu Yunxiang <xyx2021@mail.ustc.edu.cn>
>>
>> Reviewed-by: Amery Hung <ameryhung@gmail.com>
>>
>> Just a note: this series addresses a related but distinct problem from
>> Ihor’s work [0]. This series handles descendants escaping from
>> callee-local stack dynptrs, while Ihor’s handles helper-owned callback
>> arguments whose lifetime ends when the callback returns.
>>
>> [...]
>>
>>> @@ -11675,6 +11683,12 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
>>>                 }
>>>         }
>>>
>>> +       /* Invalidate callee-local dynptrs and their slices before the frame goes away. */
>>> +       err = destroy_dynptrs_in_stack_slots(env, callee, 0,
>>> +                                            callee->allocated_stack / BPF_REG_SIZE - 1);
>>> +       if (err)
>>> +               return err;
>>> +
>>
>> Conceptually, Ihor’s frame-reference mechanism could subsume this
>> cleanup if every local dynptr had a separate frame-ownership
>> relationship. The current parent_id cannot express that directly
>> because dynptrs already use it for object and shared-resource
>> ancestry.
>> Supporting both would require another ownership edge or field, rather
>> than simply assigning parent_id to the REF_TYPE_FRAME id.
>>
>> [0] https://lore.kernel.org/r/20260922010333.1226537-1-ihor.solodrai@linux.dev
> 
> 
> I feel it's ok to apply as-is or should we defer until Ihor's fix?

Let's apply this fix as is.

Acked-by: Ihor Solodrai <ihor.solodrai@linux.dev>

It can probably be backported, and there are no conflicts with what
I'm doing.

The REF_TYPE_FRAME change is a more long-term thing, not easily
backportable. And it's not ready to land yet, so no reason to wait.

Thanks.

> 
> pw-bot: cr