BPF List
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: bot+bpf-ci@kernel.org, bpf@vger.kernel.org
Cc: ast@kernel.org, andrii@kernel.org, daniel@iogearbox.net,
	eddyz87@gmail.com, kernel-team@fb.com, martin.lau@kernel.org,
	clm@meta.com, ihor.solodrai@linux.dev
Subject: Re: [PATCH bpf-next v3 05/11] bpf: Report which member makes a kfunc return type unsupported
Date: Fri, 28 Aug 2026 10:45:47 -0700	[thread overview]
Message-ID: <6577c990-f491-43fd-bcef-314160ea5c77@linux.dev> (raw)
In-Reply-To: <a9b017abf99b3c4062bdb77e8fbbfbb7dff502ddf0b6f0b4bc459cb28a4cac8e@mail.kernel.org>



On 8/27/26 12:04 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 90139f1b78d10..967ad010b3222 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -11646,6 +11646,15 @@ static bool is_kfunc_arg_implicit(const struct bpf_call_arg_meta *meta, u32 arg_
>>   	return argn <= arg_idx;
>>   }
>>
>> +#define BTF_MEMBER_MAX_DEPTH	4
>> +#define BTF_MEMBER_PATH_LEN	64
>> +
>> +struct btf_member_path {
>> +	const struct btf_member *member[BTF_MEMBER_MAX_DEPTH];
>> +	int depth;
>> +	bool too_deep;
>> +};
>> +
> [ ... ]
>
>> @@ -11676,31 +11686,42 @@ static bool btf_struct_member_walk(struct bpf_verifier_env *env, const struct bt
>>
>>   		member_type = btf_type_skip_modifiers(btf, member->type, NULL);
>>   		if (btf_type_is_struct(member_type)) {
>> -			if (rec >= 3) {
>> +			if (rec >= BTF_MEMBER_MAX_DEPTH - 1) {
>>   				verbose(env, "max struct nesting depth exceeded\n");
>> +				if (path)
>> +					path->too_deep = true;
>>   				return false;
>>   			}
>> -			if (!btf_struct_member_walk(env, btf, member_type, member_kinds, rec + 1))
>> -				return false;
>> +			if (!btf_struct_member_walk(env, btf, member_type, member_kinds,
>> +						    rec + 1, path))
>> +				goto bad_path;
>>   			continue;
>>   		}
>>   		if (btf_type_is_array(member_type)) {
>>   			array = btf_array(member_type);
>>   			if (!array->nelems)
>> -				return false;
>> +				goto bad_member;
>>   			member_type = btf_type_skip_modifiers(btf, array->type, NULL);
>>   		}
>>   		if (!btf_member_kind_allowed(btf, member_type, member_kinds))
>> -			return false;
>> +			goto bad_member;
>>   	}
>>   	return true;
>> +
>> +bad_member:
>> +	if (path)
>> +		path->depth = rec + 1;
>> +bad_path:
>> +	if (path && path->depth)
>> +		path->member[rec] = member;
>> +	return false;
>>   }
> [ ... ]
>
>> @@ -14077,17 +14114,46 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>   		    meta.func_id == special_kfunc_list[KF_bpf_res_spin_lock_irqsave]))
>>   			__mark_reg_const_zero(env, &regs[BPF_REG_0]);
>>   	} else if (btf_type_is_struct(t)) {
>> +		struct btf_member_path path = {};
>> +		const char *member_note = "";
>> +
>>   		/*
>>   		 * The returned struct comes back as raw register bits modeled
>>   		 * as an unknown scalar, so it must contain only scalars:
>>   		 * otherwise a pointer field would be laundered into a scalar
>>   		 * and escape provenance and reference tracking.
>>   		 */
>> -		if (!btf_type_is_scalar_struct(env, desc_btf, t)) {
>> +		if (!btf_struct_member_walk(env, desc_btf, t, BTF_MEMBER_SCALAR, 0, &path)) {
>>   			verbose(env,
>>   				"kernel function %s returns %s %s that is not composed of scalars\n",
>>   				func_name, btf_type_str(t),
>>   				btf_name_by_offset(desc_btf, t->name_off));
>> +			if (path.too_deep) {
>> +				member_note = bpf_diag_fmt(
>> +					env, " It nests structs more than %d levels deep.",
>> +					BTF_MEMBER_MAX_DEPTH);
>> +			} else if (path.depth) {
>> +				const struct btf_member *bad = path.member[path.depth - 1];
>> +				char bad_name[BTF_MEMBER_PATH_LEN];
>> +				const struct btf_type *bad_type;
>> +
>> +				btf_member_path_str(desc_btf, &path, bad_name, sizeof(bad_name));
>> +				bad_type = btf_type_skip_modifiers(desc_btf, bad->type, NULL);
>> +				verbose(env, "member '%s' has type %s\n", bad_name,
>> +					btf_type_str(bad_type));
> For an array member, does this report the type that the walk actually
> rejected?
>
> Looking at btf_struct_member_walk(), when the member is an array it
> resolves the element type before testing it:
>
>      if (btf_type_is_array(member_type)) {
>          array = btf_array(member_type);
>          if (!array->nelems)
>              goto bad_member;
>          member_type = btf_type_skip_modifiers(btf, array->type, NULL);
>      }
>      if (!btf_member_kind_allowed(btf, member_type, member_kinds))
>          goto bad_member;
>
> The walk stores only the struct member itself in the path, discarding
> the resolved element type. So for a kfunc returning:
>
>      struct s { void *p[2]; };
>
> the walk rejects the element type PTR, but this diagnostic derives
> bad_type from bad->type (the array type), printing:
>
>      member 'p' has type ARRAY
>      ... Its member 'p' is ARRAY, not a scalar.
>
> That contradicts the rule documented earlier ("An array member counts
> as its element type") and the accepted behaviour of __u64 a[2], which
> passes. The same issue affects struct inner arr[2] (reported as ARRAY,
> real reason is the element is a non-scalar struct) and zero-length
> arrays, whose rejection has nothing to do with type at all.

This is indeed a problem. Will fix.

>
> This is confined to verifier log text - the accept/reject decision is
> unchanged - but the message the patch exists to add is inaccurate for
> array members.
>
>> +				member_note = bpf_diag_fmt(
>> +					env, " Its member '%s' is %s, not a scalar.", bad_name,
>> +					btf_type_str(bad_type));
>> +			}
> [ ... ]
>
>
> ---
> 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/33045482525


  reply	other threads:[~2026-08-28 17:45 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-27  6:11 [PATCH bpf-next v3 00/11] bpf: Allow arena pointers in by-value returns Yonghong Song
2026-08-27  6:11 ` [PATCH bpf-next v3 01/11] bpf: Record each half of a paired return value in verifier diagnostics Yonghong Song
2026-08-27  6:11 ` [PATCH bpf-next v3 02/11] bpf: Drop the recursion depth argument of btf_type_is_scalar_struct() Yonghong Song
2026-08-27  7:04   ` bot+bpf-ci
2026-08-28 17:39     ` Yonghong Song
2026-08-27  6:11 ` [PATCH bpf-next v3 03/11] bpf: Add btf_type_is_arena_ptr() Yonghong Song
2026-08-27  6:11 ` [PATCH bpf-next v3 04/11] bpf: Let the by-value struct walk take the kinds of member it accepts Yonghong Song
2026-08-27  6:11 ` [PATCH bpf-next v3 05/11] bpf: Report which member makes a kfunc return type unsupported Yonghong Song
2026-08-27  7:04   ` bot+bpf-ci
2026-08-28 17:45     ` Yonghong Song [this message]
2026-08-27  6:11 ` [PATCH bpf-next v3 06/11] bpf: Allow a global function to return arena pointers by value Yonghong Song
2026-08-27  6:33   ` sashiko-bot
2026-08-28 18:00     ` Yonghong Song
2026-08-27  6:11 ` [PATCH bpf-next v3 07/11] bpf: Allow arena pointers in a by-value kfunc return Yonghong Song
2026-08-27  6:55   ` sashiko-bot
2026-08-28 18:10     ` Yonghong Song
2026-08-27  6:11 ` [PATCH bpf-next v3 08/11] selftests/bpf: Check the member named for an unsupported kfunc return type Yonghong Song
2026-08-27  7:04   ` bot+bpf-ci
2026-08-28 18:20     ` Yonghong Song
2026-08-27  6:12 ` [PATCH bpf-next v3 09/11] selftests/bpf: Test global functions returning arena pointers by value Yonghong Song
2026-08-27  7:04   ` bot+bpf-ci
2026-08-28 18:26     ` Yonghong Song
2026-08-27  6:12 ` [PATCH bpf-next v3 10/11] selftests/bpf: Test kfuncs " Yonghong Song
2026-08-27  7:17   ` bot+bpf-ci
2026-08-28 18:28     ` Yonghong Song
2026-08-27  6:12 ` [PATCH bpf-next v3 11/11] docs/bpf: Document arena pointers in a by-value return Yonghong Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6577c990-f491-43fd-bcef-314160ea5c77@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bot+bpf-ci@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=clm@meta.com \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox