[src/trunk]: src/sys/dev/usb Fix error handling, to prevent kernel crashes at...

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

details:   https://anonhg.NetBSD.org/src/rev/8b0b7a1b9f8a
branches:  trunk
changeset: 965381:8b0b7a1b9f8a
user:      maxv <maxv%NetBSD.org@localhost>
date:      Sat Sep 14 15:24:23 2019 +0000

description:
Fix error handling, to prevent kernel crashes at detach time. The code is
slightly reorganized. Found via vHCI.

diffstat:

 sys/dev/usb/udl.c |  88 +++++++++++++++++++++++++++++++-----------------------
 sys/dev/usb/udl.h |   8 ++++-
 2 files changed, 58 insertions(+), 38 deletions(-)

diffs (176 lines):

diff -r e5b1ca0cb259 -r 8b0b7a1b9f8a sys/dev/usb/udl.c
--- a/sys/dev/usb/udl.c Sat Sep 14 15:22:31 2019 +0000
+++ b/sys/dev/usb/udl.c Sat Sep 14 15:24:23 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: udl.c,v 1.22 2018/09/03 16:29:34 riastradh Exp $       */
+/*     $NetBSD: udl.c,v 1.23 2019/09/14 15:24:23 maxv Exp $    */
 
 /*-
  * Copyright (c) 2009 FUKAUMI Naoki.
@@ -53,7 +53,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: udl.c,v 1.22 2018/09/03 16:29:34 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: udl.c,v 1.23 2019/09/14 15:24:23 maxv Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -369,6 +369,7 @@
 
        sc->sc_dev = self;
        sc->sc_udev = uaa->uaa_device;
+       sc->sc_init_state = UDL_INIT_NONE;
 
        devinfop = usbd_devinfo_alloc(sc->sc_udev, 0);
        aprint_normal_dev(sc->sc_dev, "%s\n", devinfop);
@@ -399,9 +400,6 @@
        if (error != USBD_NORMAL_COMPLETION)
                return;
 
-       /*
-        * Allocate bulk command queue.
-        */
 #ifdef UDL_EVENT_COUNTERS
        evcnt_attach_dynamic(&sc->sc_ev_cmdq_get, EVCNT_TYPE_MISC, NULL,
            device_xname(sc->sc_dev), "udl_cmdq_get");
@@ -412,13 +410,16 @@
        evcnt_attach_dynamic(&sc->sc_ev_cmdq_timeout, EVCNT_TYPE_MISC, NULL,
            device_xname(sc->sc_dev), "udl_cmdq_timeout");
 #endif
+       cv_init(&sc->sc_cv, device_xname(sc->sc_dev));
+       mutex_init(&sc->sc_mtx, MUTEX_DEFAULT, IPL_TTY); /* XXX for tty_lock */
+       sc->sc_init_state = UDL_INIT_MIDWAY;
 
+       /*
+        * Allocate bulk command queue.
+        */
        if (udl_cmdq_alloc(sc) != 0)
                return;
 
-       cv_init(&sc->sc_cv, device_xname(sc->sc_dev));
-       mutex_init(&sc->sc_mtx, MUTEX_DEFAULT, IPL_TTY); /* XXX for tty_lock */
-
        if ((sc->sc_cmd_cur = udl_cmdq_get(sc)) == NULL)
                return;
        UDL_CMD_BUFINIT(sc);
@@ -483,6 +484,8 @@
        sc->sc_thread_stop = true;
        kthread_create(PRI_BIO, KTHREAD_MPSAFE | KTHREAD_MUSTJOIN, NULL,
            udl_update_thread, sc, &sc->sc_thread, "udlupd");
+
+       sc->sc_init_state = UDL_INIT_INITED;
 }
 
 static int
@@ -497,11 +500,13 @@
                usbd_abort_pipe(sc->sc_tx_pipeh);
        }
 
