Re: [PATCH 6.12.y v2 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` function reads the `dynptr_id` union field without verifying the register's type, causing it to erroneously invalidate unrelated registers due to memory aliasing.
--

commit e9eb0688f5ef0c81c32aeb93616da3b5719b04da
Author: Amery Hung <ameryhung@gmail.com>
bpf: Invalidate dynptr slices by dynptr_id on release

This patch invalidates dynptr slices when their parent dynptr is released by
walking registers with a matching dynptr_id in unmark_stack_slots_dynptr().
This addresses an issue where LDX/STX through slices remains allowed after
submitting or discarding the dynptr.

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

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 99b3f539e431f..5d0874a09a8db 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -862,6 +874,17 @@ static void mark_reg_invalid(const struct bpf_verifier_env *env, struct bpf_reg_
>  		__mark_reg_unknown(env, reg);
>  }
>  
> +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]
Does this code risk misinterpreting union fields in unrelated registers
due to memory aliasing in invalidate_slices_of_dynptr()?

Because dynptr_id shares a union with other fields in struct bpf_reg_state
(such as btf_id on 32-bit architectures, or first_slot for dynptrs), if the
register is not explicitly checked for type PTR_TO_MEM, this comparison
might match unrelated registers.

For instance, a program that acquires a dynptr reference (assigning it
id_gen = 1) could use a register where the union happens to alias to 1
(like a CONST_PTR_TO_DYNPTR callback argument where first_slot = true, or a
PTR_TO_BTF_ID where the BTF ID is 1). Releasing the dynptr triggers
invalidate_slices_of_dynptr(env, 1), which unconditionally checks
dreg->dynptr_id == 1 and misinterprets the aliased union field as a match.
This could incorrectly invalidate valid registers, leading to spurious
verification failures.

Could we add a check for dreg->type before accessing dreg->dynptr_id to
prevent this?

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

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