| Message ID | 20250215054431.55747-6-xueshuai@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | dmaengine: idxd: fix memory leak in error handling path | expand |
Hi, Shuai, On 2/14/25 21:44, Shuai Xue wrote: > Memory allocated for idxd is not freed if an error occurs during > idxd_pci_probe(). To fix it, free the allocated memory in the reverse > order of allocation before exiting the function in case of an error. > > Fixes: bfe1d56091c1 ("dmaengine: idxd: Init and probe for Intel data accelerators") > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> > Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/dma/idxd/init.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c > index dc34830fe7c3..ac1cdc1d82bf 100644 > --- a/drivers/dma/idxd/init.c > +++ b/drivers/dma/idxd/init.c > @@ -550,6 +550,14 @@ static void idxd_read_caps(struct idxd_device *idxd) > idxd->hw.iaa_cap.bits = ioread64(idxd->reg_base + IDXD_IAACAP_OFFSET); > } > > +static void idxd_free(struct idxd_device *idxd) > +{ > + put_device(idxd_confdev(idxd)); > + bitmap_free(idxd->opcap_bmap); > + ida_free(&idxd_ida, idxd->id); > + kfree(idxd); opcap_bmap, idxd_ida, idxd are NOT allocated during FLR re-init idxd device. In the FLR case, they should not be freed. > +} > + > static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_data *data) > { > struct device *dev = &pdev->dev; > @@ -1219,7 +1227,7 @@ int idxd_pci_probe_alloc(struct idxd_device *idxd, struct pci_dev *pdev, > err: > pci_iounmap(pdev, idxd->reg_base); > err_iomap: > - put_device(idxd_confdev(idxd)); > + idxd_free(idxd); > err_idxd_alloc: > pci_disable_device(pdev); > return rc; Thanks. -Fenghua
在 2025/2/19 04:21, Fenghua Yu 写道: > Hi, Shuai, > > On 2/14/25 21:44, Shuai Xue wrote: >> Memory allocated for idxd is not freed if an error occurs during >> idxd_pci_probe(). To fix it, free the allocated memory in the reverse >> order of allocation before exiting the function in case of an error. >> >> Fixes: bfe1d56091c1 ("dmaengine: idxd: Init and probe for Intel data accelerators") >> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> >> Reviewed-by: Dave Jiang <dave.jiang@intel.com> >> --- >> drivers/dma/idxd/init.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c >> index dc34830fe7c3..ac1cdc1d82bf 100644 >> --- a/drivers/dma/idxd/init.c >> +++ b/drivers/dma/idxd/init.c >> @@ -550,6 +550,14 @@ static void idxd_read_caps(struct idxd_device *idxd) >> idxd->hw.iaa_cap.bits = ioread64(idxd->reg_base + IDXD_IAACAP_OFFSET); >> } >> +static void idxd_free(struct idxd_device *idxd) >> +{ >> + put_device(idxd_confdev(idxd)); >> + bitmap_free(idxd->opcap_bmap); >> + ida_free(&idxd_ida, idxd->id); >> + kfree(idxd); > > opcap_bmap, idxd_ida, idxd are NOT allocated during FLR re-init idxd device. In the FLR case, they should not be freed. Great catch, thanks. Will fix it in next version. Shuai
在 2025/2/19 04:21, Fenghua Yu 写道: > Hi, Shuai, > > On 2/14/25 21:44, Shuai Xue wrote: >> Memory allocated for idxd is not freed if an error occurs during >> idxd_pci_probe(). To fix it, free the allocated memory in the reverse >> order of allocation before exiting the function in case of an error. >> >> Fixes: bfe1d56091c1 ("dmaengine: idxd: Init and probe for Intel data accelerators") >> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> >> Reviewed-by: Dave Jiang <dave.jiang@intel.com> >> --- >> drivers/dma/idxd/init.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c >> index dc34830fe7c3..ac1cdc1d82bf 100644 >> --- a/drivers/dma/idxd/init.c >> +++ b/drivers/dma/idxd/init.c >> @@ -550,6 +550,14 @@ static void idxd_read_caps(struct idxd_device *idxd) >> idxd->hw.iaa_cap.bits = ioread64(idxd->reg_base + IDXD_IAACAP_OFFSET); >> } >> +static void idxd_free(struct idxd_device *idxd) >> +{ >> + put_device(idxd_confdev(idxd)); >> + bitmap_free(idxd->opcap_bmap); >> + ida_free(&idxd_ida, idxd->id); >> + kfree(idxd); > > opcap_bmap, idxd_ida, idxd are NOT allocated during FLR re-init idxd device. In the FLR case, they should not be freed. > After relook the code, idxd_free() will not be called in FLR case, because all error lable in idxd_pci_probe_alloc() is called from alloc_idxd=true, not the FLR case. Anyway, I will add a protection in idxd_free(). Thanks. Shuai
diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c index dc34830fe7c3..ac1cdc1d82bf 100644 --- a/drivers/dma/idxd/init.c +++ b/drivers/dma/idxd/init.c @@ -550,6 +550,14 @@ static void idxd_read_caps(struct idxd_device *idxd) idxd->hw.iaa_cap.bits = ioread64(idxd->reg_base + IDXD_IAACAP_OFFSET); } +static void idxd_free(struct idxd_device *idxd) +{ + put_device(idxd_confdev(idxd)); + bitmap_free(idxd->opcap_bmap); + ida_free(&idxd_ida, idxd->id); + kfree(idxd); +} + static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_data *data) { struct device *dev = &pdev->dev; @@ -1219,7 +1227,7 @@ int idxd_pci_probe_alloc(struct idxd_device *idxd, struct pci_dev *pdev, err: pci_iounmap(pdev, idxd->reg_base); err_iomap: - put_device(idxd_confdev(idxd)); + idxd_free(idxd); err_idxd_alloc: pci_disable_device(pdev); return rc;
Memory allocated for idxd is not freed if an error occurs during idxd_pci_probe(). To fix it, free the allocated memory in the reverse order of allocation before exiting the function in case of an error. Fixes: bfe1d56091c1 ("dmaengine: idxd: Init and probe for Intel data accelerators") Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> --- drivers/dma/idxd/init.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)