bpf: Allow arena pointers in by-value returns

[kernel-patches-daemon-bpf]

Pull request for series with
subject: bpf: Allow arena pointers in by-value returns
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1151739

[kernel-patches-daemon-bpf]

Upstream branch: 1555de3
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1151739
version: 2

[kernel-patches-review-bot]

commit c65aceb68d24e42948991cf5f437dfdd64f43531
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Record each half of a paired return value in verifier diagnostics

This patch fixes a diagnostic gap in prepare_func_exit() where the
modification scope around paired register returns only named R0, causing
writes into R2 to go unrecorded.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e036ae20bf6b9..9aa29c3670087 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -10403,10 +10403,14 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
>  		 * return to the caller whatever the callee had in the
>  		 * return register(s)
>  		 */
> -		bpf_diag_mod_begin(env, &caller->regs[BPF_REG_0], r0, BPF_DIAG_MOD_WRITE);
> -		for (i = 0; i < nregs; i++)
> -			caller->regs[ret_regs[i]] = callee->regs[ret_regs[i]];
> -		bpf_diag_mod_end(env);
> +		for (i = 0; i < nregs; i++) {
> +			u32 regno = ret_regs[i];
> +
> +			bpf_diag_mod_begin(env, &caller->regs[regno], &callee->regs[regno],
> +					   BPF_DIAG_MOD_WRITE);
> +			caller->regs[regno] = callee->regs[regno];
> +			bpf_diag_mod_end(env);
> +		}
>  	}
>
>  	/* for callbacks like bpf_loop or bpf_for_each_map_elem go back to callsite,

This fix looks correct for prepare_func_exit(). Should check_func_call()
and check_kfunc_call() get the same per-return-register diagnostic scope,
or is R2 intentionally left out there?

Commit 0630ad00d96d introduced the same pattern in three places, and this
patch addresses only prepare_func_exit().

In check_func_call(), the global-subprog path has:

    bpf_diag_mod_begin(env, &caller->regs[BPF_REG_0], NULL, BPF_DIAG_MOD_WRITE);
    ...
    nregs = bpf_ret_reg_pair(env, subprog) ? 2 : 1;
    for (i = 0; i < nregs; i++)
            mark_reg_unknown(env, caller->regs, ret_regs[i]);
    bpf_diag_mod_end(env);

When nregs is 2, mark_reg_unknown() writes into caller->regs[BPF_REG_2],
but the scope target is caller->regs[BPF_REG_0], so the write emits no
BPF_DIAG_HISTORY_MOD event.

Similarly in check_kfunc_call():

    bpf_diag_mod_begin(env, &regs[BPF_REG_0], NULL, BPF_DIAG_MOD_WRITE);
    ...
    mark_kfunc_ret_regs(env, regs, t->size);   /* writes R0 and, for 8 < size <= 16, R2 */
    ...
    bpf_diag_mod_end(env);

When t->size is between 8 and 16 bytes, mark_kfunc_ret_regs() writes the
second half into R2, but again with no MOD event.

In both cases R2 does get a BPF_DIAG_MOD_CALLER_SAVED scrub record, so
the history shows R2 as clobbered by the call but never shows it being
written with the second half of the return value.

---

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

In-Reply-To-Subject: bpf: Record each half of a paired return value in verifier diagnostics
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32899532405

[kernel-patches-review-bot]

commit 81d969fb0ab5987c508d62e5f01617925bb7a300
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Add btf_type_is_arena_ptr()

This commit simplifies btf_scan_type_tags() by adding a new helper
btf_type_is_arena_ptr() and refactoring the type tag scanning logic.

> bpf: Add btf_type_is_arena_ptr()
>
> Simplify btf_scan_type_tags() and added a new helper
> btf_type_is_arena_ptr(). No functional change.

This isn't a bug, but could the changelog use the imperative mood
consistently (e.g., "Simplify ... and add ...")?  Also, could it explain
why the helper needs external linkage, such as noting that a later patch
calls it from verifier.c?

> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 89d5a5c4f1171..ddd0f4f32d24b 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -235,6 +235,7 @@ struct btf_record *btf_parse_fields(const struct btf *btf, const struct btf_type
>  				    u32 field_mask, u32 value_size);
>  int btf_check_and_fixup_fields(const struct btf *btf, struct btf_record *rec);
>  bool btf_type_is_void(const struct btf_type *t);
> +bool btf_type_is_arena_ptr(const struct btf *btf, const struct btf_type *t);
>  s32 btf_find_by_name_kind(const struct btf *btf, const char *name, u8 kind);
>  s32 bpf_find_btf_id(const char *name, u32 kind, struct btf **btf_p);
>  struct btf *btf_get_module_btf(const struct module *module);
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 47d43eb983a50..280530d25886c 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -3523,6 +3523,22 @@ static int btf_type_tag_walk(const struct btf *btf,
>  	return 0;
>  }
>
> +bool btf_type_is_arena_ptr(const struct btf *btf, const struct btf_type *t)
> +{
> +	if (!btf_type_is_ptr(t))
> +		return false;
> +
> +	for (t = btf_type_by_id(btf, t->type); btf_type_is_modifier(t);
> +	     t = btf_type_by_id(btf, t->type)) {
> +		if (!btf_type_is_type_tag(t) || btf_type_kflag(t))
> +			continue;
> +		if (!strcmp(__btf_name_by_offset(btf, t->name_off), "arena"))
> +			return true;
> +	}
> +
> +	return false;
> +}

