Message ID | 20200503142703.14903-1-peng.fan@nxp.com |
---|---|
State | New |
Headers | show |
Series | [1/4] sata: ahsata: Fix resource leak | expand |
On Sun, 3 May 2020 at 08:04, Peng Fan <peng.fan at nxp.com> wrote: > > From: Ye Li <ye.li at nxp.com> > > Fix coverity issue CID 3606684: Resource leak (RESOURCE_LEAK) > leaked_storage: Variable uc_priv going out of scope leaks the storage it points to > > Signed-off-by: Ye Li <ye.li at nxp.com> > Signed-off-by: Peng Fan <peng.fan at nxp.com> > --- > drivers/ata/dwc_ahsata.c | 5 +++++ > 1 file changed, 5 insertions(+) Reviewed-by: Simon Glass <sjg at chromium.org> > > diff --git a/drivers/ata/dwc_ahsata.c b/drivers/ata/dwc_ahsata.c > index c2e28fe518..a775214792 100644 > --- a/drivers/ata/dwc_ahsata.c > +++ b/drivers/ata/dwc_ahsata.c > @@ -847,6 +847,9 @@ static int ahci_init_one(int pdev) > struct ahci_uc_priv *uc_priv = NULL; > > uc_priv = malloc(sizeof(struct ahci_uc_priv)); > + if (!uc_priv) > + return -ENOMEM; > + > memset(uc_priv, 0, sizeof(struct ahci_uc_priv)); > uc_priv->dev = pdev; > > @@ -871,6 +874,8 @@ static int ahci_init_one(int pdev) > return 0; > > err_out: > + if (uc_priv) > + free(uc_priv); Seems like you can avoid the if() since it is always allocated at this point? The migration date for SATA was over 6 months ago so really this code should not be used at this point. Regards, Simon
Hi Simon, > Subject: Re: [PATCH 1/4] sata: ahsata: Fix resource leak > > On Sun, 3 May 2020 at 08:04, Peng Fan <peng.fan at nxp.com> wrote: > > > > From: Ye Li <ye.li at nxp.com> > > > > Fix coverity issue CID 3606684: Resource leak (RESOURCE_LEAK) > > leaked_storage: Variable uc_priv going out of scope leaks the storage > > it points to > > > > Signed-off-by: Ye Li <ye.li at nxp.com> > > Signed-off-by: Peng Fan <peng.fan at nxp.com> > > --- > > drivers/ata/dwc_ahsata.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > Reviewed-by: Simon Glass <sjg at chromium.org> > > > > > diff --git a/drivers/ata/dwc_ahsata.c b/drivers/ata/dwc_ahsata.c index > > c2e28fe518..a775214792 100644 > > --- a/drivers/ata/dwc_ahsata.c > > +++ b/drivers/ata/dwc_ahsata.c > > @@ -847,6 +847,9 @@ static int ahci_init_one(int pdev) > > struct ahci_uc_priv *uc_priv = NULL; > > > > uc_priv = malloc(sizeof(struct ahci_uc_priv)); > > + if (!uc_priv) > > + return -ENOMEM; > > + > > memset(uc_priv, 0, sizeof(struct ahci_uc_priv)); > > uc_priv->dev = pdev; > > > > @@ -871,6 +874,8 @@ static int ahci_init_one(int pdev) > > return 0; > > > > err_out: > > + if (uc_priv) > > + free(uc_priv); > > Seems like you can avoid the if() since it is always allocated at this point? > > The migration date for SATA was over 6 months ago so really this code should > not be used at this point. This is to upstream nxp downstream patch. Since this is bug fix, I hope it is ok to be in until the driver is removed. I'll also try to convert to use DM sata. Thanks, Peng. > > Regards, > Simon
On Sun, May 03, 2020 at 10:27:00PM +0800, Peng Fan wrote: > From: Ye Li <ye.li at nxp.com> > > Fix coverity issue CID 3606684: Resource leak (RESOURCE_LEAK) > leaked_storage: Variable uc_priv going out of scope leaks the storage it points to > > Signed-off-by: Ye Li <ye.li at nxp.com> > Signed-off-by: Peng Fan <peng.fan at nxp.com> > Reviewed-by: Simon Glass <sjg at chromium.org> Applied to u-boot/master, thanks!
diff --git a/drivers/ata/dwc_ahsata.c b/drivers/ata/dwc_ahsata.c index c2e28fe518..a775214792 100644 --- a/drivers/ata/dwc_ahsata.c +++ b/drivers/ata/dwc_ahsata.c @@ -847,6 +847,9 @@ static int ahci_init_one(int pdev) struct ahci_uc_priv *uc_priv = NULL; uc_priv = malloc(sizeof(struct ahci_uc_priv)); + if (!uc_priv) + return -ENOMEM; + memset(uc_priv, 0, sizeof(struct ahci_uc_priv)); uc_priv->dev = pdev; @@ -871,6 +874,8 @@ static int ahci_init_one(int pdev) return 0; err_out: + if (uc_priv) + free(uc_priv); return rc; }