Re: [PATCH v3] edac: versalnet: Use kasprintf() to simplify string allocation and fix error paths.

On Wed, Nov 19, 2025 at 12:16 AM Ayaan Mirza Baig
<[email protected]> wrote:
>
> On Tue, Nov 18, 2025 at 02:02:01PM +0000, Zhuo, Qiuxu wrote:
> > > From: Ayaan Mirza Baig <[email protected]>
> > > Sent: Monday, November 17, 2025 7:02 PM
> > > To: Zhuo, Qiuxu <[email protected]>
> > > Cc: [email protected]; [email protected]; linux-
> > > [email protected]; [email protected]
> > > Subject: [PATCH v3] edac: versalnet: Use kasprintf() to simplify string
> > > allocation and fix error paths.
> > >
> > > Replace the kmalloc() + sprintf() pattern with a single call to kasprintf(). This is
> > > cleaner, simpler, and avoids potential buffer overflows from the fixed-size 32-
> > > byte allocation.
> > > Handle possible NULL return from kasprintf() on allocation failure and ensure
> > > proper cleanup on error paths.
> > >
> > > Also free dev->init_name in the device release function to avoid leak on
> > > normal removal.
> > >
> > > Signed-off-by: Ayaan Mirza Baig <[email protected]>
> > >
> > > v2:
> > >  - Add NULL check for kasprintf() as requested by reviewer.
> > >
> > > v3:
> > >  - Free dev->init_name in versal_edac_release() to fix the existing leak.
> > > ---
> > >  drivers/edac/versalnet_edac.c | 15 ++++++++++++---
> > >  1 file changed, 12 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
> > > index 1ded4c3f0213..360d4f83ed89 100644
> > > --- a/drivers/edac/versalnet_edac.c
> > > +++ b/drivers/edac/versalnet_edac.c
> > > @@ -15,6 +15,7 @@
> > >  #include <ras/ras_event.h>
> > >
> > >  #include "edac_module.h"
> > > +#include "../../include/linux/device.h"
> > >
> > >  /* Granularity of reported error in bytes */
> > >  #define MC5_ERR_GRAIN                      1
> > > @@ -755,6 +756,7 @@ static struct rpmsg_driver amd_rpmsg_driver = {
> > >
> > >  static void versal_edac_release(struct device *dev)  {
> > > +   kfree(dev->init_name);
> > >     kfree(dev);
> > >  }
> > >
> > > @@ -812,12 +814,19 @@ static int init_versalnet(struct mc_priv *priv, struct
> > > platform_device *pdev)
> > >
> > >             dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> > >             dev->release = versal_edac_release;
> > > -           name = kmalloc(32, GFP_KERNEL);
> > > -           sprintf(name, "versal-net-ddrmc5-edac-%d", i);
> > > +           name = kasprintf(GFP_KERNEL, "versal-net-ddrmc5-edac-%d",
> > > i);
> > > +           if (!name) {
> > > +                   kfree(dev);
> > > +                   return -ENOMEM;
> >
> > On this failure, I think it should "goto err_alloc;" to free the allocated mci instances
> > instead of directly returning -ENOMEM.
>
> Okay, thanks. I did overlook this, my bad. I apologize for these mistakes I keep
> making again and again. I'll make sure there are no more errors in v4.

Did you get a chance of working on the v4 .