Re: [PATCH v3 21/21] perf annotate-arm64: Support 'mrs' instruction to track 'current' pointer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Memory leak of the dynamically allocated `type_name` string obtained from `strbuf_detach()`.
- [Medium] Cross-DSO contamination and potential type confusion due to globally caching a DWARF offset (`task_struct_dieoff`).
--

commit 0f7d6f3c161448b3cb967b47ee3887be3d974dc1
Author: Tengda Wu <wutengda@huaweicloud.com>

perf annotate-arm64: Support 'mrs' instruction to track 'current' pointer

This commit adds support for parsing the 'mrs' instruction on arm64 in
'perf annotate' to track the 'current' task pointer. It dynamically queries
the DWARF debug information for the 'get_current()' inline function's
return type to find 'struct task_struct', caching the DWARF offset for
subsequent lookups.

> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> index ec6fd59d51a27..89e3997f6721b 100644
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
[ ... ]
> @@ -787,6 +789,81 @@ static void update_adrp_insn_state(struct type_state *state,
>  		     insn_offset, tsr->addr, dreg);
>  }
>  
> +static Dwarf_Off task_struct_dieoff;
> +
> +static void update_mrs_insn_state(struct type_state *state,
> +				  struct data_loc_info *dloc, Dwarf_Die *cu_die,
> +				  struct disasm_line *dl,
> +				  struct annotated_op_loc *dst)
> +{
[ ... ]
> +	/* Handle case difference: LLVM (SP_EL0) vs objdump (sp_el0) */
> +	if (!dso__kernel(map__dso(dloc->ms->map)) ||
> +	    strcasecmp(dl->ops.source.raw, "sp_el0")) {
> +		invalidate_reg_state(tsr);
> +		return;
> +	}
> +
> +	ip = dloc->ms->sym->start + dl->al.offset;
> +	pc = map__rip_2objdump(dloc->ms->map, ip);
> +
> +	if (!task_struct_dieoff ||
> +	    !dwarf_offdie(dloc->di->dbg, task_struct_dieoff, &type_die)) {

[Severity: Medium]
Since DWARF offsets are local to their .debug_info section, if perf annotate
processes a symbol in a different kernel module (where dso__kernel() is still
true), couldn't this global task_struct_dieoff be applied to the module's
Dwarf * handle?

Would this result in type confusion or returning an arbitrary, unrelated DIE
across different DSOs?

> +		struct strbuf sb;
> +		char *type_name;
> +		/*
> +		 * Find the inline function 'get_current()' Dwarf_Die
> +		 * and obtain its return value data type, which should
> +		 * be 'struct task_struct*'.
> +		 */
> +		if (!die_find_inlinefunc(cu_die, pc, &func_die) ||
> +			!dwarf_attr_integrate(&func_die, DW_AT_type, &attr) ||
> +			!dwarf_formref_die(&attr, &type_die)) {
> +			invalidate_reg_state(tsr);
> +			return;
> +		}
> +
> +		strbuf_init(&sb, 32);
> +		die_get_typename_from_type(&type_die, &sb);
> +		type_name = strbuf_detach(&sb, NULL);
> +
> +		if (!type_name || strcmp(type_name, "struct task_struct*")) {
> +			invalidate_reg_state(tsr);
> +			return;
> +		}

[Severity: Medium]
Does this code leak the type_name string?

strbuf_detach() allocates and transfers ownership of the string to type_name,
but it appears to never be freed on either the success path or the error path
returning here.

> +
> +		/*
> +		 * Cache the 'struct task_struct*' die offset globally.
> +		 * This allows us to resolve stack canary accesses even
> +		 * in CUs that lack a full task_struct definition (e.g.,
> +		 * compiler-generated entry/exit code).
> +		 */
> +		task_struct_dieoff = dwarf_dieoffset(&type_die);
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701035355.752944-1-wutengda@huaweicloud.com?part=21