| Message ID | 20221128065512.3572550-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] EDAC/i10nm: fix refcount leak in pci_get_dev_wrapper() | expand |
> From: Yang Yingliang <yangyingliang@huawei.com> > Sent: Monday, November 28, 2022 2:55 PM > To: linux-edac@vger.kernel.org > Cc: Luck, Tony <tony.luck@intel.com>; bp@alien8.de; mchehab@kernel.org; > james.morse@arm.com; rric@kernel.org; Zhuo, Qiuxu > <qiuxu.zhuo@intel.com>; yangyingliang@huawei.com > Subject: [PATCH v2] EDAC/i10nm: fix refcount leak in pci_get_dev_wrapper() > > As the comment of pci_get_domain_bus_and_slot() says, it returns a PCI > device with refcount incremented, so it doesn't need to call an extra > pci_dev_get() in pci_get_dev_wrapper(), and the PCI device needs to be put > in the error path. > > Fixes: d4dc89d069aa ("EDAC, i10nm: Add a driver for Intel 10nm server > processors") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > v1 -> v2: > Fix some grammar errors in the commit message. > --- > drivers/edac/i10nm_base.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/edac/i10nm_base.c b/drivers/edac/i10nm_base.c index > a22ea053f8e1..8af4d2523194 100644 > --- a/drivers/edac/i10nm_base.c > +++ b/drivers/edac/i10nm_base.c > @@ -304,11 +304,10 @@ static struct pci_dev *pci_get_dev_wrapper(int > dom, unsigned int bus, > if (unlikely(pci_enable_device(pdev) < 0)) { > edac_dbg(2, "Failed to enable device %02x:%02x.%x\n", > bus, dev, fun); > + pci_dev_put(pdev); > return NULL; > } > > - pci_dev_get(pdev); > - > return pdev; > } > > -- > 2.25.1 Reviewed-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> As the comment of pci_get_domain_bus_and_slot() says, it returns > a PCI device with refcount incremented, so it doesn't need to > call an extra pci_dev_get() in pci_get_dev_wrapper(), and the PCI > device needs to be put in the error path. > > Fixes: d4dc89d069aa ("EDAC, i10nm: Add a driver for Intel 10nm server processors") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> Applied. Thanks. While this fixes this particular leak, I think we have a bigger one in many edac drivers on module unload. I don't see any "put" in the module exit path. -Tony
diff --git a/drivers/edac/i10nm_base.c b/drivers/edac/i10nm_base.c index a22ea053f8e1..8af4d2523194 100644 --- a/drivers/edac/i10nm_base.c +++ b/drivers/edac/i10nm_base.c @@ -304,11 +304,10 @@ static struct pci_dev *pci_get_dev_wrapper(int dom, unsigned int bus, if (unlikely(pci_enable_device(pdev) < 0)) { edac_dbg(2, "Failed to enable device %02x:%02x.%x\n", bus, dev, fun); + pci_dev_put(pdev); return NULL; } - pci_dev_get(pdev); - return pdev; }
As the comment of pci_get_domain_bus_and_slot() says, it returns a PCI device with refcount incremented, so it doesn't need to call an extra pci_dev_get() in pci_get_dev_wrapper(), and the PCI device needs to be put in the error path. Fixes: d4dc89d069aa ("EDAC, i10nm: Add a driver for Intel 10nm server processors") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- v1 -> v2: Fix some grammar errors in the commit message. --- drivers/edac/i10nm_base.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)