[PATCH 4/4] usb: fix UAF when probe runs concurrent to dyn ID removal

[Gary Guo]

Dynamic IDs are only guaranteed to be valid when usb_dynids_lock is held,
as remove_id_store can free the node. Thus, make a copy in
usb_probe_interface. Clarify the documentation that the id parameter is
only valid during the probe.

USB serial has the same pattern, but it does not need fixing as the IDs
cannot be removed via sysfs.

Fixes: 0c7a2b72746a ("USB: add remove_id sysfs attr for usb drivers")
Signed-off-by: Gary Guo <gary@xxxxxxxxxxx>
---
 drivers/usb/core/driver.c | 33 +++++++++++++++++----------------
 include/linux/usb.h       |  3 ++-
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index f63004417058..c26410cabdfe 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -227,18 +227,19 @@ static void usb_free_dynids(struct usb_driver *usb_drv)
 	}
 }
 
-static const struct usb_device_id *usb_match_dynamic_id(struct usb_interface *intf,
-							const struct usb_driver *drv)
+static bool usb_match_dynamic_id(struct usb_interface *intf, const struct usb_driver *drv,
+				 struct usb_device_id *id)
 {
 	struct usb_dynid *dynid;
 
 	guard(mutex)(&usb_dynids_lock);
 	list_for_each_entry(dynid, &drv->dynids.list, node) {
 		if (usb_match_one_id(intf, &dynid->id)) {
-			return &dynid->id;
+			*id = dynid->id;
+			return true;
 		}
 	}
-	return NULL;
+	return false;
 }
 
 
@@ -320,7 +321,8 @@ static int usb_probe_interface(struct device *dev)
 	struct usb_driver *driver = to_usb_driver(dev->driver);
 	struct usb_interface *intf = to_usb_interface(dev);
 	struct usb_device *udev = interface_to_usbdev(intf);
-	const struct usb_device_id *id;
+	struct usb_device_id id;
+	const struct usb_device_id *matched_id;
 	int error = -ENODEV;
 	int lpm_disable_error = -ENODEV;
 
@@ -340,11 +342,12 @@ static int usb_probe_interface(struct device *dev)
 		return error;
 	}
 
-	id = usb_match_dynamic_id(intf, driver);
-	if (!id)
-		id = usb_match_id(intf, driver->id_table);
-	if (!id)
-		return error;
+	if (!usb_match_dynamic_id(intf, driver, &id)) {
+		matched_id = usb_match_id(intf, driver->id_table);
+		if (!matched_id)
+			return error;
+		id = *matched_id;
+	}
 
 	dev_dbg(dev, "%s - got id\n", __func__);
 
@@ -393,7 +396,7 @@ static int usb_probe_interface(struct device *dev)
 		intf->needs_altsetting0 = 0;
 	}
 
-	error = driver->probe(intf, id);
+	error = driver->probe(intf, &id);
 	if (error)
 		goto err;
 
@@ -891,7 +894,7 @@ static int usb_device_match(struct device *dev, const struct device_driver *drv)
 	} else if (is_usb_interface(dev)) {
 		struct usb_interface *intf;
 		const struct usb_driver *usb_drv;
-		const struct usb_device_id *id;
+		struct usb_device_id id;
 
 		/* device drivers never match interfaces */
 		if (is_usb_device_driver(drv))
@@ -900,12 +903,10 @@ static int usb_device_match(struct device *dev, const struct device_driver *drv)
 		intf = to_usb_interface(dev);
 		usb_drv = to_usb_driver(drv);
 
-		id = usb_match_id(intf, usb_drv->id_table);
-		if (id)
+		if (usb_match_id(intf, usb_drv->id_table))
 			return 1;
 
-		id = usb_match_dynamic_id(intf, usb_drv);
-		if (id)
+		if (usb_match_dynamic_id(intf, usb_drv, &id))
 			return 1;
 	}
 
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 1da4ad1610bc..49ab8dbb885f 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1185,7 +1185,8 @@ extern ssize_t usb_show_dynids(struct usb_dynids *dynids, char *buf);
  *	interface.  It may also use usb_set_interface() to specify the
  *	appropriate altsetting.  If unwilling to manage the interface,
  *	return -ENODEV, if genuine IO errors occurred, an appropriate
- *	negative errno value.
+ *	negative errno value.  The usb_device_id parameter is only valid during
+ *	probe.
  * @disconnect: Called when the interface is no longer accessible, usually
  *	because its device has been (or is being) disconnected or the
  *	driver module is being unloaded.

-- 
2.54.0