[PATCH] drivers/perf: thunderx2_pmu: add error handling

[PATCH] drivers/perf: thunderx2_pmu: add error handling

[Greg Kroah-Hartman]

From: Griffin Kroah-Hartman <griffin@kroah.com>

Add error handling for devm_kasprintf functions in
tx2_uncore_pmu_register() and tx2_uncore_pmu_init_dev().

Regarding the latter function, I am unsure whether or not
PMU_TYPE_INVALID should be the default case.

Assisted-by: gkh_clanker_2000
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/perf/thunderx2_pmu.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
index 6ed4707bd6bb..1a6aa94a3efb 100644
--- a/drivers/perf/thunderx2_pmu.c
+++ b/drivers/perf/thunderx2_pmu.c
@@ -738,6 +738,8 @@ static int tx2_uncore_pmu_register(
 
 	tx2_pmu->pmu.name = devm_kasprintf(dev, GFP_KERNEL,
 			"%s", name);
+	if (!tx2_pmu->pmu.name)
+		return -ENOMEM;
 
 	return perf_pmu_register(&tx2_pmu->pmu, tx2_pmu->pmu.name, -1);
 }
@@ -837,6 +839,8 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev,
 		tx2_pmu->attr_groups = l3c_pmu_attr_groups;
 		tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL,
 				"uncore_l3c_%d", tx2_pmu->node);
+		if (!tx2_pmu->name)
+			goto err;
 		tx2_pmu->init_cntr_base = init_cntr_base_l3c;
 		tx2_pmu->start_event = uncore_start_event_l3c;
 		tx2_pmu->stop_event = uncore_stop_event_l3c;
@@ -852,6 +856,8 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev,
 		tx2_pmu->attr_groups = dmc_pmu_attr_groups;
 		tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL,
 				"uncore_dmc_%d", tx2_pmu->node);
+		if (!tx2_pmu->name)
+			goto err;
 		tx2_pmu->init_cntr_base = init_cntr_base_dmc;
 		tx2_pmu->start_event = uncore_start_event_dmc;
 		tx2_pmu->stop_event = uncore_stop_event_dmc;
@@ -866,17 +872,21 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev,
 		tx2_pmu->attr_groups = ccpi2_pmu_attr_groups;
 		tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL,
 				"uncore_ccpi2_%d", tx2_pmu->node);
+		if (!tx2_pmu->name)
+			goto err;
 		tx2_pmu->init_cntr_base = init_cntr_base_ccpi2;
 		tx2_pmu->start_event = uncore_start_event_ccpi2;
 		tx2_pmu->stop_event = uncore_stop_event_ccpi2;
 		tx2_pmu->hrtimer_callback = NULL;
 		break;
 	case PMU_TYPE_INVALID:
-		devm_kfree(dev, tx2_pmu);
-		return NULL;
+		goto err;
 	}
 
 	return tx2_pmu;
+err:
+	devm_kfree(dev, tx2_pmu);
+	return NULL;
 }
 
 static acpi_status tx2_uncore_pmu_add(acpi_handle handle, u32 level,
-- 
2.55.0

Re: [PATCH] drivers/perf: thunderx2_pmu: add error handling

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing unregistration on error paths during PMU addition and device probe leads to Use-After-Free of PMU and hotplug structures.
--

commit bfe9572d37443a1d40e13a34014a4451ab6d0e17
Author: Griffin Kroah-Hartman <griffin@kroah.com>

drivers/perf: thunderx2_pmu: add error handling

This commit adds missing error checks for devm_kasprintf() in
tx2_uncore_pmu_register() and tx2_uncore_pmu_init_dev(). It also centralizes
the error cleanup in tx2_uncore_pmu_init_dev() using a goto err pattern.

> diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
> index 6ed4707bd6bb5..1a6aa94a3efb9 100644
> --- a/drivers/perf/thunderx2_pmu.c
> +++ b/drivers/perf/thunderx2_pmu.c
> @@ -738,6 +738,8 @@ static int tx2_uncore_pmu_register(
>  
>  	tx2_pmu->pmu.name = devm_kasprintf(dev, GFP_KERNEL,
>  			"%s", name);
> +	if (!tx2_pmu->pmu.name)
> +		return -ENOMEM;
>  
>  	return perf_pmu_register(&tx2_pmu->pmu, tx2_pmu->pmu.name, -1);
>  }

[ ... ]

> @@ -866,17 +872,21 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev,
>  		tx2_pmu->attr_groups = ccpi2_pmu_attr_groups;
>  		tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL,
>  				"uncore_ccpi2_%d", tx2_pmu->node);
> +		if (!tx2_pmu->name)
> +			goto err;
>  		tx2_pmu->init_cntr_base = init_cntr_base_ccpi2;
>  		tx2_pmu->start_event = uncore_start_event_ccpi2;
>  		tx2_pmu->stop_event = uncore_stop_event_ccpi2;
>  		tx2_pmu->hrtimer_callback = NULL;
>  		break;
>  	case PMU_TYPE_INVALID:
> -		devm_kfree(dev, tx2_pmu);
> -		return NULL;
> +		goto err;
>  	}
>  
>  	return tx2_pmu;
> +err:
> +	devm_kfree(dev, tx2_pmu);
> +	return NULL;
>  }

[Severity: High]
This is a pre-existing issue, but looking at how error paths are handled,
could these new error returns lead to a use-after-free in the perf core or
CPU hotplug core?

If cpuhp_state_add_instance() fails in tx2_uncore_pmu_add_dev(), it returns
an error without calling perf_pmu_unregister() for the PMU that was just
registered:

	ret = tx2_uncore_pmu_register(tx2_pmu);
	if (ret) {
...
	}

	ret = cpuhp_state_add_instance(
			CPUHP_AP_PERF_ARM_CAVIUM_TX2_UNCORE_ONLINE,
			&tx2_pmu->hpnode);
	if (ret) {
		dev_err(tx2_pmu->dev, "Error %d registering hotplug", ret);
		return ret;
	}

Additionally, if devm_kasprintf() fails and returns NULL here,
tx2_uncore_pmu_add() will return AE_ERROR. This causes acpi_walk_namespace()
in tx2_uncore_probe() to fail and abort the probe:

	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
				     tx2_uncore_pmu_add,
				     NULL, dev, NULL);
	if (ACPI_FAILURE(status)) {
		dev_err(dev, "failed to probe PMU devices\n");
		return_ACPI_STATUS(status);
	}

Since tx2_uncore_probe() aborts with an error, the driver core will free
all devm_ allocated memory. Are the PMUs that were successfully added in
previous iterations of acpi_walk_namespace() unregistered before the memory
is freed, or will this leave dangling pointers in the core kernel subsystems?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/2026070635-feel-joyride-6d5b@gregkh?part=1