[src/trunk]: src/sys/dev/usb ugen(4): Ensure we close pipes on detach.

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

details:   https://anonhg.NetBSD.org/src/rev/f9a8b9664c92
branches:  trunk
changeset: 985774:f9a8b9664c92
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Tue Sep 07 10:43:11 2021 +0000

description:
ugen(4): Ensure we close pipes on detach.

Calling vdevgone has the effect of VOP_REVOKE -> spec_node_revoke ->
VOP_CLOSE -> spec_close -> cdev_close -> ugenclose, but:

(a) the flags passed to VOP_CLOSE don't have FREAD or FWRITE, and
(b) ugenclose has no effect when sc_dying is set anyway.

So create another path into the close logic, ugen_do_close.  We have
to do this in detach _after_ the references have drained because we
may free buffers that other users are still using while the reference
count is nonzero.

diffstat:

 sys/dev/usb/ugen.c |  64 ++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 43 insertions(+), 21 deletions(-)

diffs (113 lines):

diff -r a689576a7703 -r f9a8b9664c92 sys/dev/usb/ugen.c
--- a/sys/dev/usb/ugen.c        Tue Sep 07 10:42:59 2021 +0000
+++ b/sys/dev/usb/ugen.c        Tue Sep 07 10:43:11 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ugen.c,v 1.162 2021/09/07 10:42:59 riastradh Exp $     */
+/*     $NetBSD: ugen.c,v 1.163 2021/09/07 10:43:11 riastradh Exp $     */
 
 /*
  * Copyright (c) 1998, 2004 The NetBSD Foundation, Inc.
@@ -37,7 +37,7 @@
 
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ugen.c,v 1.162 2021/09/07 10:42:59 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ugen.c,v 1.163 2021/09/07 10:43:11 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -690,30 +690,25 @@
        return error;
 }
 
-static int
-ugenclose(dev_t dev, int flag, int mode, struct lwp *l)
+static void
+ugen_do_close(struct ugen_softc *sc, int flag, int endpt)
 {
-       int endpt = UGENENDPOINT(dev);
-       struct ugen_softc *sc;
        struct ugen_endpoint *sce;
        int dir;
        int i;
-       int error;
 
        KASSERT(KERNEL_LOCKED_P()); /* sc_is_open */
 
-       if ((sc = ugenif_acquire(UGENUNIT(dev))) == NULL)
-               return ENXIO;
-
-       DPRINTFN(5, ("ugenclose: flag=%d, mode=%d, unit=%d, endpt=%d\n",
-                    flag, mode, UGENUNIT(dev), endpt));
-
-       KASSERT(sc->sc_is_open[endpt]);
+       if (!sc->sc_is_open[endpt]) {
+               KASSERT(sc->sc_endpoints[endpt][IN].pipeh == NULL);
+               KASSERT(sc->sc_endpoints[endpt][OUT].pipeh == NULL);
+               KASSERT(sc->sc_endpoints[endpt][IN].ibuf == NULL);
+               KASSERT(sc->sc_endpoints[endpt][OUT].ibuf == NULL);
+               return;
+       }
 
        if (endpt == USB_CONTROL_ENDPOINT) {
                DPRINTFN(5, ("ugenclose: close control\n"));
-               sc->sc_is_open[endpt] = 0;
-               error = 0;
                goto out;
        }
 
@@ -758,11 +753,31 @@
                        sce->ibuf = NULL;
                }
        }
-       sc->sc_is_open[endpt] = 0;
-       error = 0;
+
+out:   sc->sc_is_open[endpt] = 0;
+}
+
+static int
+ugenclose(dev_t dev, int flag, int mode, struct lwp *l)
+{
+       int endpt = UGENENDPOINT(dev);
+       struct ugen_softc *sc;
+
+       DPRINTFN(5, ("ugenclose: flag=%d, mode=%d, unit=%d, endpt=%d\n",
+                    flag, mode, UGENUNIT(dev), endpt));
 
-out:   ugenif_release(sc);
-       return error;
+       KASSERT(KERNEL_LOCKED_P()); /* ugen_do_close */
+
+       if ((sc = ugenif_acquire(UGENUNIT(dev))) == NULL)
+               return ENXIO;
+
+       KASSERT(sc->sc_is_open[endpt]);
+       ugen_do_close(sc, flag, endpt);
+       KASSERT(!sc->sc_is_open[endpt]);
+
+       ugenif_release(sc);
+
+       return 0;
 }
 
 Static int
@@ -1214,10 +1229,17 @@
        /* locate the major number */
        maj = cdevsw_lookup_major(&ugen_cdevsw);
 
-       /* Nuke the vnodes for any open instances (calls close). */
+       /*
+        * Nuke the vnodes for any open instances (calls ugenclose, but
+        * with no effect because we already set sc_dying).
+        */
        mn = sc->sc_unit * USB_MAX_ENDPOINTS;
        vdevgone(maj, mn, mn + USB_MAX_ENDPOINTS - 1, VCHR);
 
+       /* Actually close any lingering pipes.  */
+       for (i = 0; i < USB_MAX_ENDPOINTS; i++)
+               ugen_do_close(sc, FREAD|FWRITE, i);
+
        usbd_add_drv_event(USB_EVENT_DRIVER_DETACH, sc->sc_udev, sc->sc_dev);
        ugenif_put_unit(sc);