[PATCH 1/5] EDAC/versalnet: Fix teardown ordering in mc_remove()

[PATCH 1/5] EDAC/versalnet: Fix teardown ordering in mc_remove()

[Prasanna Kumar T S M]

The teardown sequence in mc_remove() does not mirror the reverse of the
initialization order in mc_probe(). In particular,
unregister_rpmsg_driver() is called before remove_versalnet(), and
cdx_mcdi_finish() is called after rproc_shutdown().

Reorder mc_remove() to reverse the probe initialization sequence,
consistent with the probe error-unwind paths.

Fixes: d5fe2fec6c40d ("EDAC: Add a driver for the AMD Versal NET DDR controller")
Cc: stable@vger.kernel.org
Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
---
 drivers/edac/versalnet_edac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
index 0b47ed7fed63..f70243bc8a7a 100644
--- a/drivers/edac/versalnet_edac.c
+++ b/drivers/edac/versalnet_edac.c
@@ -954,10 +954,10 @@ static void mc_remove(struct platform_device *pdev)
 {
 	struct mc_priv *priv = platform_get_drvdata(pdev);
 
-	unregister_rpmsg_driver(&amd_rpmsg_driver);
 	remove_versalnet(priv);
-	rproc_shutdown(priv->mcdi->r5_rproc);
 	cdx_mcdi_finish(priv->mcdi);
+	unregister_rpmsg_driver(&amd_rpmsg_driver);
+	rproc_shutdown(priv->mcdi->r5_rproc);
 }
 
 static const struct of_device_id amd_edac_match[] = {
-- 
2.49.0

[PATCH 2/5] EDAC/versalnet: Release reference to remoteproc device in remove

[Prasanna Kumar T S M]

The rproc reference acquired via rproc_get_by_phandle() during probe
is not released in mc_remove(), causing a reference count leak. Add
the missing rproc_put() call.

Fixes: d5fe2fec6c40d ("EDAC: Add a driver for the AMD Versal NET DDR controller")
Cc: stable@vger.kernel.org
Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
---
 drivers/edac/versalnet_edac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
index f70243bc8a7a..28f5036f381c 100644
--- a/drivers/edac/versalnet_edac.c
+++ b/drivers/edac/versalnet_edac.c
@@ -958,6 +958,7 @@ static void mc_remove(struct platform_device *pdev)
 	cdx_mcdi_finish(priv->mcdi);
 	unregister_rpmsg_driver(&amd_rpmsg_driver);
 	rproc_shutdown(priv->mcdi->r5_rproc);
+	rproc_put(priv->mcdi->r5_rproc);
 }
 
 static const struct of_device_id amd_edac_match[] = {
-- 
2.49.0

Re: [PATCH 2/5] EDAC/versalnet: Release reference to remoteproc device in remove

[Borislav Petkov]

On Sun, Mar 22, 2026 at 06:11:34AM -0700, Prasanna Kumar T S M wrote:
> The rproc reference acquired via rproc_get_by_phandle() during probe
> is not released in mc_remove(), causing a reference count leak. Add
> the missing rproc_put() call.
> 
> Fixes: d5fe2fec6c40d ("EDAC: Add a driver for the AMD Versal NET DDR controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
> ---
>  drivers/edac/versalnet_edac.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
> index f70243bc8a7a..28f5036f381c 100644
> --- a/drivers/edac/versalnet_edac.c
> +++ b/drivers/edac/versalnet_edac.c
> @@ -958,6 +958,7 @@ static void mc_remove(struct platform_device *pdev)
>  	cdx_mcdi_finish(priv->mcdi);
>  	unregister_rpmsg_driver(&amd_rpmsg_driver);
>  	rproc_shutdown(priv->mcdi->r5_rproc);
> +	rproc_put(priv->mcdi->r5_rproc);
>  }
>  
>  static const struct of_device_id amd_edac_match[] = {
> -- 

Why is this a separate patch and not part of patch 1?

Also, do you have the hardware to test this on? IOW, have you tested those
patches?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 2/5] EDAC/versalnet: Release reference to remoteproc device in remove

[Prasanna Kumar T S M]

Hi Boris,

On 22-03-2026 21:29, Borislav Petkov wrote:
> On Sun, Mar 22, 2026 at 06:11:34AM -0700, Prasanna Kumar T S M wrote:
>> The rproc reference acquired via rproc_get_by_phandle() during probe
>> is not released in mc_remove(), causing a reference count leak. Add
>> the missing rproc_put() call.
>>
>> Fixes: d5fe2fec6c40d ("EDAC: Add a driver for the AMD Versal NET DDR controller")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
>> ---
>>   drivers/edac/versalnet_edac.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
>> index f70243bc8a7a..28f5036f381c 100644
>> --- a/drivers/edac/versalnet_edac.c
>> +++ b/drivers/edac/versalnet_edac.c
>> @@ -958,6 +958,7 @@ static void mc_remove(struct platform_device *pdev)
>>   	cdx_mcdi_finish(priv->mcdi);
>>   	unregister_rpmsg_driver(&amd_rpmsg_driver);
>>   	rproc_shutdown(priv->mcdi->r5_rproc);
>> +	rproc_put(priv->mcdi->r5_rproc);
>>   }
>>   
>>   static const struct of_device_id amd_edac_match[] = {
>> -- 
> 
> Why is this a separate patch and not part of patch 1?

I can merge this and the previous into a single patch. Will do as part 
of v2.

> 
> Also, do you have the hardware to test this on? IOW, have you tested those
> patches?

Yes, I do. Tested the change using 6.6 kernel with back ported driver.

Thanks,
Prasanna Kumar

[PATCH 3/5] EDAC/versalnet: Fix memory leak in remove and probe error paths

[Prasanna Kumar T S M]

The mcdi object allocated using kzalloc() in the setup_mcdi() is not
freed in the remove path or in probe's error handling path leading to
memory leak. Fix the memory leak by freeing the allocated memory.

Fixes: d5fe2fec6c40d ("EDAC: Add a driver for the AMD Versal NET DDR controller")
Cc: stable@vger.kernel.org
Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
---
 drivers/edac/versalnet_edac.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
index 28f5036f381c..acd51b492772 100644
--- a/drivers/edac/versalnet_edac.c
+++ b/drivers/edac/versalnet_edac.c
@@ -937,6 +937,7 @@ static int mc_probe(struct platform_device *pdev)
 
 err_init:
 	cdx_mcdi_finish(priv->mcdi);
+	kfree(priv->mcdi);
 
 err_unreg:
 	unregister_rpmsg_driver(&amd_rpmsg_driver);
@@ -959,6 +960,7 @@ static void mc_remove(struct platform_device *pdev)
 	unregister_rpmsg_driver(&amd_rpmsg_driver);
 	rproc_shutdown(priv->mcdi->r5_rproc);
 	rproc_put(priv->mcdi->r5_rproc);
+	kfree(priv->mcdi);
 }
 
 static const struct of_device_id amd_edac_match[] = {
-- 
2.49.0

Re: [PATCH 3/5] EDAC/versalnet: Fix memory leak in remove and probe error paths

[Borislav Petkov]

On Sun, Mar 22, 2026 at 06:11:39AM -0700, Prasanna Kumar T S M wrote:
> The mcdi object allocated using kzalloc() in the setup_mcdi() is not
> freed in the remove path or in probe's error handling path leading to
> memory leak. Fix the memory leak by freeing the allocated memory.
> 
> Fixes: d5fe2fec6c40d ("EDAC: Add a driver for the AMD Versal NET DDR controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
> ---
>  drivers/edac/versalnet_edac.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
> index 28f5036f381c..acd51b492772 100644
> --- a/drivers/edac/versalnet_edac.c
> +++ b/drivers/edac/versalnet_edac.c
> @@ -937,6 +937,7 @@ static int mc_probe(struct platform_device *pdev)
>  
>  err_init:
>  	cdx_mcdi_finish(priv->mcdi);
> +	kfree(priv->mcdi);
>  
>  err_unreg:
>  	unregister_rpmsg_driver(&amd_rpmsg_driver);
> @@ -959,6 +960,7 @@ static void mc_remove(struct platform_device *pdev)
>  	unregister_rpmsg_driver(&amd_rpmsg_driver);
>  	rproc_shutdown(priv->mcdi->r5_rproc);
>  	rproc_put(priv->mcdi->r5_rproc);
> +	kfree(priv->mcdi);
>  }
>  
>  static const struct of_device_id amd_edac_match[] = {
> -- 

Applied, thanks.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc()

[Prasanna Kumar T S M]

When device_register() fails, it must be followed by put_device()
rather than kfree(), because device_register() calls
device_initialize() which sets up the device refcount. The matching
release function versal_edac_release() handles the actual kfree().

Also reorder the dev allocation to after edac_mc_alloc() so the error
path no longer needs a separate err_dev_free label.

Fixes: d5fe2fec6c40d ("EDAC: Add a driver for the AMD Versal NET DDR controller")
Cc: stable@vger.kernel.org
Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
---
 drivers/edac/versalnet_edac.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
index acd51b492772..6463e88ed3d3 100644
--- a/drivers/edac/versalnet_edac.c
+++ b/drivers/edac/versalnet_edac.c
@@ -817,24 +817,26 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
 	if (!name)
 		return rc;
 
-	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-	if (!dev)
-		goto err_name_free;
-
 	mci = edac_mc_alloc(i, ARRAY_SIZE(layers), layers, sizeof(struct mc_priv));
 	if (!mci) {
 		edac_printk(KERN_ERR, EDAC_MC, "Failed memory allocation for MC%d\n", i);
-		goto err_dev_free;
+		goto err_name_free;
 	}
 
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		goto err_mc_free;
+
 	sprintf(name, "versal-net-ddrmc5-edac-%d", i);
 
 	dev->init_name = name;
 	dev->release = versal_edac_release;
 
 	rc = device_register(dev);
-	if (rc)
+	if (rc) {
+		put_device(dev);
 		goto err_mc_free;
+	}
 
 	mci->pdev = dev;
 	mc_init(mci, dev);
@@ -856,8 +858,6 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
 	device_unregister(mci->pdev);
 err_mc_free:
 	edac_mc_free(mci);
-err_dev_free:
-	kfree(dev);
 err_name_free:
 	kfree(name);
 
-- 
2.49.0

Re: [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc()

[Borislav Petkov]

On Sun, Mar 22, 2026 at 06:11:45AM -0700, Prasanna Kumar T S M wrote:
> When device_register() fails, it must be followed by put_device()
> rather than kfree(), because device_register() calls
> device_initialize() which sets up the device refcount. The matching
> release function versal_edac_release() handles the actual kfree().
> 
> Also reorder the dev allocation to after edac_mc_alloc() so the error
> path no longer needs a separate err_dev_free label.

Nope.

edac_mc_alloc() is a lot more heavy-weight than a simple k*alloc(). Pls keep
the ordering as it is.

> Fixes: d5fe2fec6c40d ("EDAC: Add a driver for the AMD Versal NET DDR controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
> ---
>  drivers/edac/versalnet_edac.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
> index acd51b492772..6463e88ed3d3 100644
> --- a/drivers/edac/versalnet_edac.c
> +++ b/drivers/edac/versalnet_edac.c
> @@ -817,24 +817,26 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
>  	if (!name)
>  		return rc;
>  
> -	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> -	if (!dev)
> -		goto err_name_free;
> -
>  	mci = edac_mc_alloc(i, ARRAY_SIZE(layers), layers, sizeof(struct mc_priv));
>  	if (!mci) {
>  		edac_printk(KERN_ERR, EDAC_MC, "Failed memory allocation for MC%d\n", i);
> -		goto err_dev_free;
> +		goto err_name_free;
>  	}
>  
> +	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> +	if (!dev)
> +		goto err_mc_free;
> +
>  	sprintf(name, "versal-net-ddrmc5-edac-%d", i);
>  
>  	dev->init_name = name;
>  	dev->release = versal_edac_release;
>  
>  	rc = device_register(dev);
> -	if (rc)
> +	if (rc) {
> +		put_device(dev);

Why here and not at the error label below?

>  		goto err_mc_free;
> +	}
>  
>  	mci->pdev = dev;
>  	mc_init(mci, dev);
> @@ -856,8 +858,6 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
>  	device_unregister(mci->pdev);
>  err_mc_free:
>  	edac_mc_free(mci);
> -err_dev_free:
> -	kfree(dev);
>  err_name_free:
>  	kfree(name);
>  
> -- 
> 2.49.0
> 

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc()

[Prasanna Kumar T S M]

On 22-03-2026 21:40, Borislav Petkov wrote:
> On Sun, Mar 22, 2026 at 06:11:45AM -0700, Prasanna Kumar T S M wrote:
>> When device_register() fails, it must be followed by put_device()
>> rather than kfree(), because device_register() calls
>> device_initialize() which sets up the device refcount. The matching
>> release function versal_edac_release() handles the actual kfree().
>>
>> Also reorder the dev allocation to after edac_mc_alloc() so the error
>> path no longer needs a separate err_dev_free label.
> 
> Nope.
> 
> edac_mc_alloc() is a lot more heavy-weight than a simple k*alloc(). Pls keep
> the ordering as it is.

Re-ordering this simplifies the error handling path. See below for details.

> 
>> Fixes: d5fe2fec6c40d ("EDAC: Add a driver for the AMD Versal NET DDR controller")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
>> ---
>>   drivers/edac/versalnet_edac.c | 16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
>> index acd51b492772..6463e88ed3d3 100644
>> --- a/drivers/edac/versalnet_edac.c
>> +++ b/drivers/edac/versalnet_edac.c
>> @@ -817,24 +817,26 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
>>   	if (!name)
>>   		return rc;
>>   
>> -	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>> -	if (!dev)
>> -		goto err_name_free;
>> -
>>   	mci = edac_mc_alloc(i, ARRAY_SIZE(layers), layers, sizeof(struct mc_priv));
>>   	if (!mci) {
>>   		edac_printk(KERN_ERR, EDAC_MC, "Failed memory allocation for MC%d\n", i);
>> -		goto err_dev_free;
>> +		goto err_name_free;
>>   	}
>>   
>> +	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>> +	if (!dev)
>> +		goto err_mc_free;
>> +
>>   	sprintf(name, "versal-net-ddrmc5-edac-%d", i);
>>   
>>   	dev->init_name = name;
>>   	dev->release = versal_edac_release;
>>   
>>   	rc = device_register(dev);
>> -	if (rc)
>> +	if (rc) {
>> +		put_device(dev);
> 
> Why here and not at the error label below?

After calling device_register(), put_device() has to be used 
irrespective of the return value. In this case, kfree(dev) will happen 
as a part of put_device().

If device_register() was not called, kfree(dev) has to be called to free 
the memory.

If kzalloc(dev) is done after edac_mc_alloc(), there is no need to 
decide between kfree(dev) or put_device(dev). This simplifies the error 
handling path. This is the reason behind re-ordering and keeping 
put_device(dev) under 'if (rc) { ... }'.

> 
>>   		goto err_mc_free;
>> +	}
>>   
>>   	mci->pdev = dev;
>>   	mc_init(mci, dev);
>> @@ -856,8 +858,6 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
>>   	device_unregister(mci->pdev);
>>   err_mc_free:
>>   	edac_mc_free(mci);
>> -err_dev_free:
>> -	kfree(dev);
>>   err_name_free:
>>   	kfree(name);
>>   
>> -- 
>> 2.49.0
>>
> 

Do you think it is best to do edac_mc_alloc() before kzalloc(dev)?

Thanks,
Prasanna Kumar

Re: [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc()

[Borislav Petkov]

On Mon, Mar 23, 2026 at 12:38:57PM +0530, Prasanna Kumar T S M wrote:
> If kzalloc(dev) is done after edac_mc_alloc(), there is no need to decide
> between kfree(dev) or put_device(dev). This simplifies the error handling
> path. This is the reason behind re-ordering and keeping put_device(dev)
> under 'if (rc) { ... }'.

I don't think you're listening to me so lemme repeat:

edac_mc_alloc() is a lot more heavy-weight than a simple k*alloc(). Pls keep
the ordering as it is.

I don't care how much it simplifies the error handling path if you have to do
all the allocations and setup edac_mc_alloc() does for *nothing*!

So let's do it another way (totally untested ofc):

---
diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
index b87fe57aa842..bf17b3ff59d5 100644
--- a/drivers/edac/versalnet_edac.c
+++ b/drivers/edac/versalnet_edac.c
@@ -772,12 +772,11 @@ static void remove_one_mc(struct mc_priv *priv, int i)
 	edac_mc_free(mci);
 }
 
-static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i)
+static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, struct device *dev, int i)
 {
 	u32 num_chans, rank, dwidth, config;
 	struct edac_mc_layer layers[2];
 	struct mem_ctl_info *mci;
-	struct device *dev;
 	enum dev_type dt;
 	char *name;
 	int rc;
@@ -817,14 +816,10 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
 	if (!name)
 		return rc;
 
-	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-	if (!dev)
-		goto err_name_free;
-
 	mci = edac_mc_alloc(i, ARRAY_SIZE(layers), layers, sizeof(struct mc_priv));
 	if (!mci) {
 		edac_printk(KERN_ERR, EDAC_MC, "Failed memory allocation for MC%d\n", i);
-		goto err_dev_free;
+		goto err_name_free;
 	}
 
 	sprintf(name, "versal-net-ddrmc5-edac-%d", i);
@@ -856,8 +851,6 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
 	device_unregister(mci->pdev);
 err_mc_free:
 	edac_mc_free(mci);
-err_dev_free:
-	kfree(dev);
 err_name_free:
 	kfree(name);
 
@@ -869,15 +862,21 @@ static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
 	int rc, i;
 
 	for (i = 0; i < NUM_CONTROLLERS; i++) {
-		rc = init_one_mc(priv, pdev, i);
-		if (rc) {
-			while (i--)
-				remove_one_mc(priv, i);
+		struct device *dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+		if (!dev)
+			goto free;
 
-			return rc;
-		}
+		rc = init_one_mc(priv, pdev, dev, i);
+		if (rc)
+			goto free;
 	}
 	return 0;
+
+free:
+	while (i--)
+		remove_one_mc(priv, i);
+
+	return rc;
 }
 
 static void remove_versalnet(struct mc_priv *priv)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc()

[Borislav Petkov]

On Tue, Mar 24, 2026 at 12:23:12PM +0100, Borislav Petkov wrote:
> So let's do it another way (totally untested ofc):

AI found a couple of issues, here's v2:

---
diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
index b87fe57aa842..953f96c8fd6f 100644
--- a/drivers/edac/versalnet_edac.c
+++ b/drivers/edac/versalnet_edac.c
@@ -772,12 +772,11 @@ static void remove_one_mc(struct mc_priv *priv, int i)
 	edac_mc_free(mci);
 }
 
-static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i)
+static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, struct device *dev, int i)
 {
 	u32 num_chans, rank, dwidth, config;
 	struct edac_mc_layer layers[2];
 	struct mem_ctl_info *mci;
-	struct device *dev;
 	enum dev_type dt;
 	char *name;
 	int rc;
@@ -802,7 +801,7 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
 	}
 
 	if (dt == DEV_UNKNOWN)
