On Wed, 4 Jan 2012, Huajun Li wrote:
> Hi Alan,
> Is any chance for user to trigger the window if the driver has been
> unregistered? thanks.
Yes, that possibility does exist.
> How about adding more codes into usb_store_new_id() to find out
> whether the driver is registered while receiving such requests? and
> this could give useful info to user space if the requests fail.
I don't think that's the best approach. Furthermore, your proposed fix
still contains a race because is_drv_ready() calls put_driver() before
returning.
In order to fix this properly, we have to make sure that
usb_store_new_id() is mutually exclusive with driver registration and
unregistration. The patch below will accomplish that.
However it does leave one problem unsolved. The udev rule still can
run too quickly. Now instead of getting an oops, the rule simply won't
work. I don't know how to fix that.
David, can you test this patch in place of the earlier one?
Alan Stern
Index: usb-3.2/drivers/usb/core/driver.c
===================================================================
--- usb-3.2.orig/drivers/usb/core/driver.c
+++ usb-3.2/drivers/usb/core/driver.c
@@ -31,6 +31,10 @@
#include "usb.h"
+/* Mutual exclusion for drvwrap->is_registered */
+DEFINE_MUTEX(usb_driver_reg_mutex);
+EXPORT_SYMBOL_GPL(usb_driver_reg_mutex);
+
#ifdef CONFIG_HOTPLUG
@@ -815,6 +819,10 @@ int usb_register_device_driver(struct us
usbcore_name, retval, new_udriver->name);
}
+ mutex_lock(&usb_driver_reg_mutex);
+ new_udriver->drvwrap.is_registered = 1;
+ mutex_unlock(&usb_driver_reg_mutex);
+
return retval;
}
EXPORT_SYMBOL_GPL(usb_register_device_driver);
@@ -831,6 +839,10 @@ void usb_deregister_device_driver(struct
pr_info("%s: deregistering device driver %s\n",
usbcore_name, udriver->name);
+ mutex_lock(&usb_driver_reg_mutex);
+ udriver->drvwrap.is_registered = 0;
+ mutex_unlock(&usb_driver_reg_mutex);
+
driver_unregister(&udriver->drvwrap.driver);
usbfs_update_special();
}
@@ -873,6 +885,10 @@ int usb_register_driver(struct usb_drive
if (retval)
goto out;
+ mutex_lock(&usb_driver_reg_mutex);
+ new_driver->drvwrap.is_registered = 1;
+ mutex_unlock(&usb_driver_reg_mutex);
+
usbfs_update_special();
retval = usb_create_newid_file(new_driver);
@@ -917,6 +933,10 @@ void usb_deregister(struct usb_driver *d
pr_info("%s: deregistering interface driver %s\n",
usbcore_name, driver->name);
+ mutex_lock(&usb_driver_reg_mutex);
+ driver->drvwrap.is_registered = 0;
+ mutex_unlock(&usb_driver_reg_mutex);
+
usb_remove_removeid_file(driver);
usb_remove_newid_file(driver);
usb_free_dynids(driver);
Index: usb-3.2/drivers/usb/serial/bus.c
===================================================================
--- usb-3.2.orig/drivers/usb/serial/bus.c
+++ usb-3.2/drivers/usb/serial/bus.c
@@ -119,13 +119,20 @@ static int usb_serial_device_remove(stru
static ssize_t store_new_id(struct device_driver *driver,
const char *buf, size_t count)
{
- struct usb_serial_driver *usb_drv = to_usb_serial_driver(driver);
- ssize_t retval = usb_store_new_id(&usb_drv->dynids, driver, buf, count);
-
- if (retval >= 0 && usb_drv->usb_driver != NULL)
- retval = usb_store_new_id(&usb_drv->usb_driver->dynids,
- &usb_drv->usb_driver->drvwrap.driver,
+ struct usb_serial_driver *ser_drv = to_usb_serial_driver(driver);
+ struct usb_driver *usb_drv = ser_drv->usb_driver;
+ ssize_t retval = usb_store_new_id(&ser_drv->dynids, driver, buf, count);
+
+ if (retval >= 0 && usb_drv != NULL) {
+ mutex_lock(&usb_driver_reg_mutex);
+ if (usb_drv->drvwrap.is_registered)
+ retval = usb_store_new_id(&usb_drv->dynids,
+ &usb_drv->drvwrap.driver,
buf, count);
+ else
+ retval = -EAGAIN;
+ mutex_unlock(&usb_driver_reg_mutex);
+ }
return retval;
}
@@ -178,7 +185,7 @@ int usb_serial_bus_register(struct usb_s
void usb_serial_bus_deregister(struct usb_serial_driver *driver)
{
- free_dynids(driver);
driver_unregister(&driver->driver);
+ free_dynids(driver);
}
Index: usb-3.2/include/linux/usb.h
===================================================================
--- usb-3.2.orig/include/linux/usb.h
+++ usb-3.2/include/linux/usb.h
@@ -792,14 +792,18 @@ extern ssize_t usb_store_new_id(struct u
struct device_driver *driver,
const char *buf, size_t count);
+extern struct mutex usb_driver_reg_mutex;
+
/**
* struct usbdrv_wrap - wrapper for driver-model structure
* @driver: The driver-model core driver structure.
* @for_devices: Non-zero for device drivers, 0 for interface drivers.
+ * @is_registered: The driver is registered (protected by usb_driver_reg_mutex)
*/
struct usbdrv_wrap {
struct device_driver driver;
- int for_devices;
+ unsigned int for_devices:1;
+ unsigned int is_registered:1;
};
/**
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
