Re: [PATCH v2] ata: libata: Fix memory leak for error path in ata_host_alloc()

[Zheng Qixing]

在 2024/8/26 12:02, Damien Le Moal 写道:
> On 8/26/24 11:49 AM, Zheng Qixing wrote:
> > 在 2024/8/23 9:10, Damien Le Moal 写道:
> > > On 8/22/24 12:30 PM, Zheng Qixing wrote:
> > > > From: Zheng Qixing <zhengqixing@xxxxxxxxxx>
> > > >
> > > > In ata_host_alloc(), if ata_port_alloc(host) fails to allocate memory
> > > > for a port, the allocated 'host' structure is not freed before returning
> > > > from the function. This results in a potential memory leak.
> > > >
> > > > This patch adds a kfree(host) before the error handling code is executed
> > > > to ensure that the 'host' structure is properly freed in case of an
> > > > allocation failure.
> > > >
> > > > Signed-off-by: Zheng Qixing <zhengqixing@xxxxxxxxxx>
> > > This needs a Fixes tag. So I added:
> > >
> > > Fixes: 2623c7a5f279 ("libata: add refcounting to ata_host")
> > > Cc: stable@xxxxxxxxxxxxxxx>
> > >
> > > and applied to for-6.11-fixes. Thanks.
> >
> > Based on Niklas Cassel's suggestion, the commit message and the actual
> > content of the patch do not match.
> >
> > It should state "if devres_alloc(ata_devres_release, 0, GFP_KERNEL)
> > fails to allocate memory" instead of "if ata_port_alloc(host) fails to allocate
> > memory for a port".
> >
> > Should I modify the commit message and submit a new version of the patch?
> No need. I fixed it up. The commit message now is:
>
>      ata: libata: Fix memory leak for error path in ata_host_alloc()
>
>      In ata_host_alloc(), if devres_alloc() fails to allocate the device host
>      resource data pointer, the already allocated ata_host structure is not
>      freed before returning from the function. This results in a potential
>      memory leak.
>
>      Call kfree(host) before jumping to the error handling path to ensure
>      that the ata_host structure is properly freed if devres_alloc() fails.


Thanks.  :)


Zheng Qixing

> > Zheng Qixing
> >
> >
> > > > ---
> > > > Changes in v2:
> > > >    - error path is wrong in v1
> > > >
> > > >    drivers/ata/libata-core.c | 4 +++-
> > > >    1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> > > > index e4023fc288ac..f27a18990c38 100644
> > > > --- a/drivers/ata/libata-core.c
> > > > +++ b/drivers/ata/libata-core.c
> > > > @@ -5663,8 +5663,10 @@ struct ata_host *ata_host_alloc(struct device *dev,
> > > > int n_ports)
> > > >        }
> > > >          dr = devres_alloc(ata_devres_release, 0, GFP_KERNEL);
> > > > -    if (!dr)
> > > > +    if (!dr) {
> > > > +        kfree(host);
> > > >            goto err_out;
> > > > +    }
> > > >          devres_add(dev, dr);
> > > >        dev_set_drvdata(dev, host);