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 v2 07/10] bpf: Allow arena pointers in a by-value kfunc return
Date: Wed, 26 Aug 2026 11:57:15 -0700 [thread overview]
Message-ID: <e7433f26-4d81-49e7-bdf1-ce5c731d85ff@linux.dev> (raw)
In-Reply-To: <78325e93a25b9a6cb4ed57afdafd0db5560e504e0a1e56eceac1c5bba029c0a2@mail.kernel.org>
On 8/25/26 3:13 PM, bot+bpf-ci@kernel.org wrote:
>> 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').
Okay, I will adjust comments then.
>
>> @@ -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?
Sounds good.
>
>> 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?
Okay, will update doc properly.
>
> 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?
Good point, let me check this __arena macro is preferred compare to
__attribute__((btf_type_tag("arena")))
>
>
> ---
> 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/32899532405
next prev parent reply other threads:[~2026-08-26 18:57 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-25 20:54 [PATCH bpf-next v2 00/10] bpf: Allow arena pointers in by-value returns Yonghong Song
2026-08-25 20:54 ` [PATCH bpf-next v2 01/10] bpf: Record each half of a paired return value in verifier diagnostics Yonghong Song
2026-08-25 21:59 ` bot+bpf-ci
2026-08-26 17:08 ` Yonghong Song
2026-08-25 20:54 ` [PATCH bpf-next v2 02/10] bpf: Drop the recursion depth argument of btf_type_is_scalar_struct() Yonghong Song
2026-08-25 20:54 ` [PATCH bpf-next v2 03/10] bpf: Add btf_type_is_arena_ptr() Yonghong Song
2026-08-25 21:59 ` bot+bpf-ci
2026-08-26 17:28 ` Yonghong Song
2026-08-25 20:54 ` [PATCH bpf-next v2 04/10] bpf: Let the by-value struct walk take the kinds of member it accepts Yonghong Song
2026-08-25 21:59 ` bot+bpf-ci
2026-08-26 17:39 ` Yonghong Song
2026-08-25 20:54 ` [PATCH bpf-next v2 05/10] bpf: Report which member makes a kfunc return type unsupported Yonghong Song
2026-08-25 21:59 ` bot+bpf-ci
2026-08-26 17:59 ` Yonghong Song
2026-08-25 20:54 ` [PATCH bpf-next v2 06/10] bpf: Allow a global function to return arena pointers by value Yonghong Song
2026-08-25 21:12 ` sashiko-bot
2026-08-26 18:40 ` Yonghong Song
2026-08-25 20:54 ` [PATCH bpf-next v2 07/10] bpf: Allow arena pointers in a by-value kfunc return Yonghong Song
2026-08-25 22:13 ` bot+bpf-ci
2026-08-26 18:57 ` Yonghong Song [this message]
2026-08-25 20:54 ` [PATCH bpf-next v2 08/10] selftests/bpf: Check the member named for an unsupported kfunc return type Yonghong Song
2026-08-25 20:54 ` [PATCH bpf-next v2 09/10] selftests/bpf: Test global functions returning arena pointers by value Yonghong Song
2026-08-25 21:59 ` bot+bpf-ci
2026-08-27 3:46 ` Yonghong Song
2026-08-25 20:55 ` [PATCH bpf-next v2 10/10] selftests/bpf: Test kfuncs " Yonghong Song
2026-08-25 21:59 ` bot+bpf-ci
2026-08-27 3:58 ` 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=e7433f26-4d81-49e7-bdf1-ce5c731d85ff@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