diff mbox

[RFC] pci: crash on pci_unregister_driver after pci_register_driver fails

Message ID 20130903153815.13638272@nehalam.linuxnetplumber.net
State Not Applicable
Headers show

Commit Message

Stephen Hemminger Sept. 3, 2013, 10:38 p.m. UTC
While debugging another problem with a PCI driver, I noticed that if
device probe routine returns an error, the kernel will crash when module
is unloaded. It looks like pci_register_driver() sets drv->bus to be PCI
then in the module unload.

module_unload
   my_device_exit_module
      pci_unregister_driver
         bus_remove_driver
           OOPS

One way to fix this would be to have pci_register_driver clear the bus
flag (it has no reference) if an error was detected.

--
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

Comments

Greg KH Sept. 3, 2013, 11:06 p.m. UTC | #1
On Tue, Sep 03, 2013 at 03:38:15PM -0700, Stephen Hemminger wrote:
> While debugging another problem with a PCI driver, I noticed that if
> device probe routine returns an error, the kernel will crash when module
> is unloaded. It looks like pci_register_driver() sets drv->bus to be PCI
> then in the module unload.
> 
> module_unload
>    my_device_exit_module
>       pci_unregister_driver
>          bus_remove_driver
>            OOPS
> 
> One way to fix this would be to have pci_register_driver clear the bus
> flag (it has no reference) if an error was detected.

Odd, no other buses do this, why would it matter for PCI?  Maybe they
just never fail their registering calls.

How about putting this in the driver core instead so that all busses get
fixed?

thanks,

greg k-h
--
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
Stephen Hemminger Sept. 4, 2013, 4:13 p.m. UTC | #2
On Tue, 3 Sep 2013 16:06:19 -0700
Greg KH <gregkh@linux-foundation.org> wrote:

> On Tue, Sep 03, 2013 at 03:38:15PM -0700, Stephen Hemminger wrote:
> > While debugging another problem with a PCI driver, I noticed that if
> > device probe routine returns an error, the kernel will crash when module
> > is unloaded. It looks like pci_register_driver() sets drv->bus to be PCI
> > then in the module unload.
> > 
> > module_unload
> >    my_device_exit_module
> >       pci_unregister_driver
> >          bus_remove_driver
> >            OOPS
> > 
> > One way to fix this would be to have pci_register_driver clear the bus
> > flag (it has no reference) if an error was detected.
> 
> Odd, no other buses do this, why would it matter for PCI?  Maybe they
> just never fail their registering calls.
> 
> How about putting this in the driver core instead so that all busses get
> fixed?
> 
> thanks,
> 
> greg k-h

Never mind, buggy driver
--
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 mbox

Patch

--- a/drivers/pci/pci-driver.c	2013-07-08 10:54:52.968241742 -0700
+++ b/drivers/pci/pci-driver.c	2013-09-03 15:34:10.112477893 -0700
@@ -1120,6 +1120,8 @@  const struct dev_pm_ops pci_dev_pm_ops =
 int __pci_register_driver(struct pci_driver *drv, struct module *owner,
 			  const char *mod_name)
 {
+	int err;
+
 	/* initialize common driver fields */
 	drv->driver.name = drv->name;
 	drv->driver.bus = &pci_bus_type;
@@ -1130,7 +1132,11 @@  int __pci_register_driver(struct pci_dri
 	INIT_LIST_HEAD(&drv->dynids.list);
 
 	/* register with core */
-	return driver_register(&drv->driver);
+	err = driver_register(&drv->driver);
+	if (err)
+		drv->driver.bus = NULL;
+
+	return err;
 }
 
 /**