Re: [PATCH bpf 09/11] bpf: Track whether dynptr type is known

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: missing type_unknown check in stacksafe() allows unsafe pruning
--

commit 8f00e7cb51fcf9e4513f3c3e9be943021ba739cf
Author: Emil Tsalapatis <emil@etsalapatis.com>

bpf: Track whether dynptr type is known

This commit adds a type_unknown field to struct bpf_reg_state's dynptr
state to distinguish locally created dynptrs from global subprog arguments.
It updates the verifier to set this flag, preparing for proper invalidation
of unknown dynptrs in subsequent commits.

> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index d5b4ab0bacd35..76aa724de1bcb 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -71,6 +71,7 @@ struct bpf_reg_state {
>  		/* For dynptr stack slots */
>  		struct {
>  			enum bpf_dynptr_type type;
> +			bool type_unknown;

[Severity: High]
Does this new field also need to be checked in stacksafe() when comparing
states?

Looking at stacksafe() in kernel/bpf/states.c, the STACK_DYNPTR case compares
type, first_slot, id, and parent_id, but it omits the new type_unknown field:

kernel/bpf/states.c:stacksafe() {
    ...
		case STACK_DYNPTR:
			old_reg = &old->stack[spi].spilled_ptr;
			cur_reg = &cur->stack[spi].spilled_ptr;
			if (old_reg->dynptr.type != cur_reg->dynptr.type ||
			    old_reg->dynptr.first_slot != cur_reg->dynptr.first_slot ||
			    !check_ids(old_reg->id, cur_reg->id, idmap) ||
			    !check_ids(old_reg->parent_id, cur_reg->parent_id, idmap))
				return false;
			break;
    ...
}

If a BPF program conditionally spills a locally created dynptr
(type_unknown=false) or an unknown argument dynptr (type_unknown=true) to
the same stack slot, could the verifier incorrectly treat the states as
equivalent and prune the unknown dynptr path? This might allow bypassing
the packet pointer invalidation logic when writing to unknown dynptrs,
potentially leading to a use-after-free of packet pointers.

>  			/* A dynptr is 16 bytes so it takes up 2 stack slots.
>  			 * We need to track which slot is the first slot
>  			 * to protect against cases where the user may try to

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916050830.8774-1-emil@etsalapatis.com?part=9