[src/trunk]: src/sys/dev/usb Revert "usb: uhub: remove unnecessary delays whe...

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

details:   https://anonhg.NetBSD.org/src/rev/3a460f2c8e3d
branches:  trunk
changeset: 1024265:3a460f2c8e3d
user:      jmcneill <jmcneill%NetBSD.org@localhost>
date:      Fri Oct 15 11:59:16 2021 +0000

description:
Revert "usb: uhub: remove unnecessary delays when powering on ports"

syzbot says that the change exposes UB in usb_free_device and I can't
see how, so revert until I have a better understanding of what's going on.

Reported-by: syzbot+c445f7149cce07d4c252%syzkaller.appspotmail.com@localhost
Reported-by: syzbot+a2ae42f37de765a54b20%syzkaller.appspotmail.com@localhost

diffstat:

 sys/dev/usb/uhub.c |  35 ++++-------------------------------
 1 files changed, 4 insertions(+), 31 deletions(-)

diffs (91 lines):

diff -r 92316bc2041e -r 3a460f2c8e3d sys/dev/usb/uhub.c
--- a/sys/dev/usb/uhub.c        Thu Oct 14 21:35:45 2021 +0000
+++ b/sys/dev/usb/uhub.c        Fri Oct 15 11:59:16 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uhub.c,v 1.157 2021/10/11 06:30:23 msaitoh Exp $       */
+/*     $NetBSD: uhub.c,v 1.158 2021/10/15 11:59:16 jmcneill Exp $      */
 /*     $FreeBSD: src/sys/dev/usb/uhub.c,v 1.18 1999/11/17 22:33:43 n_hibma Exp $       */
 /*     $OpenBSD: uhub.c,v 1.86 2015/06/29 18:27:40 mpi Exp $ */
 
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uhub.c,v 1.157 2021/10/11 06:30:23 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uhub.c,v 1.158 2021/10/15 11:59:16 jmcneill Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -45,7 +45,6 @@
 
 #include <sys/param.h>
 
-#include <sys/bitops.h>
 #include <sys/bus.h>
 #include <sys/device.h>
 #include <sys/kernel.h>
@@ -125,8 +124,6 @@
        struct lwp              *sc_exploring;
 };
 
-typedef __BITMAP_TYPE(, uint8_t, UHD_NPORTS_MAX + 1) usb_port_mask;
-
 #define UHUB_IS_HIGH_SPEED(sc) \
     ((sc)->sc_proto == UDPROTO_HSHUBSTT || (sc)->sc_proto == UDPROTO_HSHUBMTT)
 #define UHUB_IS_SINGLE_TT(sc) ((sc)->sc_proto == UDPROTO_HSHUBSTT)
@@ -498,8 +495,6 @@
        int speed;
        int port;
        int change, status, reconnect, rescan;
-       usb_port_mask powerup_port;
-       int powerup_port_count = 0;
 
        UHUBHIST_FUNC();
        UHUBHIST_CALLARGS("uhub%jd dev=%#jx addr=%jd speed=%ju",
@@ -552,8 +547,6 @@
                }
        }
 
-       __BITMAP_ZERO(&powerup_port);
-
        for (port = 1; port <= hd->bNbrPorts; port++) {
                up = &dev->ud_hub->uh_ports[port - 1];
 
@@ -684,26 +677,8 @@
                DPRINTF("unit %jd dev->speed=%ju dev->depth=%ju",
                    device_unit(sc->sc_dev), dev->ud_speed, dev->ud_depth, 0);
 
-               __BITMAP_SET(port, &powerup_port);
-               powerup_port_count++;
-       }
-
-       if (powerup_port_count == 0) {
-               goto explore;
-       }
-
-       aprint_debug_dev(sc->sc_dev, "power up %u port(s)\n",
-           powerup_port_count);
-
-       /* Wait for maximum device power up time. */
-       usbd_delay_ms(dev, USB_PORT_POWERUP_DELAY);
-
-       for (port = 1; port <= hd->bNbrPorts; port++) {
-               if (!__BITMAP_ISSET(port, &powerup_port)) {
-                       continue;
-               }
-
-               up = &dev->ud_hub->uh_ports[port - 1];
+               /* Wait for maximum device power up time. */
+               usbd_delay_ms(dev, USB_PORT_POWERUP_DELAY);
 
                /* Reset port, which implies enabling it. */
                if (usbd_reset_port(dev, port, &up->up_status)) {
@@ -841,8 +816,6 @@
                                up->up_dev->ud_hub->uh_explore(up->up_dev);
                }
        }
-
-explore:
        mutex_enter(&sc->sc_lock);
        sc->sc_explorepending = false;
        for (int i = 0; i < sc->sc_statuslen; i++) {