Re: [PATCH bpf-next v2 1/2] bpf: Reject MEM_ALLOC BTF accesses past object bounds

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] btf_struct_ids_match() hardcodes `is_alloc=false`, bypassing the MEM_ALLOC bounds protection and allowing out-of-bounds kfunc accesses.
--

commit ea4066197ea8dc72df56252849c8752b81c506d6
Author: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>

bpf: Reject MEM_ALLOC BTF accesses past object bounds

This patch modifies the BTF struct walk logic to reject memory accesses past
the object bounds when dealing with MEM_ALLOC objects. It disables the
trailing flexible array relaxation for these allocated objects, keeping
verifier-approved BTF accesses strictly within the bytes provided by the
allocation kfunc.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 15ae7c43f594b..3e68af9c1a5eb 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c

[ ... ]

> @@ -7441,7 +7445,7 @@ bool btf_struct_ids_match(struct bpf_verifier_log *log,
>  	type = btf_type_by_id(btf, id);
>  	if (!type)
>  		return false;
> -	err = btf_struct_walk(log, btf, type, off, 1, &id, &flag, NULL);
> +	err = btf_struct_walk(log, btf, type, off, 1, &id, &flag, NULL, false);

[Severity: High]
This isn't a bug introduced by this patch, but hardcoding false for the
is_alloc parameter here completely bypasses the newly introduced strict
bounds check for MEM_ALLOC types during struct matching.

Can this allow a BPF program to obtain a pointer to unallocated slab memory
and pass it to a kernel kfunc or store it in a map kptr?

For example, in kernel/bpf/verifier.c:check_kfunc_args(), it calls
btf_struct_ids_match() with reg->var_off.value without conveying that
the register possesses the MEM_ALLOC flag:

kernel/bpf/verifier.c:check_kfunc_args() {
        ...
        struct_same = btf_struct_ids_match(&env->log, reg_btf, reg_ref_id,
                                           reg->var_off.value, meta->btf,
                                           ref_id, strict_type_match);
        ...
}

A similar path exists in map_kptr_match_type() when storing into
unreferenced kptrs (where strict is false).

Does btf_struct_ids_match() need a way to know if the register type is
MEM_ALLOC so it can properly apply the bounds protection here as well?

>  	if (err != WALK_STRUCT)
>  		return false;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782197377.git.chenyy23@mails.tsinghua.edu.cn?part=1