Module Name:    src
Committed By:   riastradh
Date:           Sat Jun 12 14:43:27 UTC 2021

Modified Files:
        src/sys/dev/usb: usb_subr.c usbdi.c usbdivar.h

Log Message:
usb(4): Fix races between usbd_open_pipe* and usbd_set_interface.


To generate a diff of this commit:
cvs rdiff -u -r1.255 -r1.256 src/sys/dev/usb/usb_subr.c
cvs rdiff -u -r1.207 -r1.208 src/sys/dev/usb/usbdi.c
cvs rdiff -u -r1.125 -r1.126 src/sys/dev/usb/usbdivar.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/usb/usb_subr.c
diff -u src/sys/dev/usb/usb_subr.c:1.255 src/sys/dev/usb/usb_subr.c:1.256
--- src/sys/dev/usb/usb_subr.c:1.255	Sat Jun 12 13:58:05 2021
+++ src/sys/dev/usb/usb_subr.c	Sat Jun 12 14:43:27 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: usb_subr.c,v 1.255 2021/06/12 13:58:05 riastradh Exp $	*/
+/*	$NetBSD: usb_subr.c,v 1.256 2021/06/12 14:43:27 riastradh Exp $	*/
 /*	$FreeBSD: src/sys/dev/usb/usb_subr.c,v 1.18 1999/11/17 22:33:47 n_hibma Exp $	*/
 
 /*
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.255 2021/06/12 13:58:05 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.256 2021/06/12 14:43:27 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -403,6 +403,39 @@ usbd_find_edesc(usb_config_descriptor_t 
 	return NULL;
 }
 
+static void
+usbd_iface_init(struct usbd_device *dev, int ifaceidx)
+{
+	struct usbd_interface *ifc = &dev->ud_ifaces[ifaceidx];
+
+	memset(ifc, 0, sizeof(*ifc));
+
+	ifc->ui_dev = dev;
+	ifc->ui_idesc = NULL;
+	ifc->ui_index = 0;
+	ifc->ui_altindex = 0;
+	ifc->ui_endpoints = NULL;
+	ifc->ui_priv = NULL;
+	LIST_INIT(&ifc->ui_pipes);
+	mutex_init(&ifc->ui_pipelock, MUTEX_DEFAULT, IPL_NONE);
+}
+
+static void
+usbd_iface_fini(struct usbd_device *dev, int ifaceidx)
+{
+	struct usbd_interface *ifc = &dev->ud_ifaces[ifaceidx];
+
+	KASSERT(ifc->ui_dev == dev);
+	KASSERT(ifc->ui_idesc == NULL);
+	KASSERT(ifc->ui_index == 0);
+	KASSERT(ifc->ui_altindex == 0);
+	KASSERT(ifc->ui_endpoints == NULL);
+	KASSERT(ifc->ui_priv == NULL);
+	KASSERT(LIST_EMPTY(&ifc->ui_pipes));
+
+	mutex_destroy(&ifc->ui_pipelock);
+}
+
 usbd_status
 usbd_fill_iface_data(struct usbd_device *dev, int ifaceidx, int altidx)
 {
@@ -414,10 +447,12 @@ usbd_fill_iface_data(struct usbd_device 
 	char *p, *end;
 	int endpt, nendpt;
 
+	KASSERT(ifc->ui_dev == dev);
+	KASSERT(LIST_EMPTY(&ifc->ui_pipes));
+
 	idesc = usbd_find_idesc(dev->ud_cdesc, ifaceidx, altidx);
 	if (idesc == NULL)
 		return USBD_INVAL;
-	ifc->ui_dev = dev;
 	ifc->ui_idesc = idesc;
 	ifc->ui_index = ifaceidx;
 	ifc->ui_altindex = altidx;
@@ -488,7 +523,6 @@ usbd_fill_iface_data(struct usbd_device 
 		p += ed->bLength;
 	}
 #undef ed
-	LIST_INIT(&ifc->ui_pipes);
 	return USBD_NORMAL_COMPLETION;
 
  bad:
@@ -499,16 +533,25 @@ usbd_fill_iface_data(struct usbd_device 
 	return USBD_INVAL;
 }
 
-void
+Static void
 usbd_free_iface_data(struct usbd_device *dev, int ifcno)
 {
 	struct usbd_interface *ifc = &dev->ud_ifaces[ifcno];
+
+	KASSERT(ifc->ui_dev == dev);
+	KASSERT(ifc->ui_idesc != NULL);
+	KASSERT(LIST_EMPTY(&ifc->ui_pipes));
+
 	if (ifc->ui_endpoints) {
 		int nendpt = ifc->ui_idesc->bNumEndpoints;
 		size_t sz = nendpt * sizeof(struct usbd_endpoint);
 		kmem_free(ifc->ui_endpoints, sz);
 		ifc->ui_endpoints = NULL;
 	}
+
+	ifc->ui_altindex = 0;
+	ifc->ui_index = 0;
+	ifc->ui_idesc = NULL;
 }
 
 usbd_status
@@ -557,8 +600,10 @@ usbd_set_config_index(struct usbd_device
 		DPRINTF("free old config", 0, 0, 0, 0);
 		/* Free all configuration data structures. */
 		nifc = dev->ud_cdesc->bNumInterface;
