From c57c5c6837074019189df022447449899e61aa0a Mon Sep 17 00:00:00 2001 From: bluhm Date: Tue, 2 Jan 2024 16:32:47 +0000 Subject: [PATCH] Prevent simultaneous dt(4) open. Syskaller has hit the assertion "dtlookup(unit) == NULL" by opening dt(4) device in two parallel threads. Convert kassert into if condition. Move check that device is not used after sleep points in malloc. The list dtdev_list is protected by kernel lock which is released during sleep. Reported-by: syzbot+6d66c21f796c817948f0@syzkaller.appspotmail.com OK miod@ --- sys/conf/GENERIC | 3 ++- sys/conf/newvers.sh | 5 +++-- sys/dev/dt/dt_dev.c | 30 ++++++++++++++++++------------ 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/sys/conf/GENERIC b/sys/conf/GENERIC index f8aaca09823..6c8ad03085b 100644 --- a/sys/conf/GENERIC +++ b/sys/conf/GENERIC @@ -1,8 +1,9 @@ -# $OpenBSD: GENERIC,v 1.291 2023/10/04 18:07:13 bluhm Exp $ +# $OpenBSD: GENERIC,v 1.292 2024/01/02 16:32:47 bluhm Exp $ # # Machine-independent option; used by all architectures for their # GENERIC kernel +option HZ=50 # qemu cannot handle more option DDB # in-kernel debugger #option DDBPROF # ddb(4) based profiling #option DDB_SAFE_CONSOLE # allow break into ddb during boot diff --git a/sys/conf/newvers.sh b/sys/conf/newvers.sh index e350ed35832..33a5c7289a2 100644 --- a/sys/conf/newvers.sh +++ b/sys/conf/newvers.sh @@ -1,6 +1,6 @@ #!/bin/sh - # -# $OpenBSD: newvers.sh,v 1.198 2023/10/04 15:40:13 bluhm Exp $ +# $OpenBSD: newvers.sh,v 1.199 2024/01/02 16:32:47 bluhm Exp $ # $NetBSD: newvers.sh,v 1.17.2.1 1995/10/12 05:17:11 jtc Exp $ # # Copyright (c) 1984, 1986, 1990, 1993 @@ -42,6 +42,7 @@ fi touch version v=`cat version` u=`logname` d=${PWD%/obj} h=`hostname` t=`date` id=`basename "${d}"` +git=`git rev-parse --abbrev-ref HEAD` # additional things which need version number upgrades: # sys/sys/param.h: @@ -84,7 +85,7 @@ const char osversion[] = "${id}#${v}"; const char sccs[] = " @(#)${ost} ${osr}" STATUS " (${id}) #${v}: ${t}\n"; const char version[512] = - "${ost} ${osr}" STATUS " (${id}) #${v}: ${t}\n ${u}@${h}:${d}\n"; + "${ost} ${osr}" STATUS " (${id}) #${v}: ${t}\n ${u}@${h}:${d}*${git}\n"; eof expr ${v} + 1 > version diff --git a/sys/dev/dt/dt_dev.c b/sys/dev/dt/dt_dev.c index a103c4067ef..12c5390cdbb 100644 --- a/sys/dev/dt/dt_dev.c +++ b/sys/dev/dt/dt_dev.c @@ -1,4 +1,4 @@ -/* $OpenBSD: dt_dev.c,v 1.28 2023/07/14 07:07:08 claudio Exp $ */ +/* $OpenBSD: dt_dev.c,v 1.29 2024/01/02 16:32:48 bluhm Exp $ */ /* * Copyright (c) 2019 Martin Pieuchot @@ -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, struct proc *p) /* * 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, struct proc *p) 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 -- 2.20.1