From: Alexander Bluhm <alexander.bluhm@gmx.net>
Subject: dt open race
To: tech@openbsd.org
Date: Mon, 1 Jan 2024 14:20:57 +0100

Hi,

Syzkaller has found this crash.
https://syzkaller.appspot.com/bug?extid=6d66c21f796c817948f0

Open dt(4) in two parallel threads hits the assertion
"dtlookup(unit) == NULL".

The kassert should be an if condition and sleep points in malloc
also look like a potential race to me.  Kernel lock is released
during sleep.

ok?

bluhm

Index: dev/dt/dt_dev.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dt_dev.c,v
diff -u -p -r1.28 dt_dev.c
--- dev/dt/dt_dev.c	14 Jul 2023 07:07:08 -0000	1.28
+++ dev/dt/dt_dev.c	1 Jan 2024 13:08:49 -0000
@@ -160,13 +160,13 @@ int
 dtopen(dev_t dev, int flags, int mode, struct proc *p)
 {
 	struct dt_softc *sc;
+	struct dt_evt *queue;
+	size_t qlen;
 	int unit = minor(dev);
 
 	if (!allowdt)
 		return EPERM;
 
-	KASSERT(dtlookup(unit) == NULL);
-
 	sc = malloc(sizeof(*sc), M_DEVBUF, M_WAITOK|M_CANFAIL|M_ZERO);
 	if (sc == NULL)
 		return ENOMEM;
@@ -174,16 +174,26 @@ dtopen(dev_t dev, int flags, int mode, s
 	/*
 	 * Enough space to empty 2 full rings of events in a single read.
 	 */
-	sc->ds_bufqlen = 2 * DT_EVTRING_SIZE;
-	sc->ds_bufqueue = mallocarray(sc->ds_bufqlen, sizeof(*sc->ds_bufqueue),
-	    M_DEVBUF, M_WAITOK|M_CANFAIL);
-	if (sc->ds_bufqueue == NULL)
-		goto bad;
+	qlen = 2 * DT_EVTRING_SIZE;
+	queue = mallocarray(qlen, sizeof(*queue), M_DEVBUF, M_WAITOK|M_CANFAIL);
+	if (queue == NULL) {
+		free(sc, M_DEVBUF, sizeof(*sc));
+		return ENOMEM;
+	}
+
+	/* no sleep after this point */
+	if (dtlookup(unit) != NULL) {
+		free(queue, M_DEVBUF, qlen * sizeof(*queue));
+		free(sc, M_DEVBUF, sizeof(*sc));
+		return EBUSY;
+	}
 
 	sc->ds_unit = unit;
 	sc->ds_pid = p->p_p->ps_pid;
 	TAILQ_INIT(&sc->ds_pcbs);
 	mtx_init(&sc->ds_mtx, IPL_HIGH);
+	sc->ds_bufqlen = qlen;
+	sc->ds_bufqueue = queue;
 	sc->ds_evtcnt = 0;
 	sc->ds_readevt = 0;
 	sc->ds_dropevt = 0;
@@ -193,10 +203,6 @@ dtopen(dev_t dev, int flags, int mode, s
 	DPRINTF("dt%d: pid %d open\n", sc->ds_unit, sc->ds_pid);
 
 	return 0;
-
-bad:
-	free(sc, M_DEVBUF, sizeof(*sc));
-	return ENOMEM;
 }
 
 int