[src/trunk]: src/sys/dev/usb ugen(4): Fix race of ugenopen against itself.

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

details:   https://anonhg.NetBSD.org/src/rev/58d4d12d5038
branches:  trunk
changeset: 985771:58d4d12d5038
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Tue Sep 07 10:42:34 2021 +0000

description:
ugen(4): Fix race of ugenopen against itself.

Even though we have the kernel lock held, a sleep during kmem_alloc
or usbd_open_pipe could allow another ugenopen to run concurrently
before we have marked the endpoint opened.

To avoid this, mark the endpoint open immediately (while we still
have the kernel lock held and before any sleeps, so there is no
TOCTOU error here), and then revert on unwind in the event of
failure.

diffstat:

 sys/dev/usb/ugen.c |  13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diffs (56 lines):

diff -r 2b69710e4f8d -r 58d4d12d5038 sys/dev/usb/ugen.c
--- a/sys/dev/usb/ugen.c        Tue Sep 07 10:42:22 2021 +0000
+++ b/sys/dev/usb/ugen.c        Tue Sep 07 10:42:34 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ugen.c,v 1.159 2021/09/07 10:42:22 riastradh Exp $     */
+/*     $NetBSD: ugen.c,v 1.160 2021/09/07 10:42:34 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.159 2021/09/07 10:42:22 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ugen.c,v 1.160 2021/09/07 10:42:34 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -510,6 +510,7 @@
        struct usbd_xfer *xfer;
        int i, j;
        int error;
+       int opened;
 
        KASSERT(KERNEL_LOCKED_P()); /* sc_is_open */
 
@@ -521,7 +522,7 @@
 
        /* The control endpoint allows multiple opens. */
        if (endpt == USB_CONTROL_ENDPOINT) {
-               sc->sc_is_open[USB_CONTROL_ENDPOINT] = 1;
+               opened = sc->sc_is_open[USB_CONTROL_ENDPOINT] = 1;
                error = 0;
                goto out;
        }
@@ -530,6 +531,7 @@
                error = EBUSY;
                goto out;
        }
+       opened = sc->sc_is_open[endpt] = 1;
 
        /* Make sure there are pipes for all directions. */
        for (dir = OUT; dir <= IN; dir++) {
@@ -663,9 +665,10 @@
                        goto out;
                }
        }
-       sc->sc_is_open[endpt] = 1;
        error = 0;
-out:   ugenif_release(sc);
+out:   if (error && opened)
+               sc->sc_is_open[endpt] = 0;
+       ugenif_release(sc);
        return error;
 }