-		return 0;
+		return -EINVAL;
 
 	/* Find the first enabled device and register that one. */
 	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
@@ -817,14 +816,10 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
 	if (!name)
 		return rc;
 
-	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-	if (!dev)
-		goto err_name_free;
-
 	mci = edac_mc_alloc(i, ARRAY_SIZE(layers), layers, sizeof(struct mc_priv));
 	if (!mci) {
 		edac_printk(KERN_ERR, EDAC_MC, "Failed memory allocation for MC%d\n", i);
-		goto err_dev_free;
+		goto err_name_free;
 	}
 
 	sprintf(name, "versal-net-ddrmc5-edac-%d", i);
@@ -856,8 +851,6 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
 	device_unregister(mci->pdev);
 err_mc_free:
 	edac_mc_free(mci);
-err_dev_free:
-	kfree(dev);
 err_name_free:
 	kfree(name);
 
@@ -866,18 +859,26 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
 
 static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
 {
-	int rc, i;
+	int rc = -ENOMEM, i;
 
 	for (i = 0; i < NUM_CONTROLLERS; i++) {
-		rc = init_one_mc(priv, pdev, i);
-		if (rc) {
-			while (i--)
-				remove_one_mc(priv, i);
+		struct device *dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+		if (!dev)
+			goto free;
 
-			return rc;
+		rc = init_one_mc(priv, pdev, dev, i);
+		if (rc) {
+			kfree(dev);
+			goto free;
 		}
 	}
 	return 0;
+
+free:
+	while (i--)
+		remove_one_mc(priv, i);
+
+	return rc;
 }
 
 static void remove_versalnet(struct mc_priv *priv)


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH 5/5] EDAC/versalnet: Fix device name memory leak

