USB: make single lock for all usb dynamic id lists

There are a number of places where we accidentally pass in a constant
structure to later cast it off to a dynamic one, and then attempt to
grab a lock on it, which is not a good idea.  To help resolve this, move
the dynamic id lock out of the dynamic id structure for the driver and
into one single lock for all USB dynamic ids.  As this lock should never
have any real contention (it's only every accessed when a device is
added or removed, which is always serialized) there should not be any
difference except for some memory savings.

Note, this just converts the existing use of the dynamic id lock to the
new static lock, there is one place that is accessing the dynamic id
list without grabbing the lock, that will be fixed up in a follow-on
change.

Cc: Johan Hovold <johan@kernel.org>
Cc: Herve Codina <herve.codina@bootlin.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Grant Grundler <grundler@chromium.org>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Yajun Deng <yajun.deng@linux.dev>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: linux-usb@vger.kernel.org
Link: https://lore.kernel.org/r/2024111322-kindly-finalist-d247@gregkh
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index b7bea1015d7c1dfe1f98822c5236df8d80f35dd2..871cf199b6bf17b9377867ff93a2d868e81479ee 100644 (file)
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -415,6 +415,9 @@ EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
 struct dentry *usb_debug_root;
 EXPORT_SYMBOL_GPL(usb_debug_root);
 
+DEFINE_MUTEX(usb_dynids_lock);
+EXPORT_SYMBOL_GPL(usb_dynids_lock);
+
 static int __init usb_common_init(void)
 {
        usb_debug_root = debugfs_create_dir("usb", NULL);

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 0c3f12daac79e0038b991cbd91e9afc40baf3aac..bc3c00580238d7a682d592e5e0d6391644599d9f 100644 (file)
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -95,9 +95,9 @@ ssize_t usb_store_new_id(struct usb_dynids *dynids,
                }
        }
 
-       spin_lock(&dynids->lock);
+       mutex_lock(&usb_dynids_lock);
        list_add_tail(&dynid->node, &dynids->list);
-       spin_unlock(&dynids->lock);
+       mutex_unlock(&usb_dynids_lock);
 
        retval = driver_attach(driver);
 
@@ -160,7 +160,7 @@ static ssize_t remove_id_store(struct device_driver *driver, const char *buf,
        if (fields < 2)
                return -EINVAL;
 
-       spin_lock(&usb_driver->dynids.lock);
+       guard(mutex)(&usb_dynids_lock);
        list_for_each_entry_safe(dynid, n, &usb_driver->dynids.list, node) {
                struct usb_device_id *id = &dynid->id;
 
@@ -171,7 +171,6 @@ static ssize_t remove_id_store(struct device_driver *driver, const char *buf,
                        break;
                }
        }
-       spin_unlock(&usb_driver->dynids.lock);
        return count;
 }
 
@@ -220,12 +219,11 @@ static void usb_free_dynids(struct usb_driver *usb_drv)
 {
        struct usb_dynid *dynid, *n;
 
-       spin_lock(&usb_drv->dynids.lock);
+       guard(mutex)(&usb_dynids_lock);
        list_for_each_entry_safe(dynid, n, &usb_drv->dynids.list, node) {
                list_del(&dynid->node);
                kfree(dynid);
        }
-       spin_unlock(&usb_drv->dynids.lock);
 }
 
 static const struct usb_device_id *usb_match_dynamic_id(struct usb_interface *intf,
@@ -233,14 +231,12 @@ static const struct usb_device_id *usb_match_dynamic_id(struct usb_interface *in
 {
        struct usb_dynid *dynid;
 
-       spin_lock(&drv->dynids.lock);
+       guard(mutex)(&usb_dynids_lock);
        list_for_each_entry(dynid, &drv->dynids.list, node) {
                if (usb_match_one_id(intf, &dynid->id)) {
-                       spin_unlock(&drv->dynids.lock);
                        return &dynid->id;
                }
        }
-       spin_unlock(&drv->dynids.lock);
        return NULL;
 }
 
@@ -1076,7 +1072,6 @@ int usb_register_driver(struct usb_driver *new_driver, struct module *owner,
        new_driver->driver.owner = owner;
        new_driver->driver.mod_name = mod_name;
        new_driver->driver.dev_groups = new_driver->dev_groups;
-       spin_lock_init(&new_driver->dynids.lock);
        INIT_LIST_HEAD(&new_driver->dynids.list);
 
        retval = driver_register(&new_driver->driver);

diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c
index d200e2c29a8ffe84acda12537fc5d688627f37b3..2fea1b1db4a269c0929a0e8e2effebd9bad9d34e 100644 (file)
--- a/drivers/usb/serial/bus.c
+++ b/drivers/usb/serial/bus.c
@@ -136,12 +136,11 @@ static void free_dynids(struct usb_serial_driver *drv)
 {
        struct usb_dynid *dynid, *n;
 
-       spin_lock(&drv->dynids.lock);
+       guard(mutex)(&usb_dynids_lock);
        list_for_each_entry_safe(dynid, n, &drv->dynids.list, node) {
                list_del(&dynid->node);
                kfree(dynid);
        }
-       spin_unlock(&drv->dynids.lock);
 }
 
 const struct bus_type usb_serial_bus_type = {
@@ -157,7 +156,6 @@ int usb_serial_bus_register(struct usb_serial_driver *driver)
        int retval;
 
        driver->driver.bus = &usb_serial_bus_type;
-       spin_lock_init(&driver->dynids.lock);
        INIT_LIST_HEAD(&driver->dynids.list);
 
        retval = driver_register(&driver->driver);

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index df6a2ae0bf4246fd93b7becb328ece50e80c181a..7266558d823ac2c5e6c00417aef6ab90bd2cb4dd 100644 (file)
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -706,14 +706,12 @@ static const struct usb_device_id *match_dynamic_id(struct usb_interface *intf,
 {
        struct usb_dynid *dynid;
 
-       spin_lock(&drv->dynids.lock);
+       guard(mutex)(&usb_dynids_lock);
        list_for_each_entry(dynid, &drv->dynids.list, node) {
                if (usb_match_one_id(intf, &dynid->id)) {
-                       spin_unlock(&drv->dynids.lock);
                        return &dynid->id;
                }
        }
-       spin_unlock(&drv->dynids.lock);
        return NULL;
 }
 

diff --git a/include/linux/usb.h b/include/linux/usb.h
index 672d8fc2abdb02149c6a2bcbf4eb090075cf839f..b66b1af3e439f5efeac78ea1d0d2cfddfb5e6f09 100644 (file)
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1129,8 +1129,8 @@ static inline int usb_make_path(struct usb_device *dev, char *buf, size_t size)
 /* ----------------------------------------------------------------------- */
 
 /* Stuff for dynamic usb ids */
+extern struct mutex usb_dynids_lock;
 struct usb_dynids {
-       spinlock_t lock;
        struct list_head list;
 };