This isn't a bug, but since btf_type_tag_walk() sits right above this and
is still used by btf_find_kptr() and friends, could btf_type_is_arena_ptr()
be expressed in terms of it rather than repeating the modifier/type-tag
walk?

[ ... ]

---

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

In-Reply-To-Subject: bpf: Add btf_type_is_arena_ptr()
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32899532405

[kernel-patches-review-bot]

commit d9e6edc14914189d6be606ef785d8a7f2123f65e
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Let the by-value struct walk take the kinds of member it accepts

The by-value struct walk in the verifier is parameterized with a mask of
member kinds it will accept, and btf.c switches over to the new
btf_struct_is_composed_of() entry point while passing BTF_MEMBER_SCALAR.
The old btf_type_is_scalar_struct() becomes a file-local wrapper.

> bpf: Let the by-value struct walk take the kinds of member it accepts
>
>     Use btf_struct_is_composed_of() instead of btf_type_is_scalar_struct() in
>     btf.c so in the future, non scalar member (e.g. arena pointer) can be
>     supported as well. There is no functional change.
>
>     Signed-off-by: Yonghong Song <yonghong.song@linux.dev>

This isn't a bug, but the changelog only describes the one-line call swap
in btf.c, while two other parts of the diff are what the "no functional
change" claim rests on.

The first is a new enumeration in a global header:

> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> +/* Kinds of member a by-value struct or union may be composed of. */
> +enum btf_member_kind {
> +	BTF_MEMBER_SCALAR	= BIT(0), /* an int or an enum, or an array of them */
> +	BTF_MEMBER_ARENA_PTR	= BIT(1), /* a pointer carrying the "arena" type tag */
> +};

The second is the array-member branch of the walk losing its trailing
continue and falling through into the shared kind check:

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>  			member_type = btf_type_skip_modifiers(btf, array->type, NULL);
> -			if (!btf_type_is_scalar(member_type))
> -				return false;
> -			continue;
>  		}
> -		if (!btf_type_is_scalar(member_type))
> +		if (!btf_member_kind_allowed(btf, member_type, member_kinds))
>  			return false;

Equivalence here holds because btf_member_kind_allowed() collapses to
btf_type_is_scalar() when the mask is BTF_MEMBER_SCALAR alone, and because
the shared check is the last statement of the for_each_member() body, so no
member is skipped or checked twice.

Would it help to mention the new enum btf_member_kind in bpf_verifier.h and
this array-branch fall-through in the changelog, since those are what the
"no functional change" claim rests on?

---

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

In-Reply-To-Subject: bpf: Let the by-value struct walk take the kinds of member it accepts
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32899532405

[kernel-patches-review-bot]

commit d0ca6b14d0a20783909dea1ae39b6bc7cfb7e0fc
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Report which member makes a kfunc return type unsupported

This commit improves verifier error messages for kfunc return type
validation by recording and reporting which member makes a struct return
type unsupported, including path information for nested structs.

> A kfunc that returns a struct by value may only return scalars, and the
> message that rejects one names the type but not the member at fault:
>
>   kernel function bpf_kfunc_call_test_ret_ptr returns STRUCT
>   prog_test_ret_ptr that is not composed of scalars
>
> For a large struct that leaves the reader to find the offending member
> by inspection. Record the member that made the walk fail and name it, so
> the verifier also dumps:
>
>   member 'p' has type PTR
>
> What is recorded is a path rather than a single member, because the walk
> descends up to 4 levels. For
>
>   struct outer { struct inner { void *p; } in; __u64 tag; };
>
> naming 'p' alone would send the reader looking for a member struct outer
> does not have, so the message reads "member 'in.p' has type PTR".
>
> The detailed diagnostics for this failure:
>
>   Verification failed: Program Structure: Unsupported kernel function
>   return type
>
>   Reason:
>     bpf_kfunc_call_test_ret_ptr() returns STRUCT prog_test_ret_ptr by
>     value. Its member 'p' is PTR, not a scalar. Only scalar values, or
>     structs composed of scalar values, are supported as by-value kernel
>     function return types.

Does the Reason text shown here match what the verifier actually emits?

The code at verifier.c:14120-14128 uses this format string:

  "%s() returns %s %s by value.%s "
  "Only kfuncs returning scalar values, or "
  "structures composed of scalar values are "
  "supported."

