[PATCH] resolve_btfids: preserve parameter names when processing implicit args

[PATCH] resolve_btfids: preserve parameter names when processing implicit args

[Aelin Reidel]

process_kfunc_with_implicit_args() obtains parameter names through
btf__name_by_offset() and passes them to btf__add_func_param() while
constructing a new function prototype.

The returned name pointer references memory owned by the BTF object.
btf__add_func_param() modifies the same BTF and may grow its internal
storage, invalidating previously returned string pointers.

This can result in btf__add_func_param() dereferencing a stale pointer
when copying the parameter name, leading to crashes in strset__add_str().

Duplicate the parameter name before calling btf__add_func_param() so it
remains valid across BTF updates.

Fixes: 9d199965990c ("resolve_btfids: Support for KF_IMPLICIT_ARGS")
Cc: stable@vger.kernel.org
Signed-off-by: Aelin Reidel <aelin@mainlining.org>
---
We were noticing resolve_btfids crashing almost all the time when
building our kernels with BTF debuginfo in postmarketOS. I'm not sure
why specificially our environment triggered this extremely reliably, but
I'm glad I was able to track down the issue. With the patch, I haven't
seen any further issues and our kernel builds are succeeding again.
---
 tools/bpf/resolve_btfids/main.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index f8a91fa7584f..970b810bb779 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -1113,6 +1113,7 @@ static int process_kfunc_with_implicit_args(struct btf2btf_context *ctx, struct
 {
 	s32 idx, new_proto_id, new_func_id, proto_id;
 	const char *param_name, *tag_name;
+	char *tmp_param_name;
 	const struct btf_param *params;
 	enum btf_func_linkage linkage;
 	char tmp_name[KSYM_NAME_LEN];
@@ -1193,7 +1194,11 @@ static int process_kfunc_with_implicit_args(struct btf2btf_context *ctx, struct
 		if (is_kf_implicit_arg(btf, &params[i]))
 			break;
 		param_name = btf__name_by_offset(btf, params[i].name_off);
-		err = btf__add_func_param(btf, param_name, params[i].type);
+		tmp_param_name = strdup(param_name);
+		if (!tmp_param_name)
+			return -ENOMEM;
+		err = btf__add_func_param(btf, tmp_param_name, params[i].type);
+		free(tmp_param_name);
 		if (err < 0) {
 			pr_err("ERROR: resolve_btfids: failed to add param %s for %s\n",
 			       param_name, kfunc->name);

---
base-commit: 598c7067dd8b65b93f3ccada47e9014a13137f1b
change-id: 20260619-resolve-btfids-implicit-args-use-after-free-16fc2939529e

Best regards,
--  
Aelin Reidel <aelin@mainlining.org>

Re: [PATCH] resolve_btfids: preserve parameter names when processing implicit args

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix: Unpatched stale pointer vulnerability for tag_name in btf__add_decl_attr and btf__add_decl_tag.
- [Medium] The patch leaves a use-after-free vulnerability in the error logging path for param_name.
--

commit 77f893ea076418154cdd4dbd0ddf4fed9a32b30b
Author: Aelin Reidel <aelin@mainlining.org>

resolve_btfids: preserve parameter names when processing implicit args

process_kfunc_with_implicit_args() obtains parameter names through
btf__name_by_offset(), which returns pointers to memory owned by the BTF
object. Because btf__add_func_param() modifies the same BTF and may grow its
internal storage, these pointers can become invalid. This commit duplicates
the parameter name before calling btf__add_func_param() to prevent a stale
pointer dereference.

> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index f8a91fa7584f4..970b810bb779b 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
> @@ -1113,6 +1113,7 @@ static int process_kfunc_with_implicit_args(struct btf2btf_context *ctx, struct
>  {
>  	s32 idx, new_proto_id, new_func_id, proto_id;
>  	const char *param_name, *tag_name;

[Severity: High]
This is a pre-existing issue, but doesn't tag_name have the exact same stale
pointer vulnerability that this patch fixes for param_name?

Looking earlier in process_kfunc_with_implicit_args(), tag_name is retrieved
and passed directly to BTF-modifying functions:

    tag_name = btf__name_by_offset(btf, t->name_off);
    ...
    if (btf_kflag(t))
        err = btf__add_decl_attr(btf, tag_name, new_func_id, idx);
    else
        err = btf__add_decl_tag(btf, tag_name, new_func_id, idx);

Since tag_name is passed without duplication, if those functions trigger a
reallocation of the BTF internal string table, could string processing inside
them dereference freed memory and crash? Does it need a similar duplication
fix to ensure the pointer remains valid?

> +	char *tmp_param_name;
>  	const struct btf_param *params;
>  	enum btf_func_linkage linkage;
>  	char tmp_name[KSYM_NAME_LEN];

[ ... ]

> @@ -1193,7 +1194,11 @@ static int process_kfunc_with_implicit_args(struct btf2btf_context *ctx, struct
>  		if (is_kf_implicit_arg(btf, &params[i]))
>  			break;
>  		param_name = btf__name_by_offset(btf, params[i].name_off);
> -		err = btf__add_func_param(btf, param_name, params[i].type);
> +		tmp_param_name = strdup(param_name);
> +		if (!tmp_param_name)
> +			return -ENOMEM;
> +		err = btf__add_func_param(btf, tmp_param_name, params[i].type);
> +		free(tmp_param_name);
>  		if (err < 0) {
>  			pr_err("ERROR: resolve_btfids: failed to add param %s for %s\n",
>  			       param_name, kfunc->name);

[Severity: Medium]
This isn't a bug introduced by this patch, but does the error logging path
still contain a use-after-free vulnerability for param_name?

If btf__add_func_param() fails after having already triggered a reallocation
of the BTF internal string table, the original param_name pointer could
become a dangling pointer. Since tmp_param_name is freed right before the
error check, the pr_err() call uses the potentially stale param_name pointer.

Would it be safer to defer the free(tmp_param_name) call until after the
error check so the duplicated string can be safely used in the log message?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-resolve-btfids-implicit-args-use-after-free-v1-1-2af87d4704c8@mainlining.org?part=1