[Prasanna Kumar T S M]

The device name allocated via kzalloc() in init_one_mc() is assigned to
dev->init_name but never freed on the normal removal path.
device_register() copies init_name and then sets dev->init_name to NULL,
so the name pointer becomes unreachable from the device. Thus leaking
memory.

Track the name pointer in mc_priv and free it in remove_one_mc().

Fixes: d5fe2fec6c40d ("EDAC: Add a driver for the AMD Versal NET DDR controller")
Cc: stable@vger.kernel.org
Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
---
 drivers/edac/versalnet_edac.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
index 6463e88ed3d3..17a5c8f416b9 100644
--- a/drivers/edac/versalnet_edac.c
+++ b/drivers/edac/versalnet_edac.c
@@ -158,6 +158,7 @@ struct mc_priv {
 	u32 regs[REG_MAX];
 	u32 adec[ADEC_MAX];
 	struct mem_ctl_info *mci[NUM_CONTROLLERS];
+	char *mci_name[NUM_CONTROLLERS];
 	struct rpmsg_endpoint *ept;
 	struct cdx_mcdi *mcdi;
 };
@@ -765,11 +766,14 @@ static void versal_edac_release(struct device *dev)
 static void remove_one_mc(struct mc_priv *priv, int i)
 {
 	struct mem_ctl_info *mci;
+	char *mci_name;
 
 	mci = priv->mci[i];
 	device_unregister(mci->pdev);
 	edac_mc_del_mc(mci->pdev);
 	edac_mc_free(mci);
+	mci_name = priv->mci_name[i];
+	kfree(mci_name);
 }
 
 static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i)
