Ensure that me_evp is still NULL before assignment during open of wscons

devices. This condition is checked early on during open but since the
same routine could end up sleeping before assigning me_evp, a race
against adding the same wscons device to a wsmux could be lost. This in
turn can cause a NULL deference during close.

ok mpi@

Reported-by: syzbot+34c3041bfd96c888c8bd@syzkaller.appspotmail.com

3 files changed, 35 insertions, 26 deletions

diff --git a/sys/dev/wscons/wskbd.c b/sys/dev/wscons/wskbd.c
index a923c05a0b1..7df81e70d03 100644
--- a/sys/dev/wscons/wskbd.c
+++ b/sys/dev/wscons/wskbd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: wskbd.c,v 1.101 2020/03/22 07:59:59 anton Exp $ */
+/* $OpenBSD: wskbd.c,v 1.102 2020/03/24 07:53:24 anton Exp $ */
 /* $NetBSD: wskbd.c,v 1.80 2005/05/04 01:52:16 augustss Exp $ */
 
 /*
@@ -780,9 +780,6 @@ wskbd_mux_open(struct wsevsrc *me, struct wseventvar *evp)
 	if (sc->sc_dying)
 		return (EIO);
 
-	if (sc->sc_base.me_evp != NULL)
-		return (EBUSY);
-
 	return (wskbd_do_open(sc, evp));
 }
 #endif
@@ -831,7 +828,6 @@ wskbdopen(dev_t dev, int flags, int mode, struct proc *p)
 	if (error) {
 		DPRINTF(("%s: %s open failed\n", __func__,
 			 sc->sc_base.me_dv.dv_xname));
-		sc->sc_base.me_evp = NULL;
 		wsevent_fini(evar);
 	}
 	return (error);
@@ -840,10 +836,19 @@ wskbdopen(dev_t dev, int flags, int mode, struct proc *p)
 int
 wskbd_do_open(struct wskbd_softc *sc, struct wseventvar *evp)
 {
+	int error;
+
+	/* The device could already be attached to a mux. */
+	if (sc->sc_base.me_evp != NULL)
+		return (EBUSY);
+
 	sc->sc_base.me_evp = evp;
 	sc->sc_translating = 0;
 
-	return (wskbd_enable(sc, 1));
+	error = wskbd_enable(sc, 1);
+	if (error)
+		sc->sc_base.me_evp = NULL;
+	return (error);
 }
 
 int
diff --git a/sys/dev/wscons/wsmouse.c b/sys/dev/wscons/wsmouse.c
index c62f5abbec6..ea1181dac47 100644
--- a/sys/dev/wscons/wsmouse.c
+++ b/sys/dev/wscons/wsmouse.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: wsmouse.c,v 1.60 2020/03/22 16:39:51 bru Exp $ */
+/* $OpenBSD: wsmouse.c,v 1.61 2020/03/24 07:53:24 anton Exp $ */
 /* $NetBSD: wsmouse.c,v 1.35 2005/02/27 00:27:52 perry Exp $ */
 
 /*
@@ -335,7 +335,6 @@ wsmouseopen(dev_t dev, int flags, int mode, struct proc *p)
 	if (error) {
 		DPRINTF(("%s: %s open failed\n", __func__,
 			 sc->sc_base.me_dv.dv_xname));
-		sc->sc_base.me_evp = NULL;
 		wsevent_fini(evar);
 	}
 	return (error);
@@ -377,12 +376,20 @@ wsmouseclose(dev_t dev, int flags, int mode, struct proc *p)
 int
 wsmousedoopen(struct wsmouse_softc *sc, struct wseventvar *evp)
 {
+	int error;
+
+	/* The device could already be attached to a mux. */
+	if (sc->sc_base.me_evp != NULL)
+		return (EBUSY);
 	sc->sc_base.me_evp = evp;
 
 	wsmouse_input_reset(&sc->sc_input);
 
 	/* enable the device, and punt if that's not possible */
