diff mbox series

[2/2] drivers/perf: hisi: add NULL check for name

Message ID 20230403081423.62460-3-hejunhao3@huawei.com (mailing list archive)
State New, archived
Headers show
Series Fix NULL pointer and doing cleanup | expand

Commit Message

Junhao He April 3, 2023, 8:14 a.m. UTC 
When allocations fails that can be NULL now.

If the name provided is NULL, then the initialization process of the PMU
type and dev will be skipped in function perf_pmu_register().
Consequently, the PMU will not be able to register into the kernel.
Moreover, in the case of unregister the PMU, the function device_del()
will need to handle NULL pointers, which potentially can cause issues.

So move this allocation above the cpuhp_state_add_instance() and directly
return if it does fail.

Signed-off-by: Junhao He <hejunhao3@huawei.com>
---
 drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 17 ++++++++++-------
 drivers/perf/hisilicon/hisi_uncore_hha_pmu.c  |  7 +++++--
 drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c  | 11 +++++------
 3 files changed, 20 insertions(+), 15 deletions(-)

Comments

Jonathan Cameron April 3, 2023, 11:08 a.m. UTC | #1 
On Mon, 3 Apr 2023 16:14:23 +0800
Junhao He <hejunhao3@huawei.com> wrote:

> When allocations fails that can be NULL now.
> 
> If the name provided is NULL, then the initialization process of the PMU
> type and dev will be skipped in function perf_pmu_register().
> Consequently, the PMU will not be able to register into the kernel.
> Moreover, in the case of unregister the PMU, the function device_del()
> will need to handle NULL pointers, which potentially can cause issues.

If you could be more specific in what the problem is wrt to a NULL pointer here
that would be great.  Given there is code to skip the device registration
without it being an error, I'd expect the release flow to be safe in that condition.

Seems like a sensible change even without that. So with some more info
feel free to add

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


> 
> So move this allocation above the cpuhp_state_add_instance() and directly
> return if it does fail.
> 
> Signed-off-by: Junhao He <hejunhao3@huawei.com>
> ---
>  drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 17 ++++++++++-------
>  drivers/perf/hisilicon/hisi_uncore_hha_pmu.c  |  7 +++++--
>  drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c  | 11 +++++------
>  3 files changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
> index 8a3d74ddcd6d..ffb039d05d07 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
> @@ -499,13 +499,6 @@ static int hisi_ddrc_pmu_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_DDRC_ONLINE,
> -				       &ddrc_pmu->node);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Error %d registering hotplug;\n", ret);
> -		return ret;
> -	}
> -
>  	if (ddrc_pmu->identifier >= HISI_PMU_V2)
>  		name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
>  				      "hisi_sccl%u_ddrc%u_%u",
> @@ -516,6 +509,16 @@ static int hisi_ddrc_pmu_probe(struct platform_device *pdev)
>  				      "hisi_sccl%u_ddrc%u", ddrc_pmu->sccl_id,
>  				      ddrc_pmu->index_id);
>  
> +	if (!name)
> +		return -ENOMEM;
> +
> +	ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_DDRC_ONLINE,
> +				       &ddrc_pmu->node);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Error %d registering hotplug;\n", ret);
> +		return ret;
> +	}
> +
>  	hisi_pmu_init(ddrc_pmu, THIS_MODULE);
>  
>  	ret = perf_pmu_register(&ddrc_pmu->pmu, name, -1);
> diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> index 5701a84edb0e..15caf99e1eef 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> @@ -510,6 +510,11 @@ static int hisi_hha_pmu_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_hha%u",
> +			      hha_pmu->sccl_id, hha_pmu->index_id);
> +	if (!name)
> +		return -ENOMEM;
> +
>  	ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_HHA_ONLINE,
>  				       &hha_pmu->node);
>  	if (ret) {
> @@ -517,8 +522,6 @@ static int hisi_hha_pmu_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_hha%u",
> -			      hha_pmu->sccl_id, hha_pmu->index_id);
>  	hisi_pmu_init(hha_pmu, THIS_MODULE);
>  
>  	ret = perf_pmu_register(&hha_pmu->pmu, name, -1);
> diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
> index 68596b566344..794dbcd19b7a 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
> @@ -544,6 +544,11 @@ static int hisi_l3c_pmu_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_l3c%u",
> +			      l3c_pmu->sccl_id, l3c_pmu->ccl_id);
> +	if (!name)
> +		return -ENOMEM;
> +
>  	ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_L3_ONLINE,
>  				       &l3c_pmu->node);
>  	if (ret) {
> @@ -551,12 +556,6 @@ static int hisi_l3c_pmu_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	/*
> -	 * CCL_ID is used to identify the L3C in the same SCCL which was
> -	 * used _UID by mistake.
> -	 */
> -	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_l3c%u",
> -			      l3c_pmu->sccl_id, l3c_pmu->ccl_id);
>  	hisi_pmu_init(l3c_pmu, THIS_MODULE);
>  
>  	ret = perf_pmu_register(&l3c_pmu->pmu, name, -1);
diff mbox series