@@ -848,6 +852,7 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
 	}
 
 	priv->mci[i] = mci;
+	priv->mci_name[i] = name;
 	priv->dwidth = dt;
 
 	platform_set_drvdata(pdev, priv);
-- 
2.49.0

Re: [PATCH 5/5] EDAC/versalnet: Fix device name memory leak

[Borislav Petkov]

On Sun, Mar 22, 2026 at 06:11:49AM -0700, Prasanna Kumar T S M wrote:
> The device name allocated via kzalloc() in init_one_mc() is assigned to
> dev->init_name but never freed on the normal removal path.
> device_register() copies init_name and then sets dev->init_name to NULL,
> so the name pointer becomes unreachable from the device. Thus leaking
> memory.
> 
> Track the name pointer in mc_priv and free it in remove_one_mc().

No, get rid of the name allocation and allocate a char name[MC_NAME_LEN] on the
stack in init_one_mc() which you pass into device_register(), it copies it and
we forget about it.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 5/5] EDAC/versalnet: Fix device name memory leak

[Prasanna Kumar T S M]

On 22-03-2026 21:45, Borislav Petkov wrote:
> On Sun, Mar 22, 2026 at 06:11:49AM -0700, Prasanna Kumar T S M wrote:
>> The device name allocated via kzalloc() in init_one_mc() is assigned to
>> dev->init_name but never freed on the normal removal path.
>> device_register() copies init_name and then sets dev->init_name to NULL,
>> so the name pointer becomes unreachable from the device. Thus leaking
>> memory.
>>
>> Track the name pointer in mc_priv and free it in remove_one_mc().
> 
> No, get rid of the name allocation and allocate a char name[MC_NAME_LEN] on the
> stack in init_one_mc() which you pass into device_register(), it copies it and
> we forget about it.

Sure, I will do this in v2.

> Thx.
> 

Re: [PATCH 5/5] EDAC/versalnet: Fix device name memory leak

[kernel test robot]

Hi Prasanna,

kernel test robot noticed the following build warnings:

[auto build test WARNING on next-20260320]
[cannot apply to ras/edac-for-next linus/master v7.0-rc5 v7.0-rc4 v7.0-rc3 v7.0-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Prasanna-Kumar-T-S-M/EDAC-versalnet-Release-reference-to-remoteproc-device-in-remove/20260324-014933
base:   next-20260320
patch link:    https://lore.kernel.org/r/20260322131149.1684771-1-ptsm%40linux.microsoft.com
patch subject: [PATCH 5/5] EDAC/versalnet: Fix device name memory leak
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20260327/202603270107.jJ92lLm3-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260327/202603270107.jJ92lLm3-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603270107.jJ92lLm3-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/edac/versalnet_edac.c:163 struct member 'mci_name' not described in 'mc_priv'
>> Warning: drivers/edac/versalnet_edac.c:163 struct member 'mci_name' not described in 'mc_priv'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki