Re: [PATCH bpf-next v2 1/2] resolve_btfids: Emit arena attributes from kfunc parameter suffixes

[email protected]
Newsgroups org.kernel.vger.bpf
Message-ID <b79dc1dd49e77f0ac33db634050651db29bc9349415954e5c8e4645bc668a0b1@mail.kernel.org>
>     resolve_btfids: Emit arena attributes from kfunc parameter suffixes
>
>     Kfunc declarations can identify arena arguments through parameter name
>     suffixes without repeating KF_ARENA_ARG flags in their BTF ID sets.
>     resolve_btfids currently misses those arguments when synthesizing the
>     address_space(1) attributes used by generated vmlinux.h files.
>
>     Teach the arena prototype rewrite to recognize __arena and
>     __arena__nullable directly on each parameter. Keep KF_ARENA_ARG1 and
>     KF_ARENA_ARG2 handling for explicitly flagged kfuncs, while allowing
>     suffixes on any argument without synthesizing kfunc flags.
>
>     Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>

This looks like a bug fix for missing arena suffix handling. Should this
include:

  Fixes: 27a78c2e7eee ("resolve_btfids: Process KF_ARENA_* flags in resolve_btfids")

> diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
> index 1004eb0bec617..10e725cbe64cd 100644
> --- a/Documentation/bpf/kfuncs.rst
> +++ b/Documentation/bpf/kfuncs.rst

[ ... ]

> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index d2e4176339da6..37d7e7224207d 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c

[ ... ]

> @@ -1067,6 +1069,22 @@ static int collect_decl_tags(struct btf2btf_context *ctx)
>  	return 0;
>  }
>
> +static bool param_name_has_suffix(const char *name, const char *suffix)
> +{
> +	size_t name_len = strlen(name);
> +	size_t suffix_len = strlen(suffix);
> +
> +	return name_len >= suffix_len && !strcmp(name + name_len - suffix_len, suffix);
> +}

Does this accept parameter names that are exactly "__arena" or
"__arena__nullable"?  The kernel's btf_param_match_suffix() in
kernel/bpf/btf.c deliberately rejects that case:

    len = strlen(param_name);
    if (len <= suffix_len)
            return false;

Both verifier and JIT go through that helper. If resolve_btfids uses >= but
the kernel uses >, a parameter whose whole name is the suffix would get
address_space(1) in the generated vmlinux.h but would be treated as an
ordinary pointer by the verifier and JIT. The header and kernel would then
disagree about the calling convention.

Should the comparison here be name_len > suffix_len to match the kernel?

> +
> +static bool is_arena_param(const struct btf *btf, const struct btf_param *param)
> +{
> +	const char *name = btf__name_by_offset(btf, param->name_off);
> +
> +	return param_name_has_suffix(name, PARAM_SUFFIX_ARENA) ||
> +	       param_name_has_suffix(name, PARAM_SUFFIX_ARENA_NULLABLE);
> +}

[ ... ]

> @@ -1299,8 +1317,12 @@ static int process_kfunc_with_implicit_args(struct btf2btf_context *ctx, struct
>  	return 0;
>  }
>
> -static bool is_arena_arg(struct kfunc *kfunc, u32 idx)
> +static bool is_arena_arg(const struct btf *btf, const struct kfunc *kfunc,
> +			 const struct btf_param *param, u32 idx)
>  {
> +	if (is_arena_param(btf, param))
> +		return true;
> +
>  	switch (idx) {
>  	case 0:
>  		return kfunc->flags & KF_ARENA_ARG1;

[ ... ]


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