Module Name:    src
Committed By:   riastradh
Date:           Tue Sep  7 10:42:34 UTC 2021

Modified Files:
        src/sys/dev/usb: ugen.c

Log Message:
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.


To generate a diff of this commit:
cvs rdiff -u -r1.159 -r1.160 src/sys/dev/usb/ugen.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/usb/ugen.c
diff -u src/sys/dev/usb/ugen.c:1.159 src/sys/dev/usb/ugen.c:1.160
--- src/sys/dev/usb/ugen.c:1.159	Tue Sep  7 10:42:22 2021
+++ src/sys/dev/usb/ugen.c	Tue Sep  7 10:42:34 2021
@@ -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 @@ ugenopen(dev_t dev, int flag, int mode, 
 	struct usbd_xfer *xfer;
 	int i, j;
 	int error;
+	int opened;
 
 	KASSERT(KERNEL_LOCKED_P()); /* sc_is_open */
 
@@ -521,7 +522,7 @@ ugenopen(dev_t dev, int flag, int mode, 
 
 	/* 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 @@ ugenopen(dev_t dev, int flag, int mode, 
 		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 @@ ugenopen(dev_t dev, int flag, int mode, 
 			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;
 }
 

Reply via email to