[src/trunk]: src/sys/dev/usb add new usbd_do_request_len() that can allocate ...

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

details:   https://anonhg.NetBSD.org/src/rev/6dda731b376c
branches:  trunk
changeset: 965016:6dda731b376c
user:      mrg <mrg%NetBSD.org@localhost>
date:      Wed Aug 28 01:44:39 2019 +0000

description:
add new usbd_do_request_len() that can allocate a larger than
request size buffer.  reimplement usbd_do_request_flags() in
terms of this.  use this for fetching string descriptors.

fixes a very strange problem where an axe(4) attaching (either
has ugen(4) or axe(4)) would ask for 2 bytes, usb_mem.c would
allocate a 2 byte fragment, perform the operation, and sometime
shortly afterwards (usually by the time the next allocation
is made for this fragment), would become corrupted (usually
two bytes were written with 0x0304.)

(initial request of 4 bytes also avoids the problem on this
device.  it really seems like a HC problem -- host should not
allow the device to write more than req.wLength!  nor should
it allow this write to happen after completion.)

avoid an (almost) always double-log in usbd_transfer().

diffstat:

 sys/dev/usb/usb_subr.c |  21 ++++++++++++++-------
 sys/dev/usb/usbdi.c    |  26 ++++++++++++++++++--------
 sys/dev/usb/usbdi.h    |   5 ++++-
 3 files changed, 36 insertions(+), 16 deletions(-)

diffs (152 lines):

diff -r 88ed95c35132 -r 6dda731b376c sys/dev/usb/usb_subr.c
--- a/sys/dev/usb/usb_subr.c    Tue Aug 27 22:48:53 2019 +0000
+++ b/sys/dev/usb/usb_subr.c    Wed Aug 28 01:44:39 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usb_subr.c,v 1.238 2019/08/21 10:48:37 mrg Exp $       */
+/*     $NetBSD: usb_subr.c,v 1.239 2019/08/28 01:44:39 mrg 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.238 2019/08/21 10:48:37 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.239 2019/08/28 01:44:39 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -122,13 +122,20 @@
        usbd_status err;
        int actlen;
 
+       /*
+        * Pass a full-sized buffer to usbd_do_request_len().  At least
+        * one device has been seen returning additional data beyond the
+        * provided buffers (2-bytes written shortly after the request
+        * claims to have completed and returned the 2 byte header,
+        * corrupting other memory.)
+        */
        req.bmRequestType = UT_READ_DEVICE;
        req.bRequest = UR_GET_DESCRIPTOR;
        USETW2(req.wValue, UDESC_STRING, sindex);
        USETW(req.wIndex, langid);
        USETW(req.wLength, 2);  /* only size byte first */
-       err = usbd_do_request_flags(dev, &req, sdesc, USBD_SHORT_XFER_OK,
-               &actlen, USBD_DEFAULT_TIMEOUT);
+       err = usbd_do_request_len(dev, &req, sizeof(*sdesc), sdesc,
+           USBD_SHORT_XFER_OK, &actlen, USBD_DEFAULT_TIMEOUT);
        if (err)
                return err;
 
@@ -138,8 +145,8 @@
        if (sdesc->bLength > sizeof(*sdesc))
                return USBD_INVAL;
        USETW(req.wLength, sdesc->bLength);     /* the whole string */
-       err = usbd_do_request_flags(dev, &req, sdesc, USBD_SHORT_XFER_OK,
-               &actlen, USBD_DEFAULT_TIMEOUT);
+       err = usbd_do_request_len(dev, &req, sizeof(*sdesc), sdesc,
+           USBD_SHORT_XFER_OK, &actlen, USBD_DEFAULT_TIMEOUT);
        if (err)
                return err;
 
@@ -1175,7 +1182,7 @@
        req.bRequest = UR_GET_DESCRIPTOR;
        USETW2(req.wValue, UDESC_DEVICE, 0);
        USETW(req.wIndex, 0);
-       USETW(req.wLength, 64);
+       USETW(req.wLength, 8);
        res = usbd_do_request_flags(dev, &req, buf, USBD_SHORT_XFER_OK,
                &actlen, USBD_DEFAULT_TIMEOUT);
        if (res)