-       /*
-        * Free command xfer buffers.
-        */
-       udl_cmdq_flush(sc);
-       udl_cmdq_free(sc);
+       if (sc->sc_init_state >= UDL_INIT_MIDWAY) {
+               /*
+                * Free command xfer buffers.
+                */
+               udl_cmdq_flush(sc);
+               udl_cmdq_free(sc);
+       }
 
        if (sc->sc_tx_pipeh != NULL) {
                usbd_close_pipe(sc->sc_tx_pipeh);
@@ -512,36 +517,45 @@
         */
        udl_comp_unload(sc);
 
-       /*
-        * Free framebuffer memory.
-        */
-       udl_fbmem_free(sc);
+       if (sc->sc_init_state >= UDL_INIT_INITED) {
+               /*
+                * Free framebuffer memory.
+                */
+               udl_fbmem_free(sc);
 
-       mutex_enter(&sc->sc_thread_mtx);
-       sc->sc_dying = true;
-       cv_broadcast(&sc->sc_thread_cv);
-       mutex_exit(&sc->sc_thread_mtx);
-       kthread_join(sc->sc_thread);
+               mutex_enter(&sc->sc_thread_mtx);
+               sc->sc_dying = true;
+               cv_broadcast(&sc->sc_thread_cv);
+               mutex_exit(&sc->sc_thread_mtx);
+               kthread_join(sc->sc_thread);
+               cv_destroy(&sc->sc_thread_cv);
+               mutex_destroy(&sc->sc_thread_mtx);
+       }
+
+       if (sc->sc_init_state >= UDL_INIT_MIDWAY) {
+               cv_destroy(&sc->sc_cv);
+               mutex_destroy(&sc->sc_mtx);
+       }
 
-       cv_destroy(&sc->sc_cv);
-       mutex_destroy(&sc->sc_mtx);
-       cv_destroy(&sc->sc_thread_cv);
-       mutex_destroy(&sc->sc_thread_mtx);
+       if (sc->sc_init_state >= UDL_INIT_INITED) {
+               /*
+                * Detach wsdisplay.
+                */
+               if (sc->sc_wsdisplay != NULL)
+                       config_detach(sc->sc_wsdisplay, DETACH_FORCE);
 
-       /*
-        * Detach wsdisplay.
-        */
-       if (sc->sc_wsdisplay != NULL)
-               config_detach(sc->sc_wsdisplay, DETACH_FORCE);
+               usbd_add_drv_event(USB_EVENT_DRIVER_DETACH, sc->sc_udev,
+                   sc->sc_dev);
+       }
 
-       usbd_add_drv_event(USB_EVENT_DRIVER_DETACH, sc->sc_udev, sc->sc_dev);
-
+       if (sc->sc_init_state >= UDL_INIT_MIDWAY) {
 #ifdef UDL_EVENT_COUNTERS
-       evcnt_detach(&sc->sc_ev_cmdq_get);
-       evcnt_detach(&sc->sc_ev_cmdq_put);
-       evcnt_detach(&sc->sc_ev_cmdq_wait);
-       evcnt_detach(&sc->sc_ev_cmdq_timeout);
+               evcnt_detach(&sc->sc_ev_cmdq_get);
+               evcnt_detach(&sc->sc_ev_cmdq_put);
+               evcnt_detach(&sc->sc_ev_cmdq_wait);
+               evcnt_detach(&sc->sc_ev_cmdq_timeout);
 #endif
+       }
 
        return 0;
 }
diff -r e5b1ca0cb259 -r 8b0b7a1b9f8a sys/dev/usb/udl.h
--- a/sys/dev/usb/udl.h Sat Sep 14 15:22:31 2019 +0000
+++ b/sys/dev/usb/udl.h Sat Sep 14 15:24:23 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: udl.h,v 1.4 2016/10/18 20:17:37 nat Exp $      */
+/*     $NetBSD: udl.h,v 1.5 2019/09/14 15:24:23 maxv Exp $     */
 
 /*-
  * Copyright (c) 2009 FUKAUMI Naoki.
@@ -81,6 +81,12 @@
        struct usbd_interface *  sc_iface;
        struct usbd_pipe *       sc_tx_pipeh;
 
+       enum {
+               UDL_INIT_NONE,
+               UDL_INIT_MIDWAY,
+               UDL_INIT_INITED
+       } sc_init_state;
+
        struct udl_cmdq          sc_cmdq[UDL_NCMDQ];
        TAILQ_HEAD(udl_cmdq_head, udl_cmdq)     sc_freecmd,
                                                sc_xfercmd;