linux-kernel - Re: [PATCH] perf pmu: Handle memory failure in tool_pmu_

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9f9c007a-78fe-4709-9063-eb4df72d4f63@linaro.org>
Date: Wed, 19 Mar 2025 09:58:14 +0000
From: James Clark <james.clark@...aro.org>
To: Thomas Richter <tmricht@...ux.ibm.com>
Cc: agordeev@...ux.ibm.com, gor@...ux.ibm.com, sumanthk@...ux.ibm.com,
 hca@...ux.ibm.com, linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
 linux-perf-users@...r.kernel.org, acme@...nel.org, namhyung@...nel.org,
 irogers@...gle.com
Subject: Re: [PATCH] perf pmu: Handle memory failure in tool_pmu__new()



On 19/03/2025 9:28 am, Thomas Richter wrote:
> On linux-next
> commit 72c6f57a4193 ("perf pmu: Dynamically allocate tool PMU")
> allocated PMU named "tool" dynamicly. However that allocation
> can fail and a NULL pointer is returned. That case is currently
> not handled and would result in an invalid address reference.
> Add a check for NULL pointer.
> 
> Fixes: 72c6f57a4193 ("perf pmu: Dynamically allocate tool PMU")
> Signed-off-by: Thomas Richter <tmricht@...ux.ibm.com>
> Cc: Ian Rogers <irogers@...gle.com>
> Cc: James Clark <james.clark@...aro.org>
> ---
>   tools/perf/util/pmus.c     | 3 ++-
>   tools/perf/util/tool_pmu.c | 9 +++++++++
>   2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index 9b5a63ecb249..b99292de7669 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -265,7 +265,8 @@ static void pmu_read_sysfs(unsigned int to_read_types)
>   	if ((to_read_types & PERF_TOOL_PMU_TYPE_TOOL_MASK) != 0 &&
>   	    (read_pmu_types & PERF_TOOL_PMU_TYPE_TOOL_MASK) == 0) {
>   		tool_pmu = tool_pmu__new();
> -		list_add_tail(&tool_pmu->list, &other_pmus);
> +		if (tool_pmu)
> +			list_add_tail(&tool_pmu->list, &other_pmus);
>   	}
>   	if ((to_read_types & PERF_TOOL_PMU_TYPE_HWMON_MASK) != 0 &&
>   	    (read_pmu_types & PERF_TOOL_PMU_TYPE_HWMON_MASK) == 0)
> diff --git a/tools/perf/util/tool_pmu.c b/tools/perf/util/tool_pmu.c
> index b60ac390d52d..d764c4734be6 100644
> --- a/tools/perf/util/tool_pmu.c
> +++ b/tools/perf/util/tool_pmu.c
> @@ -495,12 +495,21 @@ struct perf_pmu *tool_pmu__new(void)
>   {
>   	struct perf_pmu *tool = zalloc(sizeof(struct perf_pmu));
>   
> +	if (!tool)
> +		goto out;
>   	tool->name = strdup("tool");
> +	if (!tool->name) {
> +		zfree(tool);
> +		tool = NULL;

Hi Thomas,

zfree() already sets the thing to NULL but you need to pass a pointer to it:

   zfree(&tool);

Without doing that you only free the first u64 of the struct, which 
happens to be zero in this case so free() does nothing. Then zfree() 
sets the first u64 of the struct to zero which it already is.

With that fixed:

Reviewed-by: James Clark <james.clark@...aro.org>