[src/trunk]: src/sys/dev/usb usb(4): Make usbd_fill_iface_data atomic.

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/000e01a1ac79
branches:  trunk
changeset: 1021659:000e01a1ac79
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sat Jun 12 15:41:22 2021 +0000

description:
usb(4): Make usbd_fill_iface_data atomic.

Now either it replaces and frees the old endpoints array, or it
leaves everything in place; it never leaves a partial update nor
requires the caller to free the old array.

diffstat:

 sys/dev/usb/usb_subr.c |  45 +++++++++++++++++++++++++++------------------
 sys/dev/usb/usbdi.c    |  19 +++----------------
 2 files changed, 30 insertions(+), 34 deletions(-)

diffs (149 lines):

diff -r 86c984a4b92a -r 000e01a1ac79 sys/dev/usb/usb_subr.c
--- a/sys/dev/usb/usb_subr.c    Sat Jun 12 15:40:07 2021 +0000
+++ b/sys/dev/usb/usb_subr.c    Sat Jun 12 15:41:22 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usb_subr.c,v 1.258 2021/06/12 15:39:57 riastradh Exp $ */
+/*     $NetBSD: usb_subr.c,v 1.259 2021/06/12 15:41:22 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.258 2021/06/12 15:39:57 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.259 2021/06/12 15:41:22 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -444,6 +444,7 @@
            ifaceidx, altidx, 0, 0);
        struct usbd_interface *ifc = &dev->ud_ifaces[ifaceidx];
        usb_interface_descriptor_t *idesc;
+       struct usbd_endpoint *endpoints;
        char *p, *end;
        int endpt, nendpt;
 
@@ -453,18 +454,16 @@
        idesc = usbd_find_idesc(dev->ud_cdesc, ifaceidx, altidx);
        if (idesc == NULL)
                return USBD_INVAL;
-       ifc->ui_idesc = idesc;
-       ifc->ui_index = ifaceidx;
-       ifc->ui_altindex = altidx;
-       nendpt = ifc->ui_idesc->bNumEndpoints;
+
+       nendpt = idesc->bNumEndpoints;
        DPRINTFN(4, "found idesc nendpt=%jd", nendpt, 0, 0, 0);
        if (nendpt != 0) {
-               ifc->ui_endpoints = kmem_alloc(nendpt * sizeof(struct usbd_endpoint),
-                               KM_SLEEP);
+               endpoints = kmem_alloc(nendpt * sizeof(struct usbd_endpoint),
+                   KM_SLEEP);
        } else
-               ifc->ui_endpoints = NULL;
-       ifc->ui_priv = NULL;
-       p = (char *)ifc->ui_idesc + ifc->ui_idesc->bLength;
+               endpoints = NULL;
+
+       p = (char *)idesc + idesc->bLength;
        end = (char *)dev->ud_cdesc + UGETW(dev->ud_cdesc->wTotalLength);
 #define ed ((usb_endpoint_descriptor_t *)p)
        for (endpt = 0; endpt < nendpt; endpt++) {
@@ -495,7 +494,7 @@
                }
                goto bad;
        found:
-               ifc->ui_endpoints[endpt].ue_edesc = ed;
+               endpoints[endpt].ue_edesc = ed;
                if (dev->ud_speed == USB_SPEED_HIGH) {
                        u_int mps;
                        /* Control and bulk endpoints have max packet limits. */
@@ -518,18 +517,28 @@
                                break;
                        }
                }
-               ifc->ui_endpoints[endpt].ue_refcnt = 0;
-               ifc->ui_endpoints[endpt].ue_toggle = 0;
+               endpoints[endpt].ue_refcnt = 0;
+               endpoints[endpt].ue_toggle = 0;
                p += ed->bLength;
        }
 #undef ed
+
+       /* Success!  Free the old endpoints and commit the changes.  */
+       if (ifc->ui_endpoints) {
+               kmem_free(ifc->ui_endpoints, (sizeof(ifc->ui_endpoints[0]) *
+                       ifc->ui_idesc->bNumEndpoints));
+       }
+
+       ifc->ui_idesc = idesc;
+       ifc->ui_index = ifaceidx;
+       ifc->ui_altindex = altidx;
+       ifc->ui_endpoints = endpoints;
+
        return USBD_NORMAL_COMPLETION;
 
  bad:
-       if (ifc->ui_endpoints != NULL) {
-               kmem_free(ifc->ui_endpoints, nendpt * sizeof(struct usbd_endpoint));
-               ifc->ui_endpoints = NULL;
-       }
+       if (endpoints)
+               kmem_free(endpoints, nendpt * sizeof(struct usbd_endpoint));
        return USBD_INVAL;
 }
 
diff -r 86c984a4b92a -r 000e01a1ac79 sys/dev/usb/usbdi.c
--- a/sys/dev/usb/usbdi.c       Sat Jun 12 15:40:07 2021 +0000
+++ b/sys/dev/usb/usbdi.c       Sat Jun 12 15:41:22 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usbdi.c,v 1.213 2021/06/12 15:40:07 riastradh Exp $    */
+/*     $NetBSD: usbdi.c,v 1.214 2021/06/12 15:41:22 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.213 2021/06/12 15:40:07 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.214 2021/06/12 15:41:22 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -867,9 +867,9 @@
 {
        usb_device_request_t req;
        usbd_status err;
-       void *endpoints;
 
        USBHIST_FUNC();
+       USBHIST_CALLARGS(usbdebug, "iface %#jx", (uintptr_t)iface, 0, 0, 0);
 
        mutex_enter(&iface->ui_pipelock);
        if (LIST_FIRST(&iface->ui_pipes) != NULL) {
@@ -877,23 +877,10 @@
                goto out;
        }
 
-       endpoints = iface->ui_endpoints;
-       int nendpt = iface->ui_idesc->bNumEndpoints;
-       USBHIST_CALLARGS(usbdebug, "iface %#jx endpoints = %#jx nendpt %jd",
-           (uintptr_t)iface, (uintptr_t)endpoints,
-           iface->ui_idesc->bNumEndpoints, 0);
        err = usbd_fill_iface_data(iface->ui_dev, iface->ui_index, altidx);
        if (err)
                goto out;
 
-       /* new setting works, we can free old endpoints */
-       if (endpoints != NULL) {
-               USBHIST_LOG(usbdebug, "iface %#jx endpoints = %#jx nendpt %jd",
-                   (uintptr_t)iface, (uintptr_t)endpoints, nendpt, 0);
-               kmem_free(endpoints, nendpt * sizeof(struct usbd_endpoint));
-       }
-       KASSERT(iface->ui_idesc != NULL);
-
        req.bmRequestType = UT_WRITE_INTERFACE;
        req.bRequest = UR_SET_INTERFACE;
        USETW(req.wValue, iface->ui_idesc->bAlternateSetting);