* [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
@ 2026-02-21 7:16 Eric-Terminal
2026-02-22 2:44 ` Borislav Petkov
0 siblings, 1 reply; 14+ messages in thread
From: Eric-Terminal @ 2026-02-21 7:16 UTC (permalink / raw)
To: Shubhrajyoti Datta
Cc: Borislav Petkov, Tony Luck, linux-edac, linux-kernel,
Eric-Terminal
init_versalnet() has several bugs in its error handling:
- kzalloc() and kmalloc() return values are used without NULL checks,
causing a NULL pointer dereference when allocation fails.
- The cleanup loop uses while (i--) which skips the current failing
index, leaking the resources already allocated for that slot.
- edac_mc_del_mc() is called unconditionally during unwind, even for
controllers that were never registered with edac_mc_add_mc().
- sprintf() is used instead of snprintf() on a fixed-size buffer.
Fix by adding NULL checks for dev and name allocations, replacing
while (i--) with for (j = i; j >= 0; j--) to include the failing
index, tracking successful edac_mc_add_mc() calls with a bool array,
and switching to snprintf().
Signed-off-by: Eric-Terminal <ericterminal@gmail.com>
---
drivers/edac/versalnet_edac.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
index 1a1092793..128e9cd5f 100644
--- a/drivers/edac/versalnet_edac.c
+++ b/drivers/edac/versalnet_edac.c
@@ -764,11 +764,12 @@ static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
{
u32 num_chans, rank, dwidth, config;
struct edac_mc_layer layers[2];
+ bool mc_added[NUM_CONTROLLERS] = { };
struct mem_ctl_info *mci;
struct device *dev;
enum dev_type dt;
char *name;
- int rc, i;
+ int rc, i, j;
for (i = 0; i < NUM_CONTROLLERS; i++) {
config = priv->adec[CONF + i * ADEC_NUM];
@@ -813,11 +814,25 @@ static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
priv->dwidth = dt;
dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev) {
+ rc = -ENOMEM;
+ goto err_alloc;
+ }
+
dev->release = versal_edac_release;
name = kmalloc(32, GFP_KERNEL);
- sprintf(name, "versal-net-ddrmc5-edac-%d", i);
+ if (!name) {
+ kfree(dev);
+ rc = -ENOMEM;
+ goto err_alloc;
+ }
+
+ snprintf(name, 32, "versal-net-ddrmc5-edac-%d", i);
dev->init_name = name;
rc = device_register(dev);
+ kfree(name);
+ if (rc)
+ put_device(dev);
if (rc)
goto err_alloc;
@@ -831,21 +846,25 @@ static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
edac_printk(KERN_ERR, EDAC_MC, "Failed to register MC%d with EDAC core\n", i);
goto err_alloc;
}
+
+ mc_added[i] = true;
}
return 0;
err_alloc:
- while (i--) {
- mci = priv->mci[i];
+ for (j = i; j >= 0; j--) {
+ mci = priv->mci[j];
if (!mci)
continue;
if (mci->pdev) {
device_unregister(mci->pdev);
- edac_mc_del_mc(mci->pdev);
+ if (mc_added[j])
+ edac_mc_del_mc(mci->pdev);
}
edac_mc_free(mci);
+ priv->mci[j] = NULL;
}
return rc;
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
2026-02-21 7:16 [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet() Eric-Terminal
@ 2026-02-22 2:44 ` Borislav Petkov
[not found] ` <CAKNPVZA36XvT7R+QWTOTvAeQNA-sZfnG-h5jV3FnKvKOxqiGtA@mail.gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2026-02-22 2:44 UTC (permalink / raw)
To: Eric-Terminal, Shubhrajyoti Datta; +Cc: Tony Luck, linux-edac, linux-kernel
On February 21, 2026 7:16:43 AM UTC, Eric-Terminal <ericterminal@gmail.com> wrote:
>init_versalnet() has several bugs in its error handling:
I'm waiting for a while now for Shubhrajyoti to do something about some outstanding issues there but no move. Might as well remove him from MAINTAINERS and let this driver rot...
>Signed-off-by: Eric-Terminal <ericterminal@gmail.com>
Your last name is "Terminal"?!?
--
Small device. Typos and formatting crap
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
[not found] ` <FE3C5380-E82B-4EC8-B1C0-16026CEEF8A2@alien8.de>
@ 2026-02-22 10:39 ` Eric-Terminal
2026-02-23 13:47 ` Borislav Petkov
2026-02-22 10:48 ` [PATCH] " Eric_Terminal
1 sibling, 1 reply; 14+ messages in thread
From: Eric-Terminal @ 2026-02-22 10:39 UTC (permalink / raw)
To: Borislav Petkov
Cc: Shubhrajyoti Datta, Tony Luck, linux-edac, linux-kernel,
Yufan Chen
From: Yufan Chen <ericterminal@gmail.com>
init_versalnet() has several bugs in its error handling:
- kzalloc() and kmalloc() return values are used without NULL checks,
causing a NULL pointer dereference when allocation fails.
- The cleanup loop uses while (i--) which skips the current failing
index, leaking the resources already allocated for that slot.
- edac_mc_del_mc() is called unconditionally during unwind, even for
controllers that were never registered with edac_mc_add_mc().
- sprintf() is used instead of snprintf() on a fixed-size buffer.
Fix by adding NULL checks for dev and name allocations, replacing
while (i--) with for (j = i; j >= 0; j--) to include the failing
index, tracking successful edac_mc_add_mc() calls with a bool array,
and switching to snprintf().
Signed-off-by: Yufan Chen <ericterminal@gmail.com>
---
drivers/edac/versalnet_edac.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
index 1a1092793..128e9cd5f 100644
--- a/drivers/edac/versalnet_edac.c
+++ b/drivers/edac/versalnet_edac.c
@@ -764,11 +764,12 @@ static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
{
u32 num_chans, rank, dwidth, config;
struct edac_mc_layer layers[2];
+ bool mc_added[NUM_CONTROLLERS] = { };
struct mem_ctl_info *mci;
struct device *dev;
enum dev_type dt;
char *name;
- int rc, i;
+ int rc, i, j;
for (i = 0; i < NUM_CONTROLLERS; i++) {
config = priv->adec[CONF + i * ADEC_NUM];
@@ -813,11 +814,25 @@ static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
priv->dwidth = dt;
dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev) {
+ rc = -ENOMEM;
+ goto err_alloc;
+ }
+
dev->release = versal_edac_release;
name = kmalloc(32, GFP_KERNEL);
- sprintf(name, "versal-net-ddrmc5-edac-%d", i);
+ if (!name) {
+ kfree(dev);
+ rc = -ENOMEM;
+ goto err_alloc;
+ }
+
+ snprintf(name, 32, "versal-net-ddrmc5-edac-%d", i);
dev->init_name = name;
rc = device_register(dev);
+ kfree(name);
+ if (rc)
+ put_device(dev);
if (rc)
goto err_alloc;
@@ -831,21 +846,25 @@ static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
edac_printk(KERN_ERR, EDAC_MC, "Failed to register MC%d with EDAC core\n", i);
goto err_alloc;
}
+
+ mc_added[i] = true;
}
return 0;
err_alloc:
- while (i--) {
- mci = priv->mci[i];
+ for (j = i; j >= 0; j--) {
+ mci = priv->mci[j];
if (!mci)
continue;
if (mci->pdev) {
device_unregister(mci->pdev);
- edac_mc_del_mc(mci->pdev);
+ if (mc_added[j])
+ edac_mc_del_mc(mci->pdev);
}
edac_mc_free(mci);
+ priv->mci[j] = NULL;
}
return rc;
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
[not found] ` <FE3C5380-E82B-4EC8-B1C0-16026CEEF8A2@alien8.de>
2026-02-22 10:39 ` Eric-Terminal
@ 2026-02-22 10:48 ` Eric_Terminal
2026-02-23 8:54 ` Borislav Petkov
1 sibling, 1 reply; 14+ messages in thread
From: Eric_Terminal @ 2026-02-22 10:48 UTC (permalink / raw)
To: Borislav Petkov; +Cc: Shubhrajyoti Datta, linux-kernel, Tony Luck, linux-edac
On Sun, Feb 22, 2026 at 3:22 PM Borislav Petkov <bp@alien8.de> wrote:
> First of all,
>
> do not top-post when replying to community mails.
My apologies for the formatting errors and for accidentally taking
this discussion off-list. I'm a newcomer and still learning the
etiquette.
> Then, when you reply, hit reply-to-all in your mail client. The
> discussions we have are not private ones but are documented on public
> mailing lists for a reason.
Understood. I have added the mailing lists back to CC and will ensure
all future technical discussions remain public.
> Then, about this particular question:
>
> "...then you just add a line saying:
>
> Signed-off-by: Random J Developer <random@developer.example.org>
>
> using your real name (sorry, no pseudonyms or anonymous contributions.)"
I see. Thank you for the clarification. My real name is Yufan Chen.
I have thoroughly read the submission documentation and have just
resubmitted the patch as V2 with the corrected
Signed-off-by: Yufan Chen <ericterminal@gmail.com>.
Best regards,
Yufan Chen
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
2026-02-22 10:48 ` [PATCH] " Eric_Terminal
@ 2026-02-23 8:54 ` Borislav Petkov
2026-02-23 10:08 ` Eric_Terminal
0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2026-02-23 8:54 UTC (permalink / raw)
To: ericterminal; +Cc: shubhrajyoti.datta, linux-kernel, tony.luck, linux-edac
Eric_Terminal <ericterminal@gmail.com> wrote:
>I have thoroughly read the submission documentation and have just
>resubmitted the patch as V2 with the corrected
In my not-so-short experience no new submitter just reads all of the submitting-patches document without complaining at least a little and
replies with a new version of the patch - all that in three hours, and all that in way too polished English.
If I look at the actual diff, am I going to find a bunch of nonsensical AI slop and am I going to waste my time talking to an AI bot as a result?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
2026-02-23 8:54 ` Borislav Petkov
@ 2026-02-23 10:08 ` Eric_Terminal
2026-02-23 10:40 ` Borislav Petkov
0 siblings, 1 reply; 14+ messages in thread
From: Eric_Terminal @ 2026-02-23 10:08 UTC (permalink / raw)
To: Borislav Petkov; +Cc: shubhrajyoti.datta, linux-kernel, tony.luck, linux-edac
On Mon, Feb 23, 2026 at 4:54 PM Borislav Petkov <bp@alien8.de> wrote:
> In my not-so-short experience no new submitter just reads all of the submitting-patches document without complaining at least a little and
> replies with a new version of the patch - all that in three hours, and all that in way too polished English.
To explain the "way too polished English": English is not my native
language. Because I want to show proper respect to the kernel
community, I relied on translation and grammar tools to make sure my
replies were polite and appropriate. That is why the text might look
artificially perfect.
As for not complaining: I don't know how other submitters do it, but
as a newcomer, I honestly didn't dare to complain on the mailing list.
I simply assumed that complaining would leave a bad impression and
might get my patch rejected. So I just tried my best to read the
documentation and follow the rules as quickly as possible.
Regarding the speed of sending a "new version" in three hours: it
wasn't a rewrite of the patch. I merely updated the Signed-off-by tag
to my real name, re-signed it with patatt, and sent it out. The code
itself didn't change at all, which is why it was fast.
> If I look at the actual diff, am I going to find a bunch of nonsensical AI slop and am I going to waste my time talking to an AI bot as a result?
If I were a fully automated AI bot, I probably wouldn't have made the
very human mistakes of top-posting and forgetting to "Reply-to-all" in
my email before the last one. :)
I completely understand your concern about AI-generated spam.
However, the NULL pointer dereferences and memory leaks in
init_versalnet() are genuine logical flaws. I hope you can take a
brief moment to review the actual v2 diff. If there is any technical
issue with my code, I am more than happy to learn and correct it.
Best regards,
Yufan Chen (Eric Terminal)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
2026-02-23 10:08 ` Eric_Terminal
@ 2026-02-23 10:40 ` Borislav Petkov
2026-02-23 10:52 ` Eric_Terminal
0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2026-02-23 10:40 UTC (permalink / raw)
To: Eric_Terminal; +Cc: shubhrajyoti.datta, linux-kernel, tony.luck, linux-edac
On February 23, 2026 10:08:02 AM UTC, Eric_Terminal <ericterminal@gmail.com> wrote:
>To explain the "way too polished English": English is not my native
>language. Because I want to show proper respect to the kernel
>community, I relied on translation and grammar tools to make sure my
>replies were polite and appropriate. That is why the text might look
>artificially perfect.
Such quality must come from a LLM-like tool...
>As for not complaining: I don't know how other submitters do it, but
>as a newcomer, I honestly didn't dare to complain on the mailing list.
>I simply assumed that complaining would leave a bad impression and
>might get my patch rejected. So I just tried my best to read the
>documentation and follow the rules as quickly as possible.
You should never aim for "quickly" but for correctly. Never rush your replies.
>If I were a fully automated AI bot, I probably
So you are using some AI to generate your mails? You should state that in your commit message.
But I'll take a look at your patch first...
--
Small device. Typos and formatting crap
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
2026-02-23 10:40 ` Borislav Petkov
@ 2026-02-23 10:52 ` Eric_Terminal
0 siblings, 0 replies; 14+ messages in thread
From: Eric_Terminal @ 2026-02-23 10:52 UTC (permalink / raw)
To: Borislav Petkov; +Cc: shubhrajyoti.datta, linux-kernel, tony.luck, linux-edac
On Mon, Feb 23, 2026 at 6:41 PM Borislav Petkov <bp@alien8.de> wrote:
> So you are using some AI to generate your mails? You should state that in your commit message.
Okay, I get it. I indeed used AI to polish my emails... it even
deleted all my emojis. If I wrote directly, it would probably sound
like this. After all, my own English isn't as "inspired" as the AI's.
Do I need to resubmit and declare this in the commit message?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
2026-02-22 10:39 ` Eric-Terminal
@ 2026-02-23 13:47 ` Borislav Petkov
2026-02-23 14:58 ` Eric_Terminal
0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2026-02-23 13:47 UTC (permalink / raw)
To: Eric-Terminal
Cc: Shubhrajyoti Datta, Tony Luck, linux-edac, linux-kernel,
Yufan Chen
On February 22, 2026 10:39:18 AM UTC, Eric-Terminal <ericterminal@gmail.com> wrote:
>From: Yufan Chen <ericterminal@gmail.com>
>
>init_versalnet() has several bugs in its error handling:
>
> - kzalloc() and kmalloc() return values are used without NULL checks,
> causing a NULL pointer dereference when allocation fails.
>
> - The cleanup loop uses while (i--) which skips the current failing
> index, leaking the resources already allocated for that slot.
>
> - edac_mc_del_mc() is called unconditionally during unwind, even for
> controllers that were never registered with edac_mc_add_mc().
>
> - sprintf() is used instead of snprintf() on a fixed-size buffer.
>
>Fix by adding NULL checks for dev and name allocations, replacing
>while (i--) with for (j = i; j >= 0; j--) to include the failing
>index, tracking successful edac_mc_add_mc() calls with a bool array,
>and switching to snprintf().
>
>Signed-off-by: Yufan Chen <ericterminal@gmail.com>
>---
> drivers/edac/versalnet_edac.c | 29 ++++++++++++++++++++++++-----
So that particular area of stinking goo is being dealt with here:
<https://lore.kernel.org/all/20251104093932.3838876-1-shubhrajyoti.datta@amd.com/T/#u>
And I'd usually say you can take the current diffs, productize them and send them after testing.
However, testing is the problem here - I highly doubt you have access to the hardware and Shubhrajyoti is probably one of small number of people who can test it.
Except that he's not really moving here - this particular issue has been outstanding for at least three months.
I'll ping him when I get back from vacation to see whether there's still interest in this driver or I can remove it for lack of support.
Oh well.
--
Small device. Typos and formatting crap
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
2026-02-23 13:47 ` Borislav Petkov
@ 2026-02-23 14:58 ` Eric_Terminal
2026-02-26 6:54 ` Datta, Shubhrajyoti
0 siblings, 1 reply; 14+ messages in thread
From: Eric_Terminal @ 2026-02-23 14:58 UTC (permalink / raw)
To: Borislav Petkov; +Cc: Shubhrajyoti Datta, Tony Luck, linux-edac, linux-kernel
On Mon, Feb 23, 2026 at 9:47 PM Borislav Petkov <bp@alien8.de> wrote:
> And I'd usually say you can take the current diffs, productize them and send them after testing.
Uh, yeah, that’s true — after all, most of us regular developers don’t
exactly have an FPGA sitting around at home.
> However, testing is the problem here - I highly doubt you have access to the hardware and Shubhrajyoti is probably one of small number of people who can test it.
This is purely a logic bug in the code. What I did was just fix an
error-handling path; it doesn’t change the normal execution flow at
all. The issue itself can be derived through static analysis of the
existing code, so it’s not something that depends on specific hardware
behavior.
> Except that he's not really moving here - this particular issue has been outstanding for at least three months.
If the driver isn’t actively maintained… well… I can understand that
too. Still, this patch is very low risk, and merging it shouldn’t
really add to your workload. Maybe straightforward logic fixes like
this could be considered based on static review alone? At the very
least, it would help improve the baseline code quality. :)
Anyway, enjoy your vacation.
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
2026-02-23 14:58 ` Eric_Terminal
@ 2026-02-26 6:54 ` Datta, Shubhrajyoti
2026-02-26 11:29 ` [PATCH v2] " Eric-Terminal
0 siblings, 1 reply; 14+ messages in thread
From: Datta, Shubhrajyoti @ 2026-02-26 6:54 UTC (permalink / raw)
To: Eric_Terminal, Borislav Petkov
Cc: Tony Luck, linux-edac@vger.kernel.org,
linux-kernel@vger.kernel.org
[AMD Official Use Only - AMD Internal Distribution Only]
> -----Original Message-----
> From: Eric_Terminal <ericterminal@gmail.com>
> Sent: Monday, February 23, 2026 8:28 PM
> To: Borislav Petkov <bp@alien8.de>
> Cc: Datta, Shubhrajyoti <shubhrajyoti.datta@amd.com>; Tony Luck
> <tony.luck@intel.com>; linux-edac@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in
> init_versalnet()
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Mon, Feb 23, 2026 at 9:47 PM Borislav Petkov <bp@alien8.de> wrote:
> > And I'd usually say you can take the current diffs, productize them and send
> them after testing.
>
I was out last week because of a personal emergency. My apologies on the delay.
BTW I was able to test the patch.
Feel free to add my
Reviewed-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
However are you planning to send a v2 correcting the small nits like name etc?
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
2026-02-26 6:54 ` Datta, Shubhrajyoti
@ 2026-02-26 11:29 ` Eric-Terminal
2026-02-26 12:59 ` Borislav Petkov
0 siblings, 1 reply; 14+ messages in thread
From: Eric-Terminal @ 2026-02-26 11:29 UTC (permalink / raw)
To: Shubhrajyoti Datta
Cc: Borislav Petkov, Tony Luck, linux-edac, linux-kernel, Yufan Chen
From: Yufan Chen <ericterminal@gmail.com>
init_versalnet() has several bugs in its error handling. kzalloc() and
kmalloc() return values are used without NULL checks, causing a NULL
pointer dereference when allocation fails. The cleanup loop uses
while (i--) which skips the current failing index, leaking the
resources already allocated for that slot. edac_mc_del_mc() is called
unconditionally during unwind, even for controllers that were never
registered with edac_mc_add_mc(). Also, sprintf() is used instead of
snprintf() on a fixed-size buffer.
Fix by adding NULL checks for dev and name allocations, replacing
while (i--) with for (j = i; j >= 0; j--) to include the failing
index, tracking successful edac_mc_add_mc() calls with a bool array,
and switching to snprintf().
Signed-off-by: Yufan Chen <ericterminal@gmail.com>
Reviewed-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
---
v2: Correct Signed-off-by name and add Reviewed-by tag. Fix commit message formatting.
drivers/edac/versalnet_edac.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
index 1a1092793..128e9cd5f 100644
--- a/drivers/edac/versalnet_edac.c
+++ b/drivers/edac/versalnet_edac.c
@@ -764,11 +764,12 @@ static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
{
u32 num_chans, rank, dwidth, config;
struct edac_mc_layer layers[2];
+ bool mc_added[NUM_CONTROLLERS] = { };
struct mem_ctl_info *mci;
struct device *dev;
enum dev_type dt;
char *name;
- int rc, i;
+ int rc, i, j;
for (i = 0; i < NUM_CONTROLLERS; i++) {
config = priv->adec[CONF + i * ADEC_NUM];
@@ -813,11 +814,25 @@ static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
priv->dwidth = dt;
dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev) {
+ rc = -ENOMEM;
+ goto err_alloc;
+ }
+
dev->release = versal_edac_release;
name = kmalloc(32, GFP_KERNEL);
- sprintf(name, "versal-net-ddrmc5-edac-%d", i);
+ if (!name) {
+ kfree(dev);
+ rc = -ENOMEM;
+ goto err_alloc;
+ }
+
+ snprintf(name, 32, "versal-net-ddrmc5-edac-%d", i);
dev->init_name = name;
rc = device_register(dev);
+ kfree(name);
+ if (rc)
+ put_device(dev);
if (rc)
goto err_alloc;
@@ -831,21 +846,25 @@ static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
edac_printk(KERN_ERR, EDAC_MC, "Failed to register MC%d with EDAC core\n", i);
goto err_alloc;
}
+
+ mc_added[i] = true;
}
return 0;
err_alloc:
- while (i--) {
- mci = priv->mci[i];
+ for (j = i; j >= 0; j--) {
+ mci = priv->mci[j];
if (!mci)
continue;
if (mci->pdev) {
device_unregister(mci->pdev);
- edac_mc_del_mc(mci->pdev);
+ if (mc_added[j])
+ edac_mc_del_mc(mci->pdev);
}
edac_mc_free(mci);
+ priv->mci[j] = NULL;
}
return rc;
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
2026-02-26 11:29 ` [PATCH v2] " Eric-Terminal
@ 2026-02-26 12:59 ` Borislav Petkov
2026-02-26 16:52 ` Datta, Shubhrajyoti
0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2026-02-26 12:59 UTC (permalink / raw)
To: Eric-Terminal; +Cc: Shubhrajyoti Datta, Tony Luck, linux-edac, linux-kernel
On Thu, Feb 26, 2026 at 07:29:07PM +0800, Eric-Terminal wrote:
> From: Yufan Chen <ericterminal@gmail.com>
>
> init_versalnet() has several bugs in its error handling. kzalloc() and
> kmalloc() return values are used without NULL checks, causing a NULL
> pointer dereference when allocation fails. The cleanup loop uses
> while (i--) which skips the current failing index, leaking the
> resources already allocated for that slot. edac_mc_del_mc() is called
> unconditionally during unwind, even for controllers that were never
> registered with edac_mc_add_mc(). Also, sprintf() is used instead of
> snprintf() on a fixed-size buffer.
>
> Fix by adding NULL checks for dev and name allocations, replacing
> while (i--) with for (j = i; j >= 0; j--) to include the failing
> index, tracking successful edac_mc_add_mc() calls with a bool array,
> and switching to snprintf().
>
> Signed-off-by: Yufan Chen <ericterminal@gmail.com>
> Reviewed-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
> ---
> v2: Correct Signed-off-by name and add Reviewed-by tag. Fix commit message formatting.
>
> drivers/edac/versalnet_edac.c | 29 ++++++++++++++++++++++++-----
> 1 file changed, 24 insertions(+), 5 deletions(-)
No, thanks, we'll do the proper cleanup here:
https://lore.kernel.org/all/20251104093932.3838876-1-shubhrajyoti.datta@amd.com/T/#u
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v2] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet()
2026-02-26 12:59 ` Borislav Petkov
@ 2026-02-26 16:52 ` Datta, Shubhrajyoti
0 siblings, 0 replies; 14+ messages in thread
From: Datta, Shubhrajyoti @ 2026-02-26 16:52 UTC (permalink / raw)
To: Borislav Petkov, Eric-Terminal
Cc: Tony Luck, linux-edac@vger.kernel.org,
linux-kernel@vger.kernel.org
[AMD Official Use Only - AMD Internal Distribution Only]
> -----Original Message-----
> From: Borislav Petkov <bp@alien8.de>
> Sent: Thursday, February 26, 2026 6:30 PM
> To: Eric-Terminal <ericterminal@gmail.com>
> Cc: Datta, Shubhrajyoti <shubhrajyoti.datta@amd.com>; Tony Luck
> <tony.luck@intel.com>; linux-edac@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v2] EDAC/versalnet: Fix resource leaks and NULL derefs in
> init_versalnet()
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Thu, Feb 26, 2026 at 07:29:07PM +0800, Eric-Terminal wrote:
> > From: Yufan Chen <ericterminal@gmail.com>
> >
> > init_versalnet() has several bugs in its error handling. kzalloc() and
> > kmalloc() return values are used without NULL checks, causing a NULL
> > pointer dereference when allocation fails. The cleanup loop uses while
> > (i--) which skips the current failing index, leaking the resources
> > already allocated for that slot. edac_mc_del_mc() is called
> > unconditionally during unwind, even for controllers that were never
> > registered with edac_mc_add_mc(). Also, sprintf() is used instead of
> > snprintf() on a fixed-size buffer.
> >
> > Fix by adding NULL checks for dev and name allocations, replacing
> > while (i--) with for (j = i; j >= 0; j--) to include the failing
> > index, tracking successful edac_mc_add_mc() calls with a bool array,
> > and switching to snprintf().
> >
> > Signed-off-by: Yufan Chen <ericterminal@gmail.com>
> > Reviewed-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
> > ---
> > v2: Correct Signed-off-by name and add Reviewed-by tag. Fix commit message
> formatting.
> >
> > drivers/edac/versalnet_edac.c | 29 ++++++++++++++++++++++++-----
> > 1 file changed, 24 insertions(+), 5 deletions(-)
>
> No, thanks, we'll do the proper cleanup here:
>
> https://lore.kernel.org/all/20251104093932.3838876-1-
> shubhrajyoti.datta@amd.com/T/#u
My apologies somehow I had missed it.
Tested it on the hardware.
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-02-26 16:52 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-21 7:16 [PATCH] EDAC/versalnet: Fix resource leaks and NULL derefs in init_versalnet() Eric-Terminal
2026-02-22 2:44 ` Borislav Petkov
[not found] ` <CAKNPVZA36XvT7R+QWTOTvAeQNA-sZfnG-h5jV3FnKvKOxqiGtA@mail.gmail.com>
[not found] ` <FE3C5380-E82B-4EC8-B1C0-16026CEEF8A2@alien8.de>
2026-02-22 10:39 ` Eric-Terminal
2026-02-23 13:47 ` Borislav Petkov
2026-02-23 14:58 ` Eric_Terminal
2026-02-26 6:54 ` Datta, Shubhrajyoti
2026-02-26 11:29 ` [PATCH v2] " Eric-Terminal
2026-02-26 12:59 ` Borislav Petkov
2026-02-26 16:52 ` Datta, Shubhrajyoti
2026-02-22 10:48 ` [PATCH] " Eric_Terminal
2026-02-23 8:54 ` Borislav Petkov
2026-02-23 10:08 ` Eric_Terminal
2026-02-23 10:40 ` Borislav Petkov
2026-02-23 10:52 ` Eric_Terminal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox