Source-Changes-HG archive

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

[src/trunk]: src/sys/dev/usb usb(4): Fix races between usbd_open_pipe* and us...



details:   https://anonhg.NetBSD.org/src/rev/a5aa5f9b05ad
branches:  trunk
changeset: 379622:a5aa5f9b05ad
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sat Jun 12 14:43:27 2021 +0000

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

diffstat:

 sys/dev/usb/usb_subr.c |  66 +++++++++++++++++++++++++++++++++++++++++++------
 sys/dev/usb/usbdi.c    |  25 ++++++++++++------
 sys/dev/usb/usbdivar.h |   3 +-
 3 files changed, 77 insertions(+), 17 deletions(-)

diffs (243 lines):

diff -r 9fb555f8b65e -r a5aa5f9b05ad sys/dev/usb/usb_subr.c
--- a/sys/dev/usb/usb_subr.c    Sat Jun 12 13:58:05 2021 +0000
+++ b/sys/dev/usb/usb_subr.c    Sat Jun 12 14:43:27 2021 +0000
@@ -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));
        }
diff -r 9fb555f8b65e -r a5aa5f9b05ad sys/dev/usb/usbdi.c
--- a/sys/dev/usb/usbdi.c       Sat Jun 12 13:58:05 2021 +0000
+++ b/sys/dev/usb/usbdi.c       Sat Jun 12 14:43:27 2021 +0000
@@ -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
diff -r 9fb555f8b65e -r a5aa5f9b05ad sys/dev/usb/usbdivar.h
--- a/sys/dev/usb/usbdivar.h    Sat Jun 12 13:58:05 2021 +0000
+++ b/sys/dev/usb/usbdivar.h    Sat Jun 12 14:43:27 2021 +0000
@@ -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;
 };
 



Home | Main Index | Thread Index | Old Index