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

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2025-12-15 at 09:17 +0000, Alan Maguire wrote:

If strict kind layout checks are the goal, would it make sense to
check sizes declared in kind_layout for known types?

[...]

> @@ -5215,23 +5216,36 @@ static s32 btf_check_meta(struct btf_verifier_env *env,
>  		return -EINVAL;
>  	}
>  
> -	if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX ||
> -	    BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
> +	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_name_offset_valid(env->btf, t->name_off)) {
> -		btf_verifier_log(env, "[%u] Invalid name_offset:%u",
> -				 env->log_type_id, t->name_off);
> +	if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX && env->btf->kind_layout &&
> +	    ((BTF_INFO_KIND(t->info) + 1) * sizeof(struct btf_kind_layout)) <
> +	     env->btf->hdr.kind_layout_len) {
> +		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;
>  	}
>  
> -	var_meta_size = btf_type_ops(t)->check_meta(env, t, meta_left);
> -	if (var_meta_size < 0)
> -		return var_meta_size;
> -
>  	meta_left -= var_meta_size;

It appears that a smaller change here would achieve same results:

    -        if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX ||
    +        u32 layout_kind_max = env->btf->hdr.kind_layout_len / sizeof(struct btf_kind_layout);
    +        if (BTF_INFO_KIND(t->info) > layout_kind_max ||
                 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) {
    +                btf_verifier_log(env, "[%u] unknown but required kind %u",
    +                                 env->log_type_id,
    +                                 BTF_INFO_KIND(t->info));
    +        }
    +
             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);

wdyt?

But tbh, the "unknown but required kind" message seems unnecessary,

>  
>  	return saved_meta_left - meta_left;
> @@ -5405,7 +5419,8 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
>  	start = btf->nohdr_data + hdr->str_off;
>  	end = start + hdr->str_len;
>  
> -	if (end != btf->data + btf->data_size) {
> +	if (hdr->hdr_len < sizeof(struct btf_header) &&
> +	    end != btf->data + btf->data_size) {

Given that btf_check_sec_info() checks for overlap and total size,
is this check needed at all?

>  		btf_verifier_log(env, "String section is not at the end");
>  		return -EINVAL;
>  	}

[...]