Re: [PATCH 6.6.y 1/2] bpf: Invalidate dynptr slices by dynptr_id on release

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added `invalidate_slices_of_dynptr()` accesses `dreg->dynptr_id` without verifying that `dreg->type` is `PTR_TO_MEM` or `PTR_TO_MEM | PTR_MAYBE_NULL`. Because `dynptr_id` is part of a union in `struct bpf_reg_state`, this leads to unsafe alias reads and incorrect register invalidation.
--

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

bpf: Invalidate dynptr slices by dynptr_id on release

This patch invalidates dynptr slices by matching dynptr_id when a dynptr is
released. It adds invalidate_slices_of_dynptr() to iterate over registers
and invalidate those matching the released dynptr_id.

Link: https://lore.kernel.org/r/20260529014936.2811085-6-ameryhung@gmail.com

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> +static void invalidate_slices_of_dynptr(struct bpf_verifier_env *env, int dynptr_id)
> +{
> +	struct bpf_func_state *fstate;
> +	struct bpf_reg_state *dreg;
> +
> +	bpf_for_each_reg_in_vstate(env->cur_state, fstate, dreg, ({
> +		if (dreg->dynptr_id == dynptr_id)
[Severity: High]
Is it safe to read dreg->dynptr_id here without checking the register type?

Because dynptr_id is part of a union in struct bpf_reg_state, it aliases
other fields like btf_id and map_uid (especially on 32-bit systems where
these fields occupy the same bytes).

If a register holds a PTR_TO_BTF_ID or an inner map PTR_TO_MAP_VALUE, and
its btf_id or map_uid happens to match the released dynptr_id, could this
result in a false positive that invalidates an unrelated valid register?

For comparison, destroy_if_dynptr_stack_slot() restricts this check:

    if (dreg->type != (PTR_TO_MEM | PTR_MAYBE_NULL) &&
        dreg->type != PTR_TO_MEM)
        continue;
    if (dreg->dynptr_id == dynptr_id)
        mark_reg_invalid(env, dreg);

Should invalidate_slices_of_dynptr() include a similar type check before
accessing dynptr_id?

> +			mark_reg_invalid(env, dreg);
> +	}));
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829032754.2324760-1-xyx2021@mail.ustc.edu.cn?part=1