Messages in this thread | | | Subject | Re: [PATCH -next 2/3] PCI: fix possible memory leak in error case in pci_register_host_bridge() | From | Yang Yingliang <> | Date | Sat, 27 Aug 2022 17:29:35 +0800 |
| |
On 2022/8/27 6:38, Bjorn Helgaas wrote: > [+cc Arnd, Rob] > > On Thu, Aug 25, 2022 at 08:27:52PM +0800, Yang Yingliang wrote: >> If device_register() fails in pci_register_host_bridge(), the refcount >> of bus device is leaked, so device name that set by dev_set_name() can >> not be freed. Fix this by calling put_device() when device_register() >> fails, so the device name will be freed in kobject_cleanup(). >> >> Fixes: 37d6a0a6f470 ("PCI: Add pci_register_host_bridge() interface") >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >> --- >> drivers/pci/probe.c | 17 +++++++++++------ >> 1 file changed, 11 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> index e500eb9d6468..292d9da146ce 100644 >> --- a/drivers/pci/probe.c >> +++ b/drivers/pci/probe.c >> @@ -948,8 +948,17 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) >> name = dev_name(&bus->dev); >> >> err = device_register(&bus->dev); >> - if (err) >> - goto unregister; >> + if (err) { >> + /* >> + * release_pcibus_dev() will decrease the refcount of bridge >> + * device and free the memory of bus. >> + * The memory of bus device name will be freed when the refcount >> + * get to zero. >> + */ >> + put_device(&bus->dev); >> + device_unregister(&bridge->dev); >> + return err; >> + } > Calling put_device(X) after device_register(X) returns failure doesn't > need explanation because that's the standard pattern. I think that > was just missing before. > > In this error case, we previously did called put_device() for the > *bridge* instead of the bus. That was likely a typo and seems like > the important thing here. put_device() for the bridge will be called in the callback of put for the bus. So it doesn't call put bridge device here.
Thanks, Yang >> pcibios_add_bus(bus); >> >> @@ -1025,10 +1034,6 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) >> >> return 0; >> >> -unregister: >> - put_device(&bridge->dev); >> - device_unregister(&bridge->dev); >> - >> free: >> kfree(bus); >> return err; >> -- >> 2.25.1 >> > .
| |