diff -r 88ed95c35132 -r 6dda731b376c sys/dev/usb/usbdi.c
--- a/sys/dev/usb/usbdi.c       Tue Aug 27 22:48:53 2019 +0000
+++ b/sys/dev/usb/usbdi.c       Wed Aug 28 01:44:39 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usbdi.c,v 1.185 2019/08/24 07:43:00 mrg Exp $  */
+/*     $NetBSD: usbdi.c,v 1.186 2019/08/28 01:44:39 mrg 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.185 2019/08/24 07:43:00 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.186 2019/08/28 01:44:39 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -323,8 +323,6 @@
 
        /* xfer is not valid after the transfer method unless synchronous */
        err = pipe->up_methods->upm_transfer(xfer);
-       USBHIST_LOG(usbdebug, "<- done transfer %#jx, err = %jd",
-           (uintptr_t)xfer, err, 0, 0);
 
        if (err != USBD_IN_PROGRESS && err) {
                /*
@@ -332,6 +330,8 @@
                 * accepted by the HCD for some reason.  It needs removing
                 * from the pipe queue.
                 */
+               USBHIST_LOG(usbdebug, "xfer failed: %s, reinserting",
+                   err, 0, 0, 0);
                usbd_lock_pipe(pipe);
                SIMPLEQ_REMOVE_HEAD(&pipe->up_queue, ux_next);
                if (pipe->up_serialise)
@@ -346,8 +346,8 @@
        }
 
        if (err != USBD_IN_PROGRESS) {
-               USBHIST_LOG(usbdebug, "<- done xfer %#jx, sync (err %jd)"
-                   "(complete/error)", (uintptr_t)xfer, err, 0, 0);
+               USBHIST_LOG(usbdebug, "<- done xfer %#jx, sync (err %jd)",
+                   (uintptr_t)xfer, err, 0, 0);
                return err;
        }
 
@@ -1100,12 +1100,22 @@
 usbd_do_request_flags(struct usbd_device *dev, usb_device_request_t *req,
     void *data, uint16_t flags, int *actlen, uint32_t timeout)
 {
+       size_t len = UGETW(req->wLength);
+
+       return usbd_do_request_len(dev, req, len, data, flags, actlen, timeout);
+}
+
+usbd_status
+usbd_do_request_len(struct usbd_device *dev, usb_device_request_t *req,
+    size_t len, void *data, uint16_t flags, int *actlen, uint32_t timeout)
+{
        struct usbd_xfer *xfer;
        usbd_status err;
-       size_t len = UGETW(req->wLength);
+
+       KASSERT(len >= UGETW(req->wLength));
 
        USBHIST_FUNC();
-       USBHIST_CALLARGS(usbdebug, "dev=%#jx req=%jx flgas=%jx len=%jx",
+       USBHIST_CALLARGS(usbdebug, "dev=%#jx req=%jx flags=%jx len=%jx",
            (uintptr_t)dev, (uintptr_t)req, flags, len);
 
        ASSERT_SLEEPABLE();
diff -r 88ed95c35132 -r 6dda731b376c sys/dev/usb/usbdi.h
--- a/sys/dev/usb/usbdi.h       Tue Aug 27 22:48:53 2019 +0000
+++ b/sys/dev/usb/usbdi.h       Wed Aug 28 01:44:39 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usbdi.h,v 1.96 2019/01/27 02:08:42 pgoyette Exp $      */
+/*     $NetBSD: usbdi.h,v 1.97 2019/08/28 01:44:39 mrg Exp $   */
 /*     $FreeBSD: src/sys/dev/usb/usbdi.h,v 1.18 1999/11/17 22:33:49 n_hibma Exp $      */
 
 /*
@@ -143,6 +143,9 @@
     usb_device_request_t *, void *, usbd_callback);
 usbd_status usbd_do_request_flags(struct usbd_device *, usb_device_request_t *,
     void *, uint16_t, int *, uint32_t);
+usbd_status usbd_do_request_len(struct usbd_device *dev,
+    usb_device_request_t *req, size_t len, void *data, uint16_t flags,
+    int *actlen, uint32_t timeout);
 
 usb_interface_descriptor_t *
     usbd_get_interface_descriptor(struct usbd_interface *);