* [PATCH] perf pmu: Handle memory failure in tool_pmu__new()
@ 2025-03-19 9:28 Thomas Richter
2025-03-19 9:58 ` James Clark
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Richter @ 2025-03-19 9:28 UTC (permalink / raw)
To: linux-kernel, linux-s390, linux-perf-users, acme, namhyung,
irogers, james.clark
Cc: agordeev, gor, sumanthk, hca, Thomas Richter
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@linux.ibm.com>
Cc: Ian Rogers <irogers@google.com>
Cc: James Clark <james.clark@linaro.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;
+ goto out;
+ }
+
tool->type = PERF_PMU_TYPE_TOOL;
INIT_LIST_HEAD(&tool->aliases);
INIT_LIST_HEAD(&tool->caps);
INIT_LIST_HEAD(&tool->format);
tool->events_table = find_core_events_table("common", "common");
+out:
return tool;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] perf pmu: Handle memory failure in tool_pmu__new()
2025-03-19 9:28 [PATCH] perf pmu: Handle memory failure in tool_pmu__new() Thomas Richter
@ 2025-03-19 9:58 ` James Clark
2025-03-19 12:28 ` Thomas Richter
0 siblings, 1 reply; 3+ messages in thread
From: James Clark @ 2025-03-19 9:58 UTC (permalink / raw)
To: Thomas Richter
Cc: agordeev, gor, sumanthk, hca, linux-kernel, linux-s390,
linux-perf-users, acme, namhyung, irogers
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@linux.ibm.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: James Clark <james.clark@linaro.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@linaro.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] perf pmu: Handle memory failure in tool_pmu__new()
2025-03-19 9:58 ` James Clark
@ 2025-03-19 12:28 ` Thomas Richter
0 siblings, 0 replies; 3+ messages in thread
From: Thomas Richter @ 2025-03-19 12:28 UTC (permalink / raw)
To: James Clark
Cc: agordeev, gor, sumanthk, hca, linux-kernel, linux-s390,
linux-perf-users, acme, namhyung, irogers
On 3/19/25 10:58, James Clark wrote:
>
>
> 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@linux.ibm.com>
>> Cc: Ian Rogers <irogers@google.com>
>> Cc: James Clark <james.clark@linaro.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@linaro.org>
>
>
Thanks for the finding. I'll post verion 2 soon.
--
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-03-19 12:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-19 9:28 [PATCH] perf pmu: Handle memory failure in tool_pmu__new() Thomas Richter
2025-03-19 9:58 ` James Clark
2025-03-19 12:28 ` Thomas Richter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).