| Message ID | 1389741079-12976-1-git-send-email-tixxdz@opendz.org |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Jan 15, 2014 at 12:11:19AM +0100, Djalal Harouni wrote: > On error paths make sure that the pci_host_bridge struct is freed. > > Signed-off-by: Djalal Harouni <tixxdz@opendz.org> Applied to pci/misc, thanks! > --- > On top of linux-next. > > drivers/pci/probe.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 23cdfac..483566c 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -1771,15 +1771,13 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, > bridge->dev.release = pci_release_host_bridge_dev; > dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus); > error = pcibios_root_bridge_prepare(bridge); > - if (error) { > - kfree(bridge); > - goto err_out; > - } > + if (error) > + goto host_bridge_err; > > error = device_register(&bridge->dev); > if (error) { > put_device(&bridge->dev); > - goto err_out; > + goto host_bridge_err; > } > b->bridge = get_device(&bridge->dev); > device_enable_async_suspend(b->bridge); > @@ -1836,6 +1834,8 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, > class_dev_reg_err: > put_device(&bridge->dev); > device_unregister(&bridge->dev); > +host_bridge_err: > + kfree(bridge); > err_out: > kfree(b); > return NULL; > -- > 1.7.11.7 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 29, 2014 at 3:12 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Wed, Jan 15, 2014 at 12:11:19AM +0100, Djalal Harouni wrote: >> On error paths make sure that the pci_host_bridge struct is freed. >> >> Signed-off-by: Djalal Harouni <tixxdz@opendz.org> > > Applied to pci/misc, thanks! > >> --- >> On top of linux-next. >> >> drivers/pci/probe.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> index 23cdfac..483566c 100644 >> --- a/drivers/pci/probe.c >> +++ b/drivers/pci/probe.c >> @@ -1771,15 +1771,13 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, >> bridge->dev.release = pci_release_host_bridge_dev; >> dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus); >> error = pcibios_root_bridge_prepare(bridge); >> - if (error) { >> - kfree(bridge); >> - goto err_out; >> - } >> + if (error) >> + goto host_bridge_err; >> >> error = device_register(&bridge->dev); >> if (error) { >> put_device(&bridge->dev); >> - goto err_out; >> + goto host_bridge_err; >> } >> b->bridge = get_device(&bridge->dev); >> device_enable_async_suspend(b->bridge); >> @@ -1836,6 +1834,8 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, >> class_dev_reg_err: >> put_device(&bridge->dev); >> device_unregister(&bridge->dev); >> +host_bridge_err: >> + kfree(bridge); >> err_out: >> kfree(b); >> return NULL; >> -- Are you joking? NAK. even device_register fail,it will have one ref hold, and put_device(&bridge->dev) it will trigger bridge->dev.release aka pci_release_host_bridge_dev; that function will free the bridge. You will have double free. Please check commit log for sure. commit 343df771e671d821478dd3ef525a0610b808dbf8 Author: Jiang Liu <liuj97@gmail.com> Date: Fri Jun 7 01:10:08 2013 +0800 PCI: Fix refcount issue in pci_create_root_bus() error recovery path After calling device_register(&bridge->dev), the bridge is reference- counted, and it is illegal to call kfree() on it except in the release function. [bhelgaas: changelog, use put_device() after device_register() failure] Signed-off-by: Jiang Liu <jiang.liu@huawei.com> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Cc: stable@vger.kernel.org -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 29, 2014 at 5:23 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Wed, Jan 29, 2014 at 3:12 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> On Wed, Jan 15, 2014 at 12:11:19AM +0100, Djalal Harouni wrote: >>> On error paths make sure that the pci_host_bridge struct is freed. >>> >>> Signed-off-by: Djalal Harouni <tixxdz@opendz.org> >> >> Applied to pci/misc, thanks! >> >>> --- >>> On top of linux-next. >>> >>> drivers/pci/probe.c | 10 +++++----- >>> 1 file changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >>> index 23cdfac..483566c 100644 >>> --- a/drivers/pci/probe.c >>> +++ b/drivers/pci/probe.c >>> @@ -1771,15 +1771,13 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, >>> bridge->dev.release = pci_release_host_bridge_dev; >>> dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus); >>> error = pcibios_root_bridge_prepare(bridge); >>> - if (error) { >>> - kfree(bridge); >>> - goto err_out; >>> - } >>> + if (error) >>> + goto host_bridge_err; >>> >>> error = device_register(&bridge->dev); >>> if (error) { >>> put_device(&bridge->dev); >>> - goto err_out; >>> + goto host_bridge_err; >>> } >>> b->bridge = get_device(&bridge->dev); >>> device_enable_async_suspend(b->bridge); >>> @@ -1836,6 +1834,8 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, >>> class_dev_reg_err: >>> put_device(&bridge->dev); >>> device_unregister(&bridge->dev); >>> +host_bridge_err: >>> + kfree(bridge); >>> err_out: >>> kfree(b); >>> return NULL; >>> -- > > Are you joking? > > NAK. > > even device_register fail,it will have one ref hold, and > put_device(&bridge->dev) > it will trigger bridge->dev.release aka pci_release_host_bridge_dev; > that function will free the bridge. > You will have double free. OK, dropped, thanks. I wasn't joking, I just made a mistake. > commit 343df771e671d821478dd3ef525a0610b808dbf8 > Author: Jiang Liu <liuj97@gmail.com> > Date: Fri Jun 7 01:10:08 2013 +0800 > > PCI: Fix refcount issue in pci_create_root_bus() error recovery path > > After calling device_register(&bridge->dev), the bridge is reference- > counted, and it is illegal to call kfree() on it except in the release > function. > > [bhelgaas: changelog, use put_device() after device_register() failure] > Signed-off-by: Jiang Liu <jiang.liu@huawei.com> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > Cc: stable@vger.kernel.org -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 23cdfac..483566c 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1771,15 +1771,13 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, bridge->dev.release = pci_release_host_bridge_dev; dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus); error = pcibios_root_bridge_prepare(bridge); - if (error) { - kfree(bridge); - goto err_out; - } + if (error) + goto host_bridge_err; error = device_register(&bridge->dev); if (error) { put_device(&bridge->dev); - goto err_out; + goto host_bridge_err; } b->bridge = get_device(&bridge->dev); device_enable_async_suspend(b->bridge); @@ -1836,6 +1834,8 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, class_dev_reg_err: put_device(&bridge->dev); device_unregister(&bridge->dev); +host_bridge_err: + kfree(bridge); err_out: kfree(b); return NULL;
On error paths make sure that the pci_host_bridge struct is freed. Signed-off-by: Djalal Harouni <tixxdz@opendz.org> --- On top of linux-next. drivers/pci/probe.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)