-	return (*sc->sc_accessops->enable)(sc->sc_accesscookie);
+	error = (*sc->sc_accessops->enable)(sc->sc_accesscookie);
+	if (error)
+		sc->sc_base.me_evp = NULL;
+	return (error);
 }
 
 int
@@ -556,9 +563,6 @@ wsmouse_mux_open(struct wsevsrc *me, struct wseventvar *evp)
 {
 	struct wsmouse_softc *sc = (struct wsmouse_softc *)me;
 
-	if (sc->sc_base.me_evp != NULL)
-		return (EBUSY);
-
 	return wsmousedoopen(sc, evp);
 }
 
diff --git a/sys/dev/wscons/wsmux.c b/sys/dev/wscons/wsmux.c
index ede38d63714..ce6b237d802 100644
--- a/sys/dev/wscons/wsmux.c
+++ b/sys/dev/wscons/wsmux.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: wsmux.c,v 1.49 2020/01/08 16:27:41 visa Exp $	*/
+/*	$OpenBSD: wsmux.c,v 1.50 2020/03/24 07:53:24 anton Exp $	*/
 /*      $NetBSD: wsmux.c,v 1.37 2005/04/30 03:47:12 augustss Exp $      */
 
 /*
@@ -90,7 +90,7 @@ int	wsmuxdebug = 0;
 int	wsmux_mux_open(struct wsevsrc *, struct wseventvar *);
 int	wsmux_mux_close(struct wsevsrc *);
 
-void	wsmux_do_open(struct wsmux_softc *, struct wseventvar *);
+int	wsmux_do_open(struct wsmux_softc *, struct wseventvar *);
 
 void	wsmux_do_close(struct wsmux_softc *);
 #if NWSDISPLAY > 0
@@ -184,7 +184,7 @@ wsmuxopen(dev_t dev, int flags, int mode, struct proc *p)
 {
 	struct wsmux_softc *sc;
 	struct wseventvar *evar;
-	int unit;
+	int error, unit;
 
 	unit = minor(dev);
 	sc = wsmux_getmux(unit);
@@ -215,9 +215,10 @@ wsmuxopen(dev_t dev, int flags, int mode, struct proc *p)
 	sc->sc_rawkbd = 0;
 #endif
 
-	wsmux_do_open(sc, evar);
-
-	return (0);
+	error = wsmux_do_open(sc, evar);
+	if (error)
+                wsevent_fini(evar);
+	return (error);
 }
 
 /*
@@ -229,23 +230,17 @@ wsmux_mux_open(struct wsevsrc *me, struct wseventvar *evar)
 	struct wsmux_softc *sc = (struct wsmux_softc *)me;
 
 #ifdef DIAGNOSTIC
-	if (sc->sc_base.me_evp != NULL) {
-		printf("wsmux_mux_open: busy\n");
-		return (EBUSY);
-	}
 	if (sc->sc_base.me_parent == NULL) {
 		printf("wsmux_mux_open: no parent\n");
 		return (EINVAL);
 	}
 #endif
 
-	wsmux_do_open(sc, evar);
-
-	return (0);
+	return (wsmux_do_open(sc, evar));
 }
 
 /* Common part of opening a mux. */
-void
+int
 wsmux_do_open(struct wsmux_softc *sc, struct wseventvar *evar)
 {
 	struct wsevsrc *me;
@@ -253,6 +248,9 @@ wsmux_do_open(struct wsmux_softc *sc, struct wseventvar *evar)
 	int error;
 #endif
 
+	/* The device could already be attached to a mux. */
+	if (sc->sc_base.me_evp != NULL)
+		return (EBUSY);
 	sc->sc_base.me_evp = evar; /* remember event variable, mark as open */
 
 	/* Open all children. */
@@ -280,6 +278,8 @@ wsmux_do_open(struct wsmux_softc *sc, struct wseventvar *evar)
 #endif
 	}
 	rw_exit_read(&sc->sc_lock);
+
+	return (0);
 }
 
 /*