which would produce "... Only kfuncs returning scalar values, or
structures composed of scalar values are supported." but the quoted
output says "... Only scalar values, or structs composed of scalar
values, are supported as by-value kernel function return types."

The first two sentences of the Reason and the Suggestion match what the
code emits, but the third sentence appears different. Looking at the
forward range through 6ab11e51f544, commit 37ffe60f491e5 changes it again
to "... or arena pointers, or structures composed of scalar values and
arena pointers are supported." but still not the wording above.

Could the changelog be updated with the actual verifier output?

>   ...
>   Suggestion:
>     Call a kernel function that returns only scalars by value.
>
> A type nested deeper than the walk descends has no single member to
> blame, so that case reports the depth instead:
>
>   Reason:
>     bpf_kfunc_call_test_ret_deep() returns STRUCT prog_test_ret_deep by
>     value. It nests structs more than 4 levels deep. ...

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 5ea95e75e726a..edbc48a1fdc8f 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -11635,10 +11644,11 @@ static bool btf_member_kind_allowed(const struct btf *btf, const struct btf_type
>
>  /*
>   * Returns true if every member of struct @t is of a kind listed in
> - * @member_kinds, 4 levels of nesting allowed.
> + * @member_kinds, BTF_MEMBER_MAX_DEPTH levels of nesting allowed.
>   */
>  static bool btf_struct_member_walk(struct bpf_verifier_env *env, const struct btf *btf,
> -				   const struct btf_type *t, u32 member_kinds, int rec)
> +				   const struct btf_type *t, u32 member_kinds, int rec,
> +				   struct btf_member_path *path)

All external callers (btf_struct_is_composed_of() and check_kfunc_call())
pass rec=0. Could the rec parameter be removed from the public API and
initialized internally?

This was raised by Eduard Zingerman in v1 and acknowledged with "Ack, will
do" but the parameter remains in v2.

>  {
>  	const struct btf_type *member_type;
>  	const struct btf_member *member;
> @@ -11652,31 +11662,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;
>  }

[ ... ]

> @@ -11686,6 +11707,18 @@ static bool btf_type_is_scalar_struct(struct bpf_verifier_env *env,
>  	return btf_struct_is_composed_of(env, btf, t, BTF_MEMBER_SCALAR);
>  }
>
> +static void btf_member_path_str(const struct btf *btf, const struct btf_member_path *path,
> +				char *buf, size_t buf_sz)
> +{
> +	size_t len = 0;
> +	int i;
> +
> +	buf[0] = '\0';
> +	for (i = 0; i < path->depth; i++)
> +		len += scnprintf(buf + len, buf_sz - len, "%s%s", i ? "." : "",
> +				 btf_name_by_offset(btf, path->member[i]->name_off));
> +}

Can this handle anonymous struct/union members correctly?

BTF allows struct members to have name_off == 0 for anonymous members.
When btf_name_by_offset() is called with name_off == 0, it returns the
empty string (the start of the string table). The walk descends into
anonymous struct/union members since btf_type_is_struct() at
verifier.c:11664 returns true for both BTF_KIND_STRUCT and BTF_KIND_UNION.

For a kfunc like:

  struct ret_anon {
          union { void *p; __u64 v; };  /* anonymous union */
          __u64 tag;
  };
  __bpf_kfunc struct ret_anon bpf_kfunc_ret_anon(void);

the walk would fail on 'p' at rec == 1, so path->depth == 2 with
path->member[0] being the unnamed union and path->member[1] being 'p'.
This would produce the path string ".p" since the first component is empty
but the separator "." is still emitted.

Would it make sense to skip empty components or substitute something like
"<anon>" to keep the path readable?

