* [PATCH 1/5] EDAC/versalnet: Fix teardown ordering in mc_remove()
@ 2026-03-22 13:11 Prasanna Kumar T S M
2026-03-22 13:11 ` [PATCH 2/5] EDAC/versalnet: Release reference to remoteproc device in remove Prasanna Kumar T S M
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Prasanna Kumar T S M @ 2026-03-22 13:11 UTC (permalink / raw)
To: ptsm, shubhrajyoti.datta, bp, tony.luck, linux-edac, linux-kernel; +Cc: stable
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/5] EDAC/versalnet: Release reference to remoteproc device in remove
2026-03-22 13:11 [PATCH 1/5] EDAC/versalnet: Fix teardown ordering in mc_remove() Prasanna Kumar T S M
@ 2026-03-22 13:11 ` Prasanna Kumar T S M
2026-03-22 15:59 ` Borislav Petkov
2026-03-22 13:11 ` [PATCH 3/5] EDAC/versalnet: Fix memory leak in remove and probe error paths Prasanna Kumar T S M
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Prasanna Kumar T S M @ 2026-03-22 13:11 UTC (permalink / raw)
To: ptsm, shubhrajyoti.datta, bp, tony.luck, linux-edac, linux-kernel; +Cc: stable
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/5] EDAC/versalnet: Fix memory leak in remove and probe error paths
2026-03-22 13:11 [PATCH 1/5] EDAC/versalnet: Fix teardown ordering in mc_remove() Prasanna Kumar T S M
2026-03-22 13:11 ` [PATCH 2/5] EDAC/versalnet: Release reference to remoteproc device in remove Prasanna Kumar T S M
@ 2026-03-22 13:11 ` Prasanna Kumar T S M
2026-03-22 19:15 ` Borislav Petkov
2026-03-22 13:11 ` [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc() Prasanna Kumar T S M
2026-03-22 13:11 ` [PATCH 5/5] EDAC/versalnet: Fix device name memory leak Prasanna Kumar T S M
3 siblings, 1 reply; 15+ messages in thread
From: Prasanna Kumar T S M @ 2026-03-22 13:11 UTC (permalink / raw)
To: ptsm, shubhrajyoti.datta, bp, tony.luck, linux-edac, linux-kernel; +Cc: stable
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc()
2026-03-22 13:11 [PATCH 1/5] EDAC/versalnet: Fix teardown ordering in mc_remove() Prasanna Kumar T S M
2026-03-22 13:11 ` [PATCH 2/5] EDAC/versalnet: Release reference to remoteproc device in remove Prasanna Kumar T S M
2026-03-22 13:11 ` [PATCH 3/5] EDAC/versalnet: Fix memory leak in remove and probe error paths Prasanna Kumar T S M
@ 2026-03-22 13:11 ` Prasanna Kumar T S M
2026-03-22 16:10 ` Borislav Petkov
2026-03-22 13:11 ` [PATCH 5/5] EDAC/versalnet: Fix device name memory leak Prasanna Kumar T S M
3 siblings, 1 reply; 15+ messages in thread
From: Prasanna Kumar T S M @ 2026-03-22 13:11 UTC (permalink / raw)
To: ptsm, shubhrajyoti.datta, bp, tony.luck, linux-edac, linux-kernel; +Cc: stable
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/5] EDAC/versalnet: Fix device name memory leak
2026-03-22 13:11 [PATCH 1/5] EDAC/versalnet: Fix teardown ordering in mc_remove() Prasanna Kumar T S M
` (2 preceding siblings ...)
2026-03-22 13:11 ` [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc() Prasanna Kumar T S M
@ 2026-03-22 13:11 ` Prasanna Kumar T S M
2026-03-22 16:15 ` Borislav Petkov
2026-03-26 17:23 ` kernel test robot
3 siblings, 2 replies; 15+ messages in thread
From: Prasanna Kumar T S M @ 2026-03-22 13:11 UTC (permalink / raw)
To: ptsm, shubhrajyoti.datta, bp, tony.luck, linux-edac, linux-kernel; +Cc: stable
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] EDAC/versalnet: Release reference to remoteproc device in remove
2026-03-22 13:11 ` [PATCH 2/5] EDAC/versalnet: Release reference to remoteproc device in remove Prasanna Kumar T S M
@ 2026-03-22 15:59 ` Borislav Petkov
2026-03-23 7:25 ` Prasanna Kumar T S M
0 siblings, 1 reply; 15+ messages in thread
From: Borislav Petkov @ 2026-03-22 15:59 UTC (permalink / raw)
To: Prasanna Kumar T S M
Cc: shubhrajyoti.datta, tony.luck, linux-edac, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc()
2026-03-22 13:11 ` [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc() Prasanna Kumar T S M
@ 2026-03-22 16:10 ` Borislav Petkov
2026-03-23 7:08 ` Prasanna Kumar T S M
0 siblings, 1 reply; 15+ messages in thread
From: Borislav Petkov @ 2026-03-22 16:10 UTC (permalink / raw)
To: Prasanna Kumar T S M
Cc: shubhrajyoti.datta, tony.luck, linux-edac, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] EDAC/versalnet: Fix device name memory leak
2026-03-22 13:11 ` [PATCH 5/5] EDAC/versalnet: Fix device name memory leak Prasanna Kumar T S M
@ 2026-03-22 16:15 ` Borislav Petkov
2026-03-23 6:59 ` Prasanna Kumar T S M
2026-03-26 17:23 ` kernel test robot
1 sibling, 1 reply; 15+ messages in thread
From: Borislav Petkov @ 2026-03-22 16:15 UTC (permalink / raw)
To: Prasanna Kumar T S M
Cc: shubhrajyoti.datta, tony.luck, linux-edac, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/5] EDAC/versalnet: Fix memory leak in remove and probe error paths
2026-03-22 13:11 ` [PATCH 3/5] EDAC/versalnet: Fix memory leak in remove and probe error paths Prasanna Kumar T S M
@ 2026-03-22 19:15 ` Borislav Petkov
0 siblings, 0 replies; 15+ messages in thread
From: Borislav Petkov @ 2026-03-22 19:15 UTC (permalink / raw)
To: Prasanna Kumar T S M
Cc: shubhrajyoti.datta, tony.luck, linux-edac, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] EDAC/versalnet: Fix device name memory leak
2026-03-22 16:15 ` Borislav Petkov
@ 2026-03-23 6:59 ` Prasanna Kumar T S M
0 siblings, 0 replies; 15+ messages in thread
From: Prasanna Kumar T S M @ 2026-03-23 6:59 UTC (permalink / raw)
To: Borislav Petkov
Cc: shubhrajyoti.datta, tony.luck, linux-edac, linux-kernel, stable
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.
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc()
2026-03-22 16:10 ` Borislav Petkov
@ 2026-03-23 7:08 ` Prasanna Kumar T S M
2026-03-24 11:23 ` Borislav Petkov
0 siblings, 1 reply; 15+ messages in thread
From: Prasanna Kumar T S M @ 2026-03-23 7:08 UTC (permalink / raw)
To: Borislav Petkov
Cc: shubhrajyoti.datta, tony.luck, linux-edac, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] EDAC/versalnet: Release reference to remoteproc device in remove
2026-03-22 15:59 ` Borislav Petkov
@ 2026-03-23 7:25 ` Prasanna Kumar T S M
0 siblings, 0 replies; 15+ messages in thread
From: Prasanna Kumar T S M @ 2026-03-23 7:25 UTC (permalink / raw)
To: Borislav Petkov
Cc: shubhrajyoti.datta, tony.luck, linux-edac, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc()
2026-03-23 7:08 ` Prasanna Kumar T S M
@ 2026-03-24 11:23 ` Borislav Petkov
2026-03-24 12:16 ` Borislav Petkov
0 siblings, 1 reply; 15+ messages in thread
From: Borislav Petkov @ 2026-03-24 11:23 UTC (permalink / raw)
To: Prasanna Kumar T S M
Cc: shubhrajyoti.datta, tony.luck, linux-edac, linux-kernel, stable
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc()
2026-03-24 11:23 ` Borislav Petkov
@ 2026-03-24 12:16 ` Borislav Petkov
0 siblings, 0 replies; 15+ messages in thread
From: Borislav Petkov @ 2026-03-24 12:16 UTC (permalink / raw)
To: Prasanna Kumar T S M
Cc: shubhrajyoti.datta, tony.luck, linux-edac, linux-kernel, stable
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] EDAC/versalnet: Fix device name memory leak
2026-03-22 13:11 ` [PATCH 5/5] EDAC/versalnet: Fix device name memory leak Prasanna Kumar T S M
2026-03-22 16:15 ` Borislav Petkov
@ 2026-03-26 17:23 ` kernel test robot
1 sibling, 0 replies; 15+ messages in thread
From: kernel test robot @ 2026-03-26 17:23 UTC (permalink / raw)
To: Prasanna Kumar T S M, shubhrajyoti.datta, bp, tony.luck,
linux-edac, linux-kernel
Cc: llvm, oe-kbuild-all, stable
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
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-03-26 17:23 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-22 13:11 [PATCH 1/5] EDAC/versalnet: Fix teardown ordering in mc_remove() Prasanna Kumar T S M
2026-03-22 13:11 ` [PATCH 2/5] EDAC/versalnet: Release reference to remoteproc device in remove Prasanna Kumar T S M
2026-03-22 15:59 ` Borislav Petkov
2026-03-23 7:25 ` Prasanna Kumar T S M
2026-03-22 13:11 ` [PATCH 3/5] EDAC/versalnet: Fix memory leak in remove and probe error paths Prasanna Kumar T S M
2026-03-22 19:15 ` Borislav Petkov
2026-03-22 13:11 ` [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc() Prasanna Kumar T S M
2026-03-22 16:10 ` Borislav Petkov
2026-03-23 7:08 ` Prasanna Kumar T S M
2026-03-24 11:23 ` Borislav Petkov
2026-03-24 12:16 ` Borislav Petkov
2026-03-22 13:11 ` [PATCH 5/5] EDAC/versalnet: Fix device name memory leak Prasanna Kumar T S M
2026-03-22 16:15 ` Borislav Petkov
2026-03-23 6:59 ` Prasanna Kumar T S M
2026-03-26 17:23 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox