Re: [PATCH] perf arm: Fix memory leak in find_all_arm_spe_pmus()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Dereference of NULL pointer when `find_all_hisi_ptt_pmus` fails to load a matched PMU.
- [Medium] Critical allocation failure errors are silently overwritten and dropped, preventing the caller from detecting OOM states during tracer initialization.
- [Low] Missing explicit inclusion of several POSIX standard headers for libc functions and constants used in the file, which will cause compilation failures under musl libc.
--

commit 4dc56b9f02f41e7883231eea2dbc58e300bd2970
Author: Liu Jing <liujing@cmss.chinamobile.com>

perf arm: Fix memory leak in find_all_arm_spe_pmus()

In find_all_arm_spe_pmus(), if sprintf() fails, the function returns NULL
without freeing the previously allocated arm_spe_pmus array, causing a
memory leak.

> diff --git a/tools/perf/arch/arm/util/auxtrace.c b/tools/perf/arch/arm/util/auxtrace.c
> index 27bb14c8b8800..dd3e1f8189a5c 100644
> --- a/tools/perf/arch/arm/util/auxtrace.c
> +++ b/tools/perf/arch/arm/util/auxtrace.c
> @@ -39,6 +39,7 @@ static struct perf_pmu **find_all_arm_spe_pmus(int *nr_spes, int *err)
>  		if (ret < 0) {
>  			pr_err("sprintf failed\n");
>  			*err = -ENOMEM;
> +			free(arm_spe_pmus);
>  			return NULL;
>  		}

[Severity: Medium]
This is a pre-existing issue, but I noticed that critical allocation failures
from find_all_arm_spe_pmus() and find_all_hisi_ptt_pmus() are silently
swallowed:

tools/perf/arch/arm/util/auxtrace.c:auxtrace_record__init() {
    ...
	arm_spe_pmus = find_all_arm_spe_pmus(&nr_spes, err);
	hisi_ptt_pmus = find_all_hisi_ptt_pmus(&nr_ptts, err);
    ...
	/*
	 * Clear 'err' even if we haven't found an event - that way perf
	 * record can still be used even if tracers aren't present.  The NULL
	 * return value will take care of telling the infrastructure HW tracing
	 * isn't available.
	 */
	*err = 0;
	return NULL;
}

If memory allocation fails in find_all_arm_spe_pmus(), it correctly sets
*err = -ENOMEM and returns NULL. However, auxtrace_record__init()
unconditionally executes *err = 0 before returning NULL when no tracers are
loaded. Doesn't this overwrite the -ENOMEM error and falsely report to the
caller that HW tracing is simply unavailable instead of warning about resource
exhaustion?


[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential NULL
pointer dereference when find_all_hisi_ptt_pmus() fails to load a matched PMU:

tools/perf/arch/arm/util/auxtrace.c:find_all_hisi_ptt_pmus() {
    ...
	while ((dent = readdir(dir))) {
		if (strstr(dent->d_name, HISI_PTT_PMU_NAME) && idx < *nr_ptts) {
			hisi_ptt_pmus[idx] = perf_pmus__find(dent->d_name);
			if (hisi_ptt_pmus[idx])
				idx++;
		}
	}

out:
	closedir(dir);
	return hisi_ptt_pmus;
}

If perf_pmus__find() returns NULL (e.g. due to sysfs read errors), idx isn't
incremented, but the function returns without updating the out parameter
*nr_ptts to match the actual number of valid pointers (idx).

Later, the caller find_pmu_for_event() loops up to the stale nr_ptts count:

tools/perf/arch/arm/util/auxtrace.c:find_pmu_for_event() {
    ...
	for (i = 0; i < pmu_nr; i++) {
		if (evsel->core.attr.type == pmus[i]->type)
			return pmus[i];
	}
    ...
}

Will this unconditional dereference of pmus[i]->type hit the trailing NULL
entries in the array, causing a segmentation fault?


[Severity: Low]
This is a pre-existing issue, but while reviewing this file I noticed several
missing standard headers that will cause compilation failures under musl libc:

tools/perf/arch/arm/util/auxtrace.c:
    #include <dirent.h>
    #include <errno.h>
    #include <stdbool.h>
    #include <linux/coresight-pmu.h>
    #include <stdlib.h>
    #include <api/fs/fs.h>

Functions like sysconf(), sprintf(), and strstr(), and constants like PATH_MAX
are used without explicitly including <unistd.h>, <stdio.h>, <string.h>, and
<limits.h>. Could these explicit POSIX header inclusions be added?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903082128.4972-4-liujing@cmss.chinamobile.com?part=1