-		for (ifcidx = 0; ifcidx < nifc; ifcidx++)
+		for (ifcidx = 0; ifcidx < nifc; ifcidx++) {
 			usbd_free_iface_data(dev, ifcidx);
+			usbd_iface_fini(dev, ifcidx);
+		}
 		kmem_free(dev->ud_ifaces, nifc * sizeof(struct usbd_interface));
 		kmem_free(dev->ud_cdesc, UGETW(dev->ud_cdesc->wTotalLength));
 		if (dev->ud_bdesc != NULL)
@@ -730,10 +775,13 @@ usbd_set_config_index(struct usbd_device
 	dev->ud_cdesc = cdp;
 	dev->ud_config = cdp->bConfigurationValue;
 	for (ifcidx = 0; ifcidx < nifc; ifcidx++) {
+		usbd_iface_init(dev, ifcidx);
 		err = usbd_fill_iface_data(dev, ifcidx, 0);
 		if (err) {
-			while (--ifcidx >= 0)
+			while (--ifcidx >= 0) {
 				usbd_free_iface_data(dev, ifcidx);
+				usbd_iface_fini(dev, ifcidx);
+			}
 			kmem_free(dev->ud_ifaces,
 			    nifc * sizeof(struct usbd_interface));
 			dev->ud_ifaces = NULL;
@@ -1649,8 +1697,10 @@ usb_free_device(struct usbd_device *dev)
 		usbd_kill_pipe(dev->ud_pipe0);
 	if (dev->ud_ifaces != NULL) {
 		nifc = dev->ud_cdesc->bNumInterface;
-		for (ifcidx = 0; ifcidx < nifc; ifcidx++)
+		for (ifcidx = 0; ifcidx < nifc; ifcidx++) {
 			usbd_free_iface_data(dev, ifcidx);
+			usbd_iface_fini(dev, ifcidx);
+		}
 		kmem_free(dev->ud_ifaces,
 		    nifc * sizeof(struct usbd_interface));
 	}

Index: src/sys/dev/usb/usbdi.c
diff -u src/sys/dev/usb/usbdi.c:1.207 src/sys/dev/usb/usbdi.c:1.208
--- src/sys/dev/usb/usbdi.c:1.207	Sat Jun 12 13:58:05 2021
+++ src/sys/dev/usb/usbdi.c	Sat Jun 12 14:43:27 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: usbdi.c,v 1.207 2021/06/12 13:58:05 riastradh Exp $	*/
+/*	$NetBSD: usbdi.c,v 1.208 2021/06/12 14:43:27 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998, 2012, 2015 The NetBSD Foundation, Inc.
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.207 2021/06/12 13:58:05 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.208 2021/06/12 14:43:27 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -244,7 +244,9 @@ usbd_open_pipe_ival(struct usbd_interfac
 	err = usbd_setup_pipe_flags(iface->ui_dev, iface, ep, ival, &p, flags);
 	if (err)
 		return err;
+	mutex_enter(&iface->ui_pipelock);
 	LIST_INSERT_HEAD(&iface->ui_pipes, p, up_next);
+	mutex_exit(&iface->ui_pipelock);
 	*pipe = p;
 	SDT_PROBE5(usb, device, pipe, open,
 	    iface, address, flags, ival, p);
@@ -313,13 +315,14 @@ usbd_close_pipe(struct usbd_pipe *pipe)
 
 	KASSERT(SIMPLEQ_EMPTY(&pipe->up_queue));
 
-	LIST_REMOVE(pipe, up_next);
-
 	pipe->up_methods->upm_close(pipe);
 
 	usbd_unlock_pipe(pipe);
 	if (pipe->up_intrxfer != NULL)
 		usbd_destroy_xfer(pipe->up_intrxfer);
+	mutex_enter(&pipe->up_iface->ui_pipelock);
+	LIST_REMOVE(pipe, up_next);
+	mutex_exit(&pipe->up_iface->ui_pipelock);
 	usb_rem_task_wait(pipe->up_dev, &pipe->up_async_task, USB_TASKQ_DRIVER,
 	    NULL);
 	usbd_endpoint_release(pipe->up_dev, pipe->up_endpoint);
@@ -872,8 +875,11 @@ usbd_set_interface(struct usbd_interface
 
 	USBHIST_FUNC();
 
-	if (LIST_FIRST(&iface->ui_pipes) != NULL)
-		return USBD_IN_USE;
+	mutex_enter(&iface->ui_pipelock);
+	if (LIST_FIRST(&iface->ui_pipes) != NULL) {
+		err = USBD_IN_USE;
+		goto out;
+	}
 
 	endpoints = iface->ui_endpoints;
 	int nendpt = iface->ui_idesc->bNumEndpoints;
@@ -882,7 +888,7 @@ usbd_set_interface(struct usbd_interface
 	    iface->ui_idesc->bNumEndpoints, 0);
 	err = usbd_fill_iface_data(iface->ui_dev, iface->ui_index, altidx);
 	if (err)
-		return err;
+		goto out;
 
 	/* new setting works, we can free old endpoints */
 	if (endpoints != NULL) {
@@ -897,7 +903,10 @@ usbd_set_interface(struct usbd_interface
 	USETW(req.wValue, iface->ui_idesc->bAlternateSetting);
 	USETW(req.wIndex, iface->ui_idesc->bInterfaceNumber);
 	USETW(req.wLength, 0);
-	return usbd_do_request(iface->ui_dev, &req, 0);
+	err = usbd_do_request(iface->ui_dev, &req, 0);
+
+out:	mutex_exit(&iface->ui_pipelock);
+	return err;
 }
 
 int

Index: src/sys/dev/usb/usbdivar.h
diff -u src/sys/dev/usb/usbdivar.h:1.125 src/sys/dev/usb/usbdivar.h:1.126
--- src/sys/dev/usb/usbdivar.h:1.125	Sat Jun 12 13:58:05 2021
+++ src/sys/dev/usb/usbdivar.h	Sat Jun 12 14:43:27 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: usbdivar.h,v 1.125 2021/06/12 13:58:05 riastradh Exp $	*/
+/*	$NetBSD: usbdivar.h,v 1.126 2021/06/12 14:43:27 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998, 2012 The NetBSD Foundation, Inc.
@@ -231,6 +231,7 @@ struct usbd_interface {
 	int			ui_altindex;
 	struct usbd_endpoint   *ui_endpoints;
 	void		       *ui_priv;
+	kmutex_t		ui_pipelock;
 	LIST_HEAD(, usbd_pipe)	ui_pipes;
 };
 

Reply via email to