| Message ID | 20250202062357.872971-1-make24@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [v3] PCI: fix reference leak in pci_alloc_child_bus() | expand |
On Sun, 2 Feb 2025, Ma Ke wrote: > When device_register(&child->dev) failed, we should call put_device() > to explicitly release child->dev. > > As comment of device_register() says, 'NOTE: _Never_ directly free > @dev after calling this function, even if it returned an error! Always > use put_device() to give up the reference initialized in this function > instead.' > > Found by code review. > > Cc: stable@vger.kernel.org > Fixes: 4f535093cf8f ("PCI: Put pci_dev in device tree as early as possible") > Signed-off-by: Ma Ke <make24@iscas.ac.cn> > --- > Changes in v3: > - modified the description as suggestions. > Changes in v2: > - added the bug description about the comment of device_add(); > - fixed the patch as suggestions; > - added Cc and Fixes table. > --- > drivers/pci/probe.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 2e81ab0f5a25..51b78fcda4eb 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -1174,7 +1174,10 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, > add_dev: > pci_set_bus_msi_domain(child); > ret = device_register(&child->dev); > - WARN_ON(ret < 0); > + if (WARN_ON(ret < 0)) { > + put_device(&child->dev); > + return NULL; > + } > > pcibios_add_bus(child); Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Unrelated to this fix, IMO that WARN_ON() is overkill and I'm skeptical that printing a stack trace on a failure in device_register() is helpful. IMO, a simple error print would suffice to tell something (unexpectedly) went wrong here.
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 2e81ab0f5a25..51b78fcda4eb 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1174,7 +1174,10 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, add_dev: pci_set_bus_msi_domain(child); ret = device_register(&child->dev); - WARN_ON(ret < 0); + if (WARN_ON(ret < 0)) { + put_device(&child->dev); + return NULL; + } pcibios_add_bus(child);
When device_register(&child->dev) failed, we should call put_device() to explicitly release child->dev. As comment of device_register() says, 'NOTE: _Never_ directly free @dev after calling this function, even if it returned an error! Always use put_device() to give up the reference initialized in this function instead.' Found by code review. Cc: stable@vger.kernel.org Fixes: 4f535093cf8f ("PCI: Put pci_dev in device tree as early as possible") Signed-off-by: Ma Ke <make24@iscas.ac.cn> --- Changes in v3: - modified the description as suggestions. Changes in v2: - added the bug description about the comment of device_add(); - fixed the patch as suggestions; - added Cc and Fixes table. --- drivers/pci/probe.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)