> +
>  enum kfunc_ptr_arg_type {
>  	KF_ARG_CONST_MEM_SIZE,
>  	KF_ARG_MEM_SIZE,

[ ... ]

> @@ -14053,17 +14086,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)) {
> -			verbose(env,
> -				"kernel function %s returns %s %s that is not composed of scalars\n",
> +		if (!btf_struct_member_walk(env, desc_btf, t, BTF_MEMBER_SCALAR, 0, &path)) {
> +			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;
> +
> +				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));
> +				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));
> +				member_note = bpf_diag_fmt(
> +					env, " Its member '%s' is %s, not a scalar.", bad_name,
> +					btf_type_str(bad_type));
> +			}
> +			bpf_diag_program_structure(
> +				env, insn_idx, "unsupported kernel function return type",
> +				"Call a kernel function that returns only scalars by value.",
> +				"%s() returns %s %s by value.%s "
> +				"Only kfuncs returning scalar values, or "
> +				"structures composed of scalar values are "
> +				"supported.",
>  				func_name, btf_type_str(t),
> -				btf_name_by_offset(desc_btf, t->name_off));
> +				btf_name_by_offset(desc_btf, t->name_off), member_note);
>  			return -EINVAL;
>  		}
>  		mark_kfunc_ret_regs(env, regs, t->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

In-Reply-To-Subject: bpf: Report which member makes a kfunc return type unsupported
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32899532405

[kernel-patches-review-bot]

commit e4ec6bfee111b57f471b689c24d3a2bd7b9f9de5
Author: Yonghong Song <yonghong.song@linux.dev>

selftests/bpf: Test global functions returning arena pointers by value

The commit adds tests for global functions returning structs containing
arena pointers by value, covering R0:R2 pair returns, arena+scalar, arrays,
and single 8-byte structs.

> diff --git a/tools/testing/selftests/bpf/progs/aggregate_ret_func.c b/tools/testing/selftests/bpf/progs/aggregate_ret_func.c
> index 6f66fc822cede..237adb8e5ee1e 100644
> --- a/tools/testing/selftests/bpf/progs/aggregate_ret_func.c
> +++ b/tools/testing/selftests/bpf/progs/aggregate_ret_func.c
> @@ -2,6 +2,7 @@
>  /* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
>  #include <linux/bpf.h>
>  #include <bpf/bpf_helpers.h>
> +#include <bpf_arena_common.h>
>  #include "bpf_misc.h"

[ ... ]

> @@ -234,4 +235,121 @@ __naked int aggregate_ret_global_union_ptr_fail(void)
>
>  #endif
>
> +/*
> + * gcc returns a by-value struct through a hidden pointer, and emits the
> + * 'r0 = r1' returning it after the __naked body's exit, leaving the
> + * subprogram falling through. Build these with clang only.
> + */
> +#if defined(__clang__)

Should this use the same clang version check as the structurally identical
block at line 178? That block checks '#if defined(__clang_major__) &&
__clang_major__ >= 23' for __naked functions returning by-value structs:

aggregate_ret_func.c:178
#if defined(__clang_major__) && __clang_major__ >= 23
...
__naked struct with_ptr global_ret_struct_ptr(void)
{
	asm volatile (
	"r0 = 0;"
	"r2 = 0;"
	"exit;"
	);
}

SEC("tc")
__load_if_JITed()
__failure __msg("has unsupported return type")
__naked int aggregate_ret_global_struct_ptr_fail(void)
{
	asm volatile (
	"call %[global_ret_struct_ptr];"
	"r0 = 0;"
	"exit;"
	:
	: __imm(global_ret_struct_ptr)
	: __clobber_all);
}

The verifier_arena.c hunk added in this same commit also uses the '>= 23'
guard:

verifier_arena.c:737
#if defined(__clang_major__) && __clang_major__ >= 23

struct arena_page_pair {
	u32 __arena *first;
	u32 __arena *second;
};

__weak struct arena_page_pair split_arena_page(u32 __arena *page)
{
	...
}

By-value struct returns in registers are an LLVM 23 BPF-ABI feature. With
an older clang, the return is lowered through a hidden pointer (sret), which
is the gcc behavior described in your new comment.

With clang < 23, the four new '__success __retval(0)' tests would be
compiled against a different signature than intended. The callers never
initialize r1, so they would be checked against a prototype that no longer
matches 'returns struct arena_pair' in the BTF.

Can the two adjacent blocks be consistent about which toolchains can build
the same construct?

> +
> +struct arena_pair {
> +	void __arena *lo;
> +	void __arena *hi;
> +};
> +
> +struct arena_and_scalar {
> +	void __arena *p;
> +	__u64 x;
> +};
> +
> +struct arena_array {
> +	void __arena *p[2];
> +};
> +
> +struct arena_single {
> +	void __arena *p;
> +};
> +
> +__naked struct arena_pair global_ret_arena_pair(void)
> +{
> +	asm volatile (
> +	"r0 = 0;"
> +	"r2 = 0;"
> +	"exit;"
> +	);
> +}
> +
> +SEC("tc")
> +__load_if_JITed()
> +__success __retval(0)
> +__naked int aggregate_ret_global_arena_pair(void)
> +{
> +	asm volatile (
> +	"call %[global_ret_arena_pair];"
> +	"r0 = 0;"
> +	"exit;"
> +	:
> +	: __imm(global_ret_arena_pair)
> +	: __clobber_all);
> +}
> +
> +__naked struct arena_and_scalar global_ret_arena_and_scalar(void)
> +{
> +	asm volatile (
> +	"r0 = 0;"
> +	"r2 = 0;"
> +	"exit;"
> +	);
> +}
> +
> +SEC("tc")
> +__load_if_JITed()
> +__success __retval(0)
> +__naked int aggregate_ret_global_arena_and_scalar(void)
> +{
> +	asm volatile (
> +	"call %[global_ret_arena_and_scalar];"
> +	"r0 = 0;"
> +	"exit;"
> +	:
> +	: __imm(global_ret_arena_and_scalar)
> +	: __clobber_all);
> +}
> +
> +__naked struct arena_array global_ret_arena_array(void)
> +{
> +	asm volatile (
> +	"r0 = 0;"
> +	"r2 = 0;"
> +	"exit;"
> +	);
> +}
> +
> +SEC("tc")
> +__load_if_JITed()
> +__success __retval(0)
> +__naked int aggregate_ret_global_arena_array(void)
> +{
> +	asm volatile (
> +	"call %[global_ret_arena_array];"
> +	"r0 = 0;"
> +	"exit;"
> +	:
> +	: __imm(global_ret_arena_array)
> +	: __clobber_all);
> +}
> +
> +__naked struct arena_single global_ret_arena_single(void)
> +{
> +	asm volatile (
> +	"r0 = 0;"
> +	"exit;"
> +	);
> +}
> +
> +SEC("tc")
> +__success __retval(0)
> +__naked int aggregate_ret_global_arena_single(void)
> +{
> +	asm volatile (
> +	"call %[global_ret_arena_single];"
> +	"r0 = 0;"
> +	"exit;"
> +	:
> +	: __imm(global_ret_arena_single)
> +	: __clobber_all);
> +}
> +
> +#endif
> +
>  char _license[] SEC("license") = "GPL";

[ ... ]

---

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

In-Reply-To-Subject: selftests/bpf: Test global functions returning arena pointers by value
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32899532405

[kernel-patches-review-bot]

commit 6ab11e51f5443c81b8afaed2b7d28274fa828e67
Author: Yonghong Song <yonghong.song@linux.dev>

selftests/bpf: Test kfuncs returning arena pointers by value

Cover the by-value struct returns a kfunc may now make: two arena
pointers filling R0:R2, and an arena pointer beside a scalar. The
existing cases for a struct and a nested struct carrying a plain pointer
stay rejected.

> diff --git a/tools/testing/selftests/bpf/prog_tests/aggregate_ret.c b/tools/testing/selftests/bpf/prog_tests/aggregate_ret.c
> index e0b94ed10f941..07d9d6e1d6b88 100644
> --- a/tools/testing/selftests/bpf/prog_tests/aggregate_ret.c
> +++ b/tools/testing/selftests/bpf/prog_tests/aggregate_ret.c
> @@ -1,11 +1,53 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
>  #include <test_progs.h>
> +#include <bpf/btf.h>
>  #include "aggregate_ret_func.skel.h"
>  #include "aggregate_ret_kfunc.skel.h"
> +#include "aggregate_ret_kfunc_arena.skel.h"
> +
> +static bool testmod_has_arena_tagged_member(void)
> +{
> +	struct btf *vmlinux_btf, *module_btf = NULL;
> +	const struct btf_type *t;
> +	bool tagged = false;
> +	__s32 id;
> +
> +	vmlinux_btf = btf__load_vmlinux_btf();
> +	if (!vmlinux_btf)
> +		return false;
> +
> +	module_btf = btf__load_module_btf("bpf_testmod", vmlinux_btf);
> +	if (!module_btf)
> +		goto out;
> +
> +	/* prog_test_ret_arena::a is 'void __arena_tag *': PTR -> TYPE_TAG -> void */
> +	id = btf__find_by_name_kind(module_btf, "prog_test_ret_arena", BTF_KIND_STRUCT);
> +	if (id <= 0)
> +		goto out;
> +
> +	t = btf__type_by_id(module_btf, btf_members(btf__type_by_id(module_btf, id))[0].type);
> +	if (!t || !btf_is_ptr(t))
> +		goto out;
> +
> +	t = btf__type_by_id(module_btf, t->type);
> +	tagged = t && btf_is_type_tag(t) &&
> +		 !strcmp(btf__name_by_offset(module_btf, t->name_off), "arena");
> +
> +out:
> +	btf__free(module_btf);
> +	btf__free(vmlinux_btf);
> +
> +	return tagged;
> +}
>
>  void test_aggregate_ret(void)
>  {
>  	RUN_TESTS(aggregate_ret_func);
>  	RUN_TESTS(aggregate_ret_kfunc);
> +
> +	if (testmod_has_arena_tagged_member())
> +		RUN_TESTS(aggregate_ret_kfunc_arena);
> +	else
> +		test__skip();

A subsystem pattern flags this as potentially concerning:
testmod_has_arena_tagged_member() collapses four distinct outcomes
into the same 'false' return: environment broken (no vmlinux BTF),
testmod absent or BTF missing, stale bpf_testmod.ko, or the actual
capability probe (btf_type_tag attribute absent). When test__skip()
is called without a reason string, the run shows 'aggregate_ret:OK
(SKIP: 1/N)' with no indication of what was skipped or why.

Since PAHOLE_HAS_BTF_TAG depends on CC_IS_CLANG (lib/Kconfig.debug)
and GCC does not implement btf_type_tag, the skip branch is the normal
outcome on a GCC-built kernel, which makes a permanently-skipped test
easy to miss. The test appears to pass but the new coverage never runs.

The BPF selftests/bpf/prog_tests/btf_tag.c establishes a precedent:
it checks env.has_testmod, uses ASSERT_OK_PTR() so a BTF-load failure
is a real FAIL, and prints a reason string before test__skip():

    printf("%s:SKIP: btf_type_tag attribute not in %s", __func__, module_name)

Should testmod_has_arena_tagged_member() distinguish a broken environment
(ASSERT_OK_PTR on the vmlinux BTF) from a missing capability, and should
the test__skip() carry a reason string?

>  }

> diff --git a/tools/testing/selftests/bpf/progs/aggregate_ret_kfunc_arena.c b/tools/testing/selftests/bpf/progs/aggregate_ret_kfunc_arena.c
> new file mode 100644
> index 0000000000000..f68deae6c900e
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/aggregate_ret_kfunc_arena.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +#include "../test_kmods/bpf_testmod_kfunc.h"
> +
> +void __kfunc_btf_root(void)
> +{
> +	asm volatile (""
> +	:
> +	: "r"(&bpf_kfunc_call_test_ret_arena),
> +	  "r"(&bpf_kfunc_call_test_ret_arena_mixed));
> +}

A subsystem pattern flags this as potentially concerning: a new BPF
prog file is added for two test cases, but progs/aggregate_ret_kfunc.c
in the same directory already covers by-value kfunc returns
(aggregate_ret_kfunc_precise, _fastcall_fail, _ptr_fail,
_nested_ptr_fail, _too_deep_fail, _small_no_r2, _too_big_fail). The
new file duplicates the header includes, the license string, the
SEC("tc")/__arch_x86_64/__arch_arm64/__load_if_JITed() preamble and
__kfunc_btf_root() itself, for two subtests.

RUN_TESTS() operates on a whole skeleton (test_progs.h:546 ->
test_loader__run_subtests(&tester, #skel, skel##__elf_bytes)), and
these two subtests must be gated on whether the running bpf_testmod's
BTF carries the 'arena' type tag - a per-toolchain condition that has
no per-subtest expression in bpf_misc.h (__arch_*/__load_if_JITed
cover architecture and JIT only). Putting them in
aggregate_ret_kfunc.c would force the whole existing matrix behind
the same gate.

Is the separate file justified by the per-skeleton gating requirement,
or could the new cases be added to aggregate_ret_kfunc.c?

> +
> +SEC("tc")
> +__arch_x86_64 __arch_arm64
> +__load_if_JITed()
> +__success __retval(0)
> +__naked int aggregate_ret_kfunc_arena(void)
> +{
> +	asm volatile (
> +	"call %[bpf_kfunc_call_test_ret_arena];"
> +	"r0 = 0;"
> +	"exit;"
> +	:
> +	: __imm(bpf_kfunc_call_test_ret_arena)
> +	: __clobber_all);
> +}
> +
> +SEC("tc")
> +__arch_x86_64 __arch_arm64
> +__load_if_JITed()
> +__success __retval(0)
> +__naked int aggregate_ret_kfunc_arena_mixed(void)
> +{
> +	asm volatile (
> +	"r1 = 0;"
> +	"call %[bpf_kfunc_call_test_ret_arena_mixed];"
> +	"r0 = 0;"
> +	"exit;"
> +	:
> +	: __imm(bpf_kfunc_call_test_ret_arena_mixed)
> +	: __clobber_all);
> +}

The changelog claims this patch covers 'two arena pointers filling
R0:R2', but neither new program ever reads R2. Both bodies are
'call <kfunc>; r0 = 0; exit;' with __success __retval(0), which only
proves that check_kfunc_call() accepted the return type - i.e. that
btf_struct_member_walk(..., BTF_MEMBER_SCALAR | BTF_MEMBER_ARENA_PTR, ...)
returned true (kernel/bpf/verifier.c:14097). The number of return
registers is decided separately by mark_kfunc_ret_regs() -> ret_regs_cnt(size)
(kernel/bpf/verifier.c:11351, :413); if that path regressed to a
single register for a 16-byte STRUCT return, both new tests would
still pass, because R2 is never read.

The file this patch sits beside already establishes the idiom for
pinning this: aggregate_ret_kfunc_small_no_r2 asserts __failure
__msg("R2 !read_ok") to prove R2 is *not* a return register for an
8-byte struct, and aggregate_ret_kfunc_precise reads 'r6 = r2' after
the __int128 kfunc to prove R2 *is*.

Should at least one of the arena tests read R2 back to verify the
two-register return path?

> +
> +char _license[] SEC("license") = "GPL";

[ ... ]

---

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

In-Reply-To-Subject: selftests/bpf: Test kfuncs returning arena pointers by value
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32899532405

[kernel-patches-daemon-bpf]

Forwarding comment 5417260118 via email
In-Reply-To: 20260825205417.1320403-1-yonghong.song@linux.dev
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260825205417.1320403-1-yonghong.song@linux.dev/

[kernel-patches-daemon-bpf]

Forwarding comment 5417271822 via email
In-Reply-To: 20260825205427.1322594-1-yonghong.song@linux.dev
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260825205427.1322594-1-yonghong.song@linux.dev/

[kernel-patches-daemon-bpf]

Forwarding comment 5417275968 via email
In-Reply-To: 20260825205432.1322845-1-yonghong.song@linux.dev
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260825205432.1322845-1-yonghong.song@linux.dev/

[kernel-patches-daemon-bpf]

Forwarding comment 5417312123 via email
In-Reply-To: 20260825205438.1323042-1-yonghong.song@linux.dev
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260825205438.1323042-1-yonghong.song@linux.dev/

[kernel-patches-daemon-bpf]

Forwarding comment 5417327244 via email
In-Reply-To: 20260825205458.1325916-1-yonghong.song@linux.dev
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260825205458.1325916-1-yonghong.song@linux.dev/

[kernel-patches-daemon-bpf]

Forwarding comment 5417376510 via email
In-Reply-To: 20260825205503.1327012-1-yonghong.song@linux.dev
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260825205503.1327012-1-yonghong.song@linux.dev/

[kernel-patches-review-bot]

commit 37ffe60f491e533e899243b227f435e19982a6de
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Allow arena pointers in a by-value kfunc return

A kfunc may already return an arena pointer on its own, which the
program casts back into the arena address space to use. Let the members
of a by-value struct it returns be arena pointers as well, rather than
scalars only.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index edbc48a1fdc8f..0c2181d587489 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -14090,12 +14090,12 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  		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.
> +		 * The returned struct may only contain scalars and arena pointers
> +		 * as its members. Otherwise, any other pointer would be laundered
> +		 * into a scalar and escape provenance and reference tracking.
>  		 */
> -		if (!btf_struct_member_walk(env, desc_btf, t, BTF_MEMBER_SCALAR, 0, &path)) {
> +		if (!btf_struct_member_walk(env, desc_btf, t,
> +					    BTF_MEMBER_SCALAR | BTF_MEMBER_ARENA_PTR, 0, &path)) {

The rewritten comment drops the mechanism that justified the rule and, as
written, gives a rationale that contradicts the new behaviour. The
previous comment was:

  /* 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.
   */

The new text keeps the 'laundered into a scalar' conclusion but deletes
the 'comes back as raw register bits modeled as an unknown scalar' premise,
and then asserts arena pointers are fine. A reader is left with an
inconsistency, because an arena pointer member is laundered into a scalar
in exactly the same way -- mark_kfunc_ret_regs() marks both R0 and R2 as
unknown scalars regardless of member kinds.

The actual reason arena pointers are safe is not stated anywhere in the
change: a laundered arena address is only usable after
addr_space_cast/cast_kern, check_alu_op() already lets a program
cast_kern() any scalar, and cast_kern is lowered to a 32-bit ALU mov so
the result is confined to the arena window at r12. One clause noting that
arena pointers need no provenance because they are re-derived by
cast_kern would restore the invariant explanation.

Also, 'Otherwise, any other pointer' is redundant ('otherwise' plus
'any other').

> @@ -14106,7 +14106,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  				const struct btf_type *bad_type;
>
>  				verbose(env,
> -					"kernel function %s returns %s %s that is not composed of scalars\n",
> +					"kernel function %s returns %s %s that is not composed of scalars or arena pointers\n",
>  					func_name, btf_type_str(t),
>  					btf_name_by_offset(desc_btf, t->name_off));
>  				btf_member_path_str(desc_btf, &path, bad_name, sizeof(bad_name));
> @@ -14114,15 +14114,16 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  				verbose(env, "member '%s' has type %s\n", bad_name,
>  					btf_type_str(bad_type));
>  				member_note = bpf_diag_fmt(
> -					env, " Its member '%s' is %s, not a scalar.", bad_name,
> -					btf_type_str(bad_type));
> +					env,
> +					" Its member '%s' is %s, not a scalar or an arena pointer.",
> +					bad_name, btf_type_str(bad_type));
>  			}
>  			bpf_diag_program_structure(
>  				env, insn_idx, "unsupported kernel function return type",
> -				"Call a kernel function that returns only scalars by value.",
> +				"Call a kernel function that returns only scalars or arena pointers by value.",
>  				"%s() returns %s %s by value.%s "
> -				"Only kfuncs returning scalar values, or "
> -				"structures composed of scalar values are "
> +				"Only kfuncs returning scalar values or arena pointers, or "
> +				"structures composed of scalar values and arena pointers are "
>  				"supported.",
>  				func_name, btf_type_str(t),
>  				btf_name_by_offset(desc_btf, t->name_off), member_note);

In the bpf_diag_program_structure() reason text, 'structures composed of
scalar values and arena pointers' reads as a conjunction, i.e. that a
supported struct must contain both scalars and arena pointers. The
accepted set is a disjunction: BTF_MEMBER_SCALAR | BTF_MEMBER_ARENA_PTR
accepts any mix, including all-scalar and all-arena-pointer structs. The
other three strings in the same diagnostic use 'or' consistently, so this
line is the odd one out. Would 'structures composed of scalar values or
arena pointers' match?

> diff --git a/tools/testing/selftests/bpf/progs/aggregate_ret_kfunc.c b/tools/testing/selftests/bpf/progs/aggregate_ret_kfunc.c
> index d6b422ae9784b..f10e5cf6fd890 100644
> --- a/tools/testing/selftests/bpf/progs/aggregate_ret_kfunc.c
> +++ b/tools/testing/selftests/bpf/progs/aggregate_ret_kfunc.c
> @@ -71,7 +71,7 @@ __naked int aggregate_ret_kfunc_fastcall_fail(void)
>
>  SEC("tc")
>  __arch_x86_64 __arch_arm64
> -__failure __msg("is not composed of scalars")
> +__failure __msg("is not composed of scalars or arena pointers")
>  __naked int aggregate_ret_kfunc_ptr_fail(void)

[ ... ]

This commit relaxes the by-value kfunc struct return rule from
BTF_MEMBER_SCALAR to BTF_MEMBER_SCALAR | BTF_MEMBER_ARENA_PTR and updates
all four user-visible verifier strings accordingly, but
Documentation/bpf/kfuncs.rst section 2.9 still documents the old contract
and is now factually wrong in three places:

  - 'A struct or union returned by value must be composed only of scalars
    (recursively), where a scalar is an integer or an enum'
  - 'A struct or union with a pointer member is therefore rejected at load
    time' -- a struct with a 'void __arena *' (btf_type_tag("arena"))
    member is now accepted
  - 'A kfunc may also return a value larger than 8 bytes and up to 16
    bytes -- a scalar-only struct or union, or an __int128'
  - 'A global subprogram is verified in isolation, so its by-value struct
    or union return is restricted to scalars just like a kfunc's' -- also
    invalidated by patch 4 of the same series (ba9c98e6fb9db, 'bpf: Allow
    a global function to return arena pointers by value')

This paragraph is not boilerplate: it was written specifically to explain
this restriction (it even paraphrases the code comment that this commit
deletes), so it is the authoritative reference for the rule being changed.
Looking at the range 37ffe60f491e..6ab11e51f5443, no commit in the series
updates Documentation/, so the documentation stays contradictory after the
whole series is applied.

Could the paragraph be extended to say members may be scalars or arena
pointers, and explain that an arena pointer member is likewise handed back
as a scalar which the program must cast_kern() before use?

The new acceptance path is reachable only through an annotation mechanism
the kfunc ABI does not currently offer. btf_type_is_arena_ptr() matches
only a kflag=0 BTF_KIND_TYPE_TAG named 'arena' and explicitly skips
kflag'd tags. The established way a kfunc declares arena pointers is
KF_ARENA_RET / KF_ARENA_ARG1/2 plus the '__arena' parameter-name suffix,
which tools/bpf/resolve_btfids emits as a kflag=1 type attribute
'address_space(1)' -- deliberately not matched here. resolve_btfids'
arena_tag_ptr() also only tags a func proto's return pointer and its
parameters, and errors out with 'arena type is not a pointer' for a struct
return, so KF_ARENA_RET cannot express 'this returned struct's members are
arena pointers'.

So a kfunc can only hit this new path if its declaration carries a raw
__attribute__((btf_type_tag("arena"))) on the member, and no kernel-side
macro exists for that. The follow-up selftest commit 6ab11e51f5443 has to
invent its own '#define __arena_tag
__attribute__((btf_type_tag("arena")))' in
tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h, guarded by
__has_attribute(btf_type_tag), plus a runtime BTF probe
(testmod_has_arena_tagged_member()) that test__skip()s when the build
toolchain did not emit the tag. That is evidence the code is live but also
that no supported kernel-side spelling exists.

Would an in-kernel __arena macro (or teaching resolve_btfids to tag struct
members under KF_ARENA_RET) make the feature usable by real kfuncs and
testable without a skip?

---

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

In-Reply-To-Subject: bpf: Allow arena pointers in a by-value kfunc return
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32899532405

[kernel-patches-daemon-bpf]

Forwarding comment 5417461291 via email
In-Reply-To: 20260825205448.1324569-1-yonghong.song@linux.dev
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260825205448.1324569-1-yonghong.song@linux.dev/

[kernel-patches-daemon-bpf]

Upstream branch: 48b69cc
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1151739
version: 2