Patch

diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
index 8a3d74ddcd6d..ffb039d05d07 100644
--- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
@@ -499,13 +499,6 @@  static int hisi_ddrc_pmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_DDRC_ONLINE,
-				       &ddrc_pmu->node);
-	if (ret) {
-		dev_err(&pdev->dev, "Error %d registering hotplug;\n", ret);
-		return ret;
-	}
-
 	if (ddrc_pmu->identifier >= HISI_PMU_V2)
 		name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
 				      "hisi_sccl%u_ddrc%u_%u",
@@ -516,6 +509,16 @@  static int hisi_ddrc_pmu_probe(struct platform_device *pdev)
 				      "hisi_sccl%u_ddrc%u", ddrc_pmu->sccl_id,
 				      ddrc_pmu->index_id);
 
+	if (!name)
+		return -ENOMEM;
+
+	ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_DDRC_ONLINE,
+				       &ddrc_pmu->node);
+	if (ret) {
+		dev_err(&pdev->dev, "Error %d registering hotplug;\n", ret);
+		return ret;
+	}
+
 	hisi_pmu_init(ddrc_pmu, THIS_MODULE);
 
 	ret = perf_pmu_register(&ddrc_pmu->pmu, name, -1);
diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
index 5701a84edb0e..15caf99e1eef 100644
--- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
@@ -510,6 +510,11 @@  static int hisi_hha_pmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_hha%u",
+			      hha_pmu->sccl_id, hha_pmu->index_id);
+	if (!name)
+		return -ENOMEM;
+
 	ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_HHA_ONLINE,
 				       &hha_pmu->node);
 	if (ret) {
@@ -517,8 +522,6 @@  static int hisi_hha_pmu_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_hha%u",
-			      hha_pmu->sccl_id, hha_pmu->index_id);
 	hisi_pmu_init(hha_pmu, THIS_MODULE);
 
 	ret = perf_pmu_register(&hha_pmu->pmu, name, -1);
diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
index 68596b566344..794dbcd19b7a 100644
--- a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
@@ -544,6 +544,11 @@  static int hisi_l3c_pmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_l3c%u",
+			      l3c_pmu->sccl_id, l3c_pmu->ccl_id);
+	if (!name)
+		return -ENOMEM;
+
 	ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_L3_ONLINE,
 				       &l3c_pmu->node);
 	if (ret) {
@@ -551,12 +556,6 @@  static int hisi_l3c_pmu_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	/*
-	 * CCL_ID is used to identify the L3C in the same SCCL which was
-	 * used _UID by mistake.
-	 */
-	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_l3c%u",
-			      l3c_pmu->sccl_id, l3c_pmu->ccl_id);
 	hisi_pmu_init(l3c_pmu, THIS_MODULE);
 
 	ret = perf_pmu_register(&l3c_pmu->pmu, name, -1);