Re: [PATCH v9 bpf-next 06/10] btf: support kernel parsing of BTF with layout info

[email protected]
Newsgroups org.kernel.vger.dwarves,org.kernel.vger.bpf
Message-ID <dc1d9fb29ac2eba86b21fb85ad1bac9d096f0e9680ef87b42a86200120b5e7a1@mail.kernel.org>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c

[ ... ]

> @@ -5327,23 +5328,36 @@ static s32 btf_check_meta(struct btf_verifier_env *env,
>  		return -EINVAL;
>  	}
>
> +	if (!btf_name_offset_valid(env->btf, t->name_off)) {
> +		btf_verifier_log(env, "[%u] Invalid name_offset:%u",
> +				 env->log_type_id, t->name_off);
> +		return -EINVAL;
> +	}
> +
> +	if (BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
>  		btf_verifier_log(env, "[%u] Invalid kind:%u",
>  				 env->log_type_id, BTF_INFO_KIND(t->info));
>  		return -EINVAL;
>  	}
>
> +	if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX && env->btf->layout &&
> +	    ((BTF_INFO_KIND(t->info) + 1) * sizeof(struct btf_layout)) <
> +	     env->btf->hdr.layout_len) {
                                       ^

Should this be <= instead of < ?  If the layout section has exactly
N entries, layout_len is N * sizeof(struct btf_layout).  For the
last covered kind (K = N - 1), this computes:

    (K + 1) * sizeof(struct btf_layout) == layout_len

With < that evaluates to false, so the last kind covered by the
layout section falls through to the else branch and gets logged as
"Invalid kind" instead of "unknown but required kind".

The libbpf equivalent in btf_type_size_unknown() uses:

    __u32 layout_kinds = btf->hdr.layout_len / sizeof(struct btf_layout);
    if (!l || kind >= layout_kinds) {

which accepts the last entry.  Using <= here would match that
behavior.

> +		btf_verifier_log(env, "[%u] unknown but required kind %u",
> +				 env->log_type_id,
> +				 BTF_INFO_KIND(t->info));
>  		return -EINVAL;
> +	} else {
> +		if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX) {
> +			btf_verifier_log(env, "[%u] Invalid kind:%u",
> +					 env->log_type_id, BTF_INFO_KIND(t->info));
> +			return -EINVAL;
> +		}
> +		var_meta_size = btf_type_ops(t)->check_meta(env, t, meta_left);
> +		if (var_meta_size < 0)
> +			return var_meta_size;
>  	}


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22436234361

AI-authorship-score: low
AI-authorship-explanation: Multi-year revision history with detailed reviewer interactions, idiomatic kernel C, and an off-by-one error typical of manual arithmetic translation are inconsistent with AI-generated code.
issues-found: 1
issue-severity-score: low
issue-severity-explanation: Off-by-one in layout bounds check produces wrong verifier log message but does not change functional behavior since both paths return -EINVAL.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.