Re: [PATCH bpf 1/2] bpf: Preserve dynptr slice lifetime across rdonly_cast

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The verifier tracking logic intended to invalidate `bpf_rdonly_cast` aliases during dynptr stack overwrites can be bypassed by passing an unchecked `PTR_MAYBE_NULL` slice or by chaining casts.
--

commit 4dccf7918016a6c3b330526f04f7eae4bbeed8d8
Author: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
Subject: bpf: Preserve dynptr slice lifetime across rdonly_cast

This patch updates the verifier to track lifetime metadata across
bpf_rdonly_cast(). It attaches the ref_obj_id and dynptr_id to casted
registers and ensures typed aliases are invalidated when a dynptr stack
slot is overwritten.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -12082,8 +12088,17 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
>  			continue;
>  		}
>  
> -		if (is_kfunc_arg_ignore(btf, &args[i]) || is_kfunc_arg_implicit(meta, i))
> +		if (is_kfunc_arg_ignore(btf, &args[i]) || is_kfunc_arg_implicit(meta, i)) {
> +			if (meta->func_id == special_kfunc_list[KF_bpf_rdonly_cast] &&
> +			    i == 0 && is_spillable_regtype(reg->type)) {
> +				meta->rdonly_cast_src.id = reg->id;
> +				if (base_type(reg->type) == PTR_TO_MEM)
> +					meta->rdonly_cast_src.dynptr_id = reg->dynptr_id;

[Severity: Medium]
Could this logic be bypassed by chaining casts?

If a slice is first cast to a struct, the result is PTR_TO_BTF_ID and it
stores the dynptr_id in its id field.

If that result is then cast to void * (returning PTR_TO_MEM),
base_type(reg->type) == PTR_TO_MEM will be false, so
meta->rdonly_cast_src.dynptr_id remains 0.

The returned PTR_TO_MEM will get dynptr_id = 0, which would bypass the
invalidation checks in destroy_if_dynptr_stack_slot() entirely.

> +				meta->rdonly_cast_src.ref_obj_id = reg->ref_obj_id;
> +			}
> +
>  			continue;
> +		}
>  
>  		t = btf_type_skip_modifiers(btf, args[i].type, NULL);

[ ... ]

> @@ -12932,10 +12947,17 @@ static int check_special_kfunc(struct bpf_verifier_env *env, struct bpf_kfunc_ca
>  			regs[BPF_REG_0].type = PTR_TO_BTF_ID | PTR_UNTRUSTED;
>  			regs[BPF_REG_0].btf = desc_btf;
>  			regs[BPF_REG_0].btf_id = meta->arg_constant.value;
> +			regs[BPF_REG_0].id = meta->rdonly_cast_src.id;
> +			if (!regs[BPF_REG_0].id)
> +				regs[BPF_REG_0].id = meta->rdonly_cast_src.dynptr_id;

[Severity: Medium]
Does this correctly handle PTR_MAYBE_NULL slices passed to bpf_rdonly_cast?

Because bpf_rdonly_cast uses an __ign argument, it can accept an unchecked
PTR_MAYBE_NULL slice. These slices have a non-zero null-tracking id.

If meta->rdonly_cast_src.id is non-zero, this logic prioritizes the
null-tracking ID over dynptr_id. The returned typed alias will inherit the
null-tracking ID instead of the dynptr_id.

Later, when the parent dynptr is overwritten, the invalidation in
destroy_if_dynptr_stack_slot() will fail because it checks if the
dreg->id == dynptr_id.

> +			regs[BPF_REG_0].ref_obj_id = meta->rdonly_cast_src.ref_obj_id;
>  		} else if (btf_type_is_void(ret_t)) {
>  			mark_reg_known_zero(env, regs, BPF_REG_0);
>  			regs[BPF_REG_0].type = PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-f01-09-rdonly-cast-dynptr-lifetime-v1-0-2dd0a369e153@mails.tsinghua.edu.cn?part=1