[RFC 6/6] usb: check usb_hub_to_struct_hub() return value

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>

usb_hub_to_struct_hub() can return NULL in the unlikely cases a hub
without active configuration, or a hub without ports is found.

Even if unlikely we need to handle those cases properly.

[Note from Sarah: I'm not sure which stable kernels need parts of this
patch applied.  I think parts of it will need to be backported to
kernels as old as 2.6.32.

Commit 25118084ef03f4fc314ab33ef6a9d9271d0e616a "USB: check for hub
driver not bound to root hub device" (which was merged in 2.6.32)
changed hdev_to_hub() to return NULL if the driver wasn't bound to the
roothub.  Any commits after that commit that don't check for the NULL
pointer need to be fixed.

This is further complicated by the fact that commit
ad493e5e580546e6c3024b76a41535476da1546a "usb: add usb port auto power
off mechanism" renamed hdev_to_hub() to usb_hub_to_struct_hub(), and
thus this patch will not apply to kernels older than 3.9.

Also, commit 84ebc10294a3d7be4c66f51070b7aedbaa24de9b "USB: remove
CONFIG_USB_SUSPEND option" (which was merged in 3.10) removed one
instance of a potential NULL pointer dereference in the version of
usb_port_resume() that is used in older kernels when CONFIG_USB_SUSPEND
is turned off.

I will have to create separate patches for the older stable kernels.]

Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxx>
Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
 drivers/usb/core/hub.c |  107 +++++++++++++++++++++++++++++++++++------------
 1 files changed, 79 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index feef935..ff6f12b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -730,7 +730,12 @@ int usb_hub_set_port_power(struct usb_device *hdev, int port1,
 {
 	int ret;
 	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
-	struct usb_port *port_dev = hub->ports[port1 - 1];
+	struct usb_port *port_dev;
+
+	if (!hub)
+		return -EINVAL;
+
+	port_dev = hub->ports[port1 - 1];
 
 	if (set)
 		ret = set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
@@ -964,6 +969,10 @@ int usb_remove_device(struct usb_device *udev)
 	if (!udev->parent)	/* Can't remove a root hub */
 		return -EINVAL;
 	hub = usb_hub_to_struct_hub(udev->parent);
+
+	if (!hub)
+		return -EINVAL;
+
 	intf = to_usb_interface(hub->intfdev);
 
 	usb_autopm_get_interface(intf);
@@ -1733,6 +1742,9 @@ hub_ioctl(struct usb_interface *intf, unsigned int code, void *user_data)
 	struct usb_device *hdev = interface_to_usbdev (intf);
 	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
 
+	if (!hub)
+		return -EINVAL;
+
 	/* assert ifno == 0 (part of hub spec) */
 	switch (code) {
 	case USBDEVFS_HUB_PORTINFO: {
@@ -1769,15 +1781,17 @@ hub_ioctl(struct usb_interface *intf, unsigned int code, void *user_data)
 static int find_port_owner(struct usb_device *hdev, unsigned port1,
 		struct dev_state ***ppowner)
 {
+	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+
 	if (hdev->state == USB_STATE_NOTATTACHED)
 		return -ENODEV;
-	if (port1 == 0 || port1 > hdev->maxchild)
+	if (port1 == 0 || port1 > hdev->maxchild || !hub)
 		return -EINVAL;
 
 	/* This assumes that devices not managed by the hub driver
 	 * will always have maxchild equal to 0.
 	 */
-	*ppowner = &(usb_hub_to_struct_hub(hdev)->ports[port1 - 1]->port_owner);
+	*ppowner = &(hub->ports[port1 - 1]->port_owner);
 	return 0;
 }
 
@@ -1817,6 +1831,9 @@ void usb_hub_release_all_ports(struct usb_device *hdev, struct dev_state *owner)
 	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
 	int n;
 
+	if (!hub)
+		return;
+
 	for (n = 0; n < hdev->maxchild; n++) {
 		if (hub->ports[n]->port_owner == owner)
 			hub->ports[n]->port_owner = NULL;
@@ -1832,6 +1849,9 @@ bool usb_device_is_owned(struct usb_device *udev)
 	if (udev->state == USB_STATE_NOTATTACHED || !udev->parent)
 		return false;
 	hub = usb_hub_to_struct_hub(udev->parent);
+	if (!hub)
+		return false;
+
 	return !!hub->ports[udev->portnum - 1]->port_owner;
 }
 
@@ -1841,7 +1861,7 @@ static void recursively_mark_NOTATTACHED(struct usb_device *udev)
 	int i;
 
 	for (i = 0; i < udev->maxchild; ++i) {
-		if (hub->ports[i]->child)
+		if (hub && hub->ports[i]->child)
 			recursively_mark_NOTATTACHED(hub->ports[i]->child);
 	}
 	if (udev->state == USB_STATE_SUSPENDED)
@@ -2021,7 +2041,7 @@ void usb_disconnect(struct usb_device **pdev)
 
 	/* Free up all the children before we remove this device */
 	for (i = 0; i < udev->maxchild; i++) {
-		if (hub->ports[i]->child)
+		if (hub && hub->ports[i]->child)
 			usb_disconnect(&hub->ports[i]->child);
 	}
 
@@ -2035,15 +2055,19 @@ void usb_disconnect(struct usb_device **pdev)
 
 	if (udev->parent) {
 		struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
-		struct usb_port	*port_dev = hub->ports[udev->portnum - 1];
+		struct usb_port *port_dev;
 
-		sysfs_remove_link(&udev->dev.kobj, "port");
-		sysfs_remove_link(&port_dev->dev.kobj, "device");
+		if (hub) {
+			port_dev = hub->ports[udev->portnum - 1];
 
-		if (!port_dev->did_runtime_put)
-			pm_runtime_put(&port_dev->dev);
-		else
-			port_dev->did_runtime_put = false;
+			sysfs_remove_link(&udev->dev.kobj, "port");
+			sysfs_remove_link(&port_dev->dev.kobj, "device");
+
+			if (!port_dev->did_runtime_put)
+				pm_runtime_put(&port_dev->dev);
+			else
+				port_dev->did_runtime_put = false;
+		}
 	}
 
 	usb_remove_ep_devs(&udev->ep0);
@@ -2235,6 +2259,9 @@ static void set_usb_port_removable(struct usb_device *udev)
 
 	hub = usb_hub_to_struct_hub(udev->parent);
 
+	if (!hub)
+		return;
+
 	wHubCharacteristics = le16_to_cpu(hub->descriptor->wHubCharacteristics);
 
 	if (!(wHubCharacteristics & HUB_CHAR_COMPOUND))
@@ -2342,21 +2369,25 @@ int usb_new_device(struct usb_device *udev)
 	/* Create link files between child device and usb port device. */
 	if (udev->parent) {
 		struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
-		struct usb_port	*port_dev = hub->ports[udev->portnum - 1];
+		struct usb_port	*port_dev;
 
-		err = sysfs_create_link(&udev->dev.kobj,
-				&port_dev->dev.kobj, "port");
-		if (err)
-			goto fail;
+		if (hub) {
+			port_dev = hub->ports[udev->portnum - 1];
 
-		err = sysfs_create_link(&port_dev->dev.kobj,
-				&udev->dev.kobj, "device");
-		if (err) {
-			sysfs_remove_link(&udev->dev.kobj, "port");
-			goto fail;
-		}
+			err = sysfs_create_link(&udev->dev.kobj,
+						&port_dev->dev.kobj, "port");
+			if (err)
+				goto fail;
 
-		pm_runtime_get_sync(&port_dev->dev);
+			err = sysfs_create_link(&port_dev->dev.kobj,
+						&udev->dev.kobj, "device");
+			if (err) {
+				sysfs_remove_link(&udev->dev.kobj, "port");
+				goto fail;
+			}
+
+			pm_runtime_get_sync(&port_dev->dev);
+		}
 	}
 
 	(void) usb_create_ep_devs(&udev->dev, &udev->ep0, udev);
@@ -2897,12 +2928,17 @@ static int usb_disable_function_remotewakeup(struct usb_device *udev)
 int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 {
 	struct usb_hub	*hub = usb_hub_to_struct_hub(udev->parent);
-	struct usb_port *port_dev = hub->ports[udev->portnum - 1];
+	struct usb_port *port_dev;
 	enum pm_qos_flags_status pm_qos_stat;
 	int		port1 = udev->portnum;
 	int		status;
 	bool		really_suspend = true;
 
+	if (!hub)
+		return -EINVAL;
+
+	port_dev = hub->ports[udev->portnum - 1];
+
 	/* enable remote wakeup when appropriate; this lets the device
 	 * wake up the upstream hub (including maybe the root hub).
 	 *
@@ -3162,11 +3198,16 @@ static int finish_port_resume(struct usb_device *udev)
 int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 {
 	struct usb_hub	*hub = usb_hub_to_struct_hub(udev->parent);
-	struct usb_port *port_dev = hub->ports[udev->portnum  - 1];
+	struct usb_port *port_dev;
 	int		port1 = udev->portnum;
 	int		status;
 	u16		portchange, portstatus;
 
+	if (!hub)
+		return -EINVAL;
+
+	port_dev = hub->ports[udev->portnum  - 1];
+
 	if (port_dev->did_runtime_put) {
 		status = pm_runtime_get_sync(&port_dev->dev);
 		port_dev->did_runtime_put = false;
@@ -5037,6 +5078,9 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
 	}
 	parent_hub = usb_hub_to_struct_hub(parent_hdev);
 
+	if (!parent_hub)
+		return -EINVAL;
+
 	/* Disable LPM and LTM while we reset the device and reinstall the alt
 	 * settings.  Device-initiated LPM settings, and system exit latency
 	 * settings are cleared when the device is reset, so we have to set
@@ -5306,7 +5350,7 @@ struct usb_device *usb_hub_find_child(struct usb_device *hdev,
 {
 	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
 
-	if (port1 < 1 || port1 > hdev->maxchild)
+	if (port1 < 1 || port1 > hdev->maxchild || !hub)
 		return NULL;
 	return hub->ports[port1 - 1]->child;
 }
@@ -5323,7 +5367,8 @@ void usb_set_hub_port_connect_type(struct usb_device *hdev, int port1,
 {
 	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
 
-	hub->ports[port1 - 1]->connect_type = type;
+	if (hub)
+		hub->ports[port1 - 1]->connect_type = type;
 }
 
 /**
@@ -5339,6 +5384,9 @@ usb_get_hub_port_connect_type(struct usb_device *hdev, int port1)
 {
 	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
 
+	if (!hub)
+		return USB_PORT_CONNECT_TYPE_UNKNOWN;
+
 	return hub->ports[port1 - 1]->connect_type;
 }
 
@@ -5397,6 +5445,9 @@ acpi_handle usb_get_hub_port_acpi_handle(struct usb_device *hdev,
 {
 	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
 
+	if (!hub)
+		return NULL;
+
 	return DEVICE_ACPI_HANDLE(&hub->ports[port1 - 1]->dev);
 }
 #endif
-- 
1.7.9

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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux