NetBSD-Syzbot archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

assert failed: sc->sc_dk.dk_openmask == NUM



#syz test: https://github.com/NetBSD/src trunk

https://syzkaller.appspot.com/bug?id=8a00fd7f2e7459748d7a274098180a4708ff0f61

bisect redux step 2

-- 
You received this message because you are subscribed to the Google Groups "syzkaller-netbsd-bugs" group.
To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-netbsd-bugs+unsubscribe%googlegroups.com@localhost.
To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-netbsd-bugs/20230508211242.95A41609E2%40jupiter.mumble.net.
diff --git a/sys/dev/dkwedge/dk.c b/sys/dev/dkwedge/dk.c
index e244eb6d1bc3..ad6f1a7d8eb1 100644
--- a/sys/dev/dkwedge/dk.c
+++ b/sys/dev/dkwedge/dk.c
@@ -91,8 +91,6 @@ struct dkwedge_softc {
 	struct callout	sc_restart_ch;	/* callout to restart I/O */
 
 	kmutex_t	sc_iolock;
-	kcondvar_t	sc_dkdrn;
-	u_int		sc_iopend;	/* I/Os pending */
 	bool		sc_iostop;	/* don't schedule restart */
 	int		sc_mode;	/* parent open mode */
 };
@@ -101,6 +99,8 @@ static int	dkwedge_match(device_t, cfdata_t, void *);
 static void	dkwedge_attach(device_t, device_t, void *);
 static int	dkwedge_detach(device_t, int);
 
+static void	dk_set_geometry(struct dkwedge_softc *, struct disk *);
+
 static void	dkstart(struct dkwedge_softc *);
 static void	dkiodone(struct buf *);
 static void	dkrestart(void *);
@@ -114,8 +114,6 @@ static int	dkwedge_del1(struct dkwedge_info *, int);
 static int	dk_open_parent(dev_t, int, struct vnode **);
 static int	dk_close_parent(struct vnode *, int);
 
-static int	dkunit(dev_t);
-
 static dev_type_open(dkopen);
 static dev_type_close(dkclose);
 static dev_type_cancel(dkcancel);
@@ -142,7 +140,7 @@ const struct bdevsw dk_bdevsw = {
 	.d_psize = dksize,
 	.d_discard = dkdiscard,
 	.d_cfdriver = &dk_cd,
-	.d_devtounit = dkunit,
+	.d_devtounit = dev_minor_unit,
 	.d_flag = D_DISK | D_MPSAFE
 };
 
@@ -160,7 +158,7 @@ const struct cdevsw dk_cdevsw = {
 	.d_kqfilter = nokqfilter,
 	.d_discard = dkdiscard,
 	.d_cfdriver = &dk_cd,
-	.d_devtounit = dkunit,
+	.d_devtounit = dev_minor_unit,
 	.d_flag = D_DISK | D_MPSAFE
 };
 
@@ -192,24 +190,34 @@ dkwedge_match(device_t parent, cfdata_t match, void *aux)
 static void
 dkwedge_attach(device_t parent, device_t self, void *aux)
 {
+	struct dkwedge_softc *sc = aux;
+	struct disk *pdk = sc->sc_parent;
+	int unit = device_unit(self);
+
+	KASSERTMSG(unit >= 0, "unit=%d", unit);
 
 	if (!pmf_device_register(self, NULL, NULL))
 		aprint_error_dev(self, "couldn't establish power handler\n");
-}
 
-/*
- * dkwedge_wait_drain:
- *
- *	Wait for I/O on the wedge to drain.
- */
-static void
-dkwedge_wait_drain(struct dkwedge_softc *sc)
-{
+	mutex_enter(&pdk->dk_openlock);
+	rw_enter(&dkwedges_lock, RW_WRITER);
+	KASSERTMSG(unit < ndkwedges, "unit=%d ndkwedges=%u", unit, ndkwedges);
+	KASSERTMSG(sc == dkwedges[unit], "sc=%p dkwedges[%d]=%p",
+	    sc, unit, dkwedges[unit]);
+	KASSERTMSG(sc->sc_dev == NULL, "sc=%p sc->sc_dev=%p", sc, sc->sc_dev);
+	sc->sc_dev = self;
+	rw_exit(&dkwedges_lock);
+	mutex_exit(&pdk->dk_openlock);
 
-	mutex_enter(&sc->sc_iolock);
-	while (sc->sc_iopend != 0)
-		cv_wait(&sc->sc_dkdrn, &sc->sc_iolock);
-	mutex_exit(&sc->sc_iolock);
+	disk_init(&sc->sc_dk, device_xname(sc->sc_dev), NULL);
+	mutex_enter(&pdk->dk_openlock);
+	dk_set_geometry(sc, pdk);
+	mutex_exit(&pdk->dk_openlock);
+	disk_attach(&sc->sc_dk);
+
+	/* Disk wedge is ready for use! */
+	device_set_private(self, sc);
+	sc->sc_state = DKW_STATE_RUNNING;
 }
 
 /*
@@ -381,6 +389,7 @@ dkwedge_add(struct dkwedge_info *dkw)
 	u_int unit;
 	int error;
 	dev_t pdev;
+	device_t dev __diagused;
 
 	dkw->dkw_parent[sizeof(dkw->dkw_parent) - 1] = '\0';
 	pdk = disk_find(dkw->dkw_parent);
@@ -410,8 +419,11 @@ dkwedge_add(struct dkwedge_info *dkw)
 			break;
 		if (dkwedge_size(lsc) > dkw->dkw_size)
 			break;
+		if (lsc->sc_dev == NULL)
+			break;
 
 		sc = lsc;
+		device_acquire(sc->sc_dev);
 		dkwedge_size_increase(sc, dkw->dkw_size);
 		dk_set_geometry(sc, pdk);
 
@@ -441,7 +453,6 @@ dkwedge_add(struct dkwedge_info *dkw)
 	callout_setfunc(&sc->sc_restart_ch, dkrestart, sc);
 
 	mutex_init(&sc->sc_iolock, MUTEX_DEFAULT, IPL_BIO);
-	cv_init(&sc->sc_dkdrn, "dkdrn");
 
 	/*
 	 * Wedge will be added; increment the wedge count for the parent.
@@ -484,7 +495,6 @@ dkwedge_add(struct dkwedge_info *dkw)
 	}
 	mutex_exit(&pdk->dk_openlock);
 	if (error) {
-		cv_destroy(&sc->sc_dkdrn);
 		mutex_destroy(&sc->sc_iolock);
 		bufq_free(sc->sc_bufq);
 		dkwedge_size_fini(sc);
@@ -496,7 +506,7 @@ dkwedge_add(struct dkwedge_info *dkw)
 	sc->sc_cfdata.cf_name = dk_cd.cd_name;
 	sc->sc_cfdata.cf_atname = dk_ca.ca_name;
 	/* sc->sc_cfdata.cf_unit set below */
-	sc->sc_cfdata.cf_fstate = FSTATE_STAR;
+	sc->sc_cfdata.cf_fstate = FSTATE_NOTFOUND; /* use chosen cf_unit */
 
 	/* Insert the larval wedge into the array. */
 	rw_enter(&dkwedges_lock, RW_WRITER);
@@ -542,7 +552,6 @@ dkwedge_add(struct dkwedge_info *dkw)
 		LIST_REMOVE(sc, sc_plink);
 		mutex_exit(&pdk->dk_openlock);
 
-		cv_destroy(&sc->sc_dkdrn);
 		mutex_destroy(&sc->sc_iolock);
 		bufq_free(sc->sc_bufq);
 		dkwedge_size_fini(sc);
@@ -558,7 +567,7 @@ dkwedge_add(struct dkwedge_info *dkw)
 	 * This should never fail, unless we're almost totally out of
 	 * memory.
 	 */
-	if ((sc->sc_dev = config_attach_pseudo(&sc->sc_cfdata)) == NULL) {
+	if ((dev = config_attach_pseudo_acquire(&sc->sc_cfdata, sc)) == NULL) {
 		aprint_error("%s%u: unable to attach pseudo-device\n",
 		    sc->sc_cfdata.cf_name, sc->sc_cfdata.cf_unit);
 
@@ -572,7 +581,6 @@ dkwedge_add(struct dkwedge_info *dkw)
 		LIST_REMOVE(sc, sc_plink);
 		mutex_exit(&pdk->dk_openlock);
 
-		cv_destroy(&sc->sc_dkdrn);
 		mutex_destroy(&sc->sc_iolock);
 		bufq_free(sc->sc_bufq);
 		dkwedge_size_fini(sc);
@@ -580,19 +588,7 @@ dkwedge_add(struct dkwedge_info *dkw)
 		return ENOMEM;
 	}
 
-	/*
-	 * XXX Really ought to make the disk_attach() and the changing
-	 * of state to RUNNING atomic.
-	 */
-
-	disk_init(&sc->sc_dk, device_xname(sc->sc_dev), NULL);
-	mutex_enter(&pdk->dk_openlock);
-	dk_set_geometry(sc, pdk);
-	mutex_exit(&pdk->dk_openlock);
-	disk_attach(&sc->sc_dk);
-
-	/* Disk wedge is ready for use! */
-	sc->sc_state = DKW_STATE_RUNNING;
+	KASSERT(dev == sc->sc_dev);
 
 announce:
 	/* Announce our arrival. */
@@ -607,11 +603,12 @@ announce:
 	strlcpy(dkw->dkw_devname, device_xname(sc->sc_dev),
 	    sizeof(dkw->dkw_devname));
 
+	device_release(sc->sc_dev);
 	return 0;
 }
 
 /*
- * dkwedge_find:
+ * dkwedge_find_acquire:
  *
  *	Lookup a disk wedge based on the provided information.
  *	NOTE: We look up the wedge based on the wedge devname,
@@ -619,10 +616,11 @@ announce:
  *
  *	Return NULL if the wedge is not found, otherwise return
  *	the wedge's softc.  Assign the wedge's unit number to unitp
- *	if unitp is not NULL.
+ *	if unitp is not NULL.  The wedge's sc_dev is referenced and
+ *	must be released by device_release or equivalent.
  */
 static struct dkwedge_softc *
-dkwedge_find(struct dkwedge_info *dkw, u_int *unitp)
+dkwedge_find_acquire(struct dkwedge_info *dkw, u_int *unitp)
 {
 	struct dkwedge_softc *sc = NULL;
 	u_int unit;
@@ -632,8 +630,10 @@ dkwedge_find(struct dkwedge_info *dkw, u_int *unitp)
 	rw_enter(&dkwedges_lock, RW_READER);
 	for (unit = 0; unit < ndkwedges; unit++) {
 		if ((sc = dkwedges[unit]) != NULL &&
+		    sc->sc_dev != NULL &&
 		    strcmp(device_xname(sc->sc_dev), dkw->dkw_devname) == 0 &&
 		    strcmp(sc->sc_parent->dk_name, dkw->dkw_parent) == 0) {
+			device_acquire(sc->sc_dev);
 			break;
 		}
 	}
@@ -667,10 +667,10 @@ dkwedge_del1(struct dkwedge_info *dkw, int flags)
 	struct dkwedge_softc *sc = NULL;
 
 	/* Find our softc. */
-	if ((sc = dkwedge_find(dkw, NULL)) == NULL)
+	if ((sc = dkwedge_find_acquire(dkw, NULL)) == NULL)
 		return ESRCH;
 
-	return config_detach(sc->sc_dev, flags);
+	return config_detach_release(sc->sc_dev, flags);
 }
 
 /*
@@ -681,26 +681,16 @@ dkwedge_del1(struct dkwedge_info *dkw, int flags)
 static int
 dkwedge_detach(device_t self, int flags)
 {
-	struct dkwedge_softc *sc = NULL;
-	u_int unit;
-	int bmaj, cmaj, rc;
+	struct dkwedge_softc *const sc = device_private(self);
+	const u_int unit = device_unit(self);
+	int bmaj, cmaj, error;
 
-	rw_enter(&dkwedges_lock, RW_WRITER);
-	for (unit = 0; unit < ndkwedges; unit++) {
-		if ((sc = dkwedges[unit]) != NULL && sc->sc_dev == self)
-			break;
-	}
-	if (unit == ndkwedges)
-		rc = ENXIO;
-	else if ((rc = disk_begindetach(&sc->sc_dk, /*lastclose*/NULL, self,
-		    flags)) == 0) {
-		/* Mark the wedge as dying. */
-		sc->sc_state = DKW_STATE_DYING;
-	}
-	rw_exit(&dkwedges_lock);
+	error = disk_begindetach(&sc->sc_dk, /*lastclose*/NULL, self, flags);
+	if (error)
+		return error;
 
-	if (rc != 0)
-		return rc;
+	/* Mark the wedge as dying. */
+	sc->sc_state = DKW_STATE_DYING;
 
 	pmf_device_deregister(self);
 
@@ -710,14 +700,6 @@ dkwedge_detach(device_t self, int flags)
 	mutex_exit(&sc->sc_iolock);
 	callout_halt(&sc->sc_restart_ch, NULL);
 
-	/*
-	 * dkstart() will kill any queued buffers now that the
-	 * state of the wedge is not RUNNING.  Once we've done
-	 * that, wait for any other pending I/O to complete.
-	 */
-	dkstart(sc);
-	dkwedge_wait_drain(sc);
-
 	/* Locate the wedge major numbers. */
 	bmaj = bdevsw_lookup_major(&dk_bdevsw);
 	cmaj = cdevsw_lookup_major(&dk_cdevsw);
@@ -763,7 +745,6 @@ dkwedge_detach(device_t self, int flags)
 	rw_exit(&dkwedges_lock);
 
 	mutex_destroy(&sc->sc_iolock);
-	cv_destroy(&sc->sc_dkdrn);
 	dkwedge_size_fini(sc);
 
 	free(sc, M_DKWEDGE);
@@ -787,7 +768,6 @@ dkwedge_delall(struct disk *pdk)
 static void
 dkwedge_delall1(struct disk *pdk, bool idleonly)
 {
-	struct dkwedge_info dkw;
 	struct dkwedge_softc *sc;
 	int flags;
 
@@ -799,8 +779,18 @@ dkwedge_delall1(struct disk *pdk, bool idleonly)
 		mutex_enter(&pdk->dk_rawlock); /* for sc->sc_dk.dk_openmask */
 		mutex_enter(&pdk->dk_openlock);
 		LIST_FOREACH(sc, &pdk->dk_wedges, sc_plink) {
-			if (!idleonly || sc->sc_dk.dk_openmask == 0)
+			/*
+			 * Wedge is not yet created.  This is a race --
+			 * it may as well have been added just after we
+			 * deleted all the wedges, so pretend it's not
+			 * here yet.
+			 */
+			if (sc->sc_dev == NULL)
+				continue;
+			if (!idleonly || sc->sc_dk.dk_openmask == 0) {
+				device_acquire(sc->sc_dev);
 				break;
+			}
 		}
 		if (sc == NULL) {
 			KASSERT(idleonly || pdk->dk_nwedges == 0);
@@ -808,12 +798,9 @@ dkwedge_delall1(struct disk *pdk, bool idleonly)
 			mutex_exit(&pdk->dk_rawlock);
 			return;
 		}
-		strlcpy(dkw.dkw_parent, pdk->dk_name, sizeof(dkw.dkw_parent));
-		strlcpy(dkw.dkw_devname, device_xname(sc->sc_dev),
-		    sizeof(dkw.dkw_devname));
 		mutex_exit(&pdk->dk_openlock);
 		mutex_exit(&pdk->dk_rawlock);
-		(void) dkwedge_del1(&dkw, flags);
+		(void)config_detach_release(sc->sc_dev, flags);
 	}
 }
 
@@ -849,7 +836,7 @@ dkwedge_list(struct disk *pdk, struct dkwedge_list *dkwl, struct lwp *l)
 		if (uio.uio_resid < sizeof(dkw))
 			break;
 
-		if (sc->sc_state != DKW_STATE_RUNNING)
+		if (sc->sc_dev == NULL)
 			continue;
 
 		strlcpy(dkw.dkw_devname, device_xname(sc->sc_dev),
@@ -862,9 +849,19 @@ dkwedge_list(struct disk *pdk, struct dkwedge_list *dkwl, struct lwp *l)
 		dkw.dkw_size = dkwedge_size(sc);
 		strlcpy(dkw.dkw_ptype, sc->sc_ptype, sizeof(dkw.dkw_ptype));
 
+		/*
+		 * Acquire a device reference so this wedge doesn't go
+		 * away before our next iteration in LIST_FOREACH, and
+		 * then release the lock for uiomove.
+		 */
+		device_acquire(sc->sc_dev);
+		mutex_exit(&pdk->dk_openlock);
 		error = uiomove(&dkw, sizeof(dkw), &uio);
+		mutex_enter(&pdk->dk_openlock);
+		device_release(sc->sc_dev);
 		if (error)
 			break;
+
 		dkwl->dkwl_ncopied++;
 	}
 	dkwl->dkwl_nwedges = pdk->dk_nwedges;
@@ -873,8 +870,8 @@ dkwedge_list(struct disk *pdk, struct dkwedge_list *dkwl, struct lwp *l)
 	return error;
 }
 
-device_t
-dkwedge_find_by_wname(const char *wname)
+static device_t
+dkwedge_find_by_wname_acquire(const char *wname)
 {
 	device_t dv = NULL;
 	struct dkwedge_softc *sc;
@@ -882,7 +879,7 @@ dkwedge_find_by_wname(const char *wname)
 
 	rw_enter(&dkwedges_lock, RW_READER);
 	for (i = 0; i < ndkwedges; i++) {
-		if ((sc = dkwedges[i]) == NULL)
+		if ((sc = dkwedges[i]) == NULL || sc->sc_dev == NULL)
 			continue;
 		if (strcmp(sc->sc_wname, wname) == 0) {
 			if (dv != NULL) {
@@ -892,6 +889,7 @@ dkwedge_find_by_wname(const char *wname)
 				    device_xname(sc->sc_dev));
 				continue;
 			}
+			device_acquire(sc->sc_dev);
 			dv = sc->sc_dev;
 		}
 	}
@@ -899,17 +897,18 @@ dkwedge_find_by_wname(const char *wname)
 	return dv;
 }
 
-device_t
-dkwedge_find_by_parent(const char *name, size_t *i)
+static device_t
+dkwedge_find_by_parent_acquire(const char *name, size_t *i)
 {
 
 	rw_enter(&dkwedges_lock, RW_READER);
 	for (; *i < (size_t)ndkwedges; (*i)++) {
 		struct dkwedge_softc *sc;
-		if ((sc = dkwedges[*i]) == NULL)
+		if ((sc = dkwedges[*i]) == NULL || sc->sc_dev == NULL)
 			continue;
 		if (strcmp(sc->sc_parent->dk_name, name) != 0)
 			continue;
+		device_acquire(sc->sc_dev);
 		rw_exit(&dkwedges_lock);
 		return sc->sc_dev;
 	}
@@ -917,6 +916,30 @@ dkwedge_find_by_parent(const char *name, size_t *i)
 	return NULL;
 }
 
+/* XXX unsafe */
+device_t
+dkwedge_find_by_wname(const char *wname)
+{
+	device_t dv;
+
+	if ((dv = dkwedge_find_by_wname_acquire(wname)) == NULL)
+		return NULL;
+	device_release(dv);
+	return dv;
+}
+
+/* XXX unsafe */
+device_t
+dkwedge_find_by_parent(const char *name, size_t *i)
+{
+	device_t dv;
+
+	if ((dv = dkwedge_find_by_parent_acquire(name, i)) == NULL)
+		return NULL;
+	device_release(dv);
+	return dv;
+}
+
 void
 dkwedge_print_wnames(void)
 {
@@ -925,7 +948,7 @@ dkwedge_print_wnames(void)
 
 	rw_enter(&dkwedges_lock, RW_READER);
 	for (i = 0; i < ndkwedges; i++) {
-		if ((sc = dkwedges[i]) == NULL)
+		if ((sc = dkwedges[i]) == NULL || sc->sc_dev == NULL)
 			continue;
 		printf(" wedge:%s", sc->sc_wname);
 	}
@@ -1164,21 +1187,24 @@ dkwedge_read(struct disk *pdk, struct vnode *vp, daddr_t blkno,
  * dkwedge_lookup:
  *
  *	Look up a dkwedge_softc based on the provided dev_t.
+ *
+ *	Caller must guarantee the wedge is referenced.
  */
 static struct dkwedge_softc *
 dkwedge_lookup(dev_t dev)
 {
-	const int unit = minor(dev);
-	struct dkwedge_softc *sc;
 
-	rw_enter(&dkwedges_lock, RW_READER);
-	if (unit < 0 || unit >= ndkwedges)
-		sc = NULL;
-	else
-		sc = dkwedges[unit];
-	rw_exit(&dkwedges_lock);
+	return device_lookup_private(&dk_cd, minor(dev));
+}
 
-	return sc;
+static struct dkwedge_softc *
+dkwedge_lookup_acquire(dev_t dev)
+{
+	device_t dv = device_lookup_acquire(&dk_cd, minor(dev));
+
+	if (dv == NULL)
+		return NULL;
+	return device_private(dv);
 }
 
 static int
@@ -1225,36 +1251,6 @@ dk_close_parent(struct vnode *vp, int mode)
 	return error;
 }
 
-/*
- * dkunit:		[devsw entry point]
- *
- *	Return the autoconf device_t unit number of a wedge by its
- *	devsw dev_t number, or -1 if there is none.
- *
- *	XXX This is a temporary hack until dkwedge numbering is made to
- *	correspond 1:1 to autoconf device numbering.
- */
-static int
-dkunit(dev_t dev)
-{
-	int mn = minor(dev);
-	struct dkwedge_softc *sc;
-	device_t dv;
-	int unit = -1;
-
-	if (mn < 0)
-		return -1;
-
-	rw_enter(&dkwedges_lock, RW_READER);
-	if (mn < ndkwedges &&
-	    (sc = dkwedges[minor(dev)]) != NULL &&
-	    (dv = sc->sc_dev) != NULL)
-		unit = device_unit(dv);
-	rw_exit(&dkwedges_lock);
-
-	return unit;
-}
-
 /*
  * dkopen:		[devsw entry point]
  *
@@ -1470,7 +1466,6 @@ dkstrategy(struct buf *bp)
 
 	/* Place it in the queue and start I/O on the unit. */
 	mutex_enter(&sc->sc_iolock);
-	sc->sc_iopend++;
 	disk_wait(&sc->sc_dk);
 	bufq_put(sc->sc_bufq, bp);
 	mutex_exit(&sc->sc_iolock);
@@ -1500,8 +1495,6 @@ dkstart(struct dkwedge_softc *sc)
 	while ((bp = bufq_peek(sc->sc_bufq)) != NULL) {
 		if (sc->sc_iostop) {
 			(void) bufq_get(sc->sc_bufq);
-			if (--sc->sc_iopend == 0)
-				cv_broadcast(&sc->sc_dkdrn);
 			mutex_exit(&sc->sc_iolock);
 			bp->b_error = ENXIO;
 			bp->b_resid = bp->b_bcount;
@@ -1587,9 +1580,6 @@ dkiodone(struct buf *bp)
 	putiobuf(bp);
 
 	mutex_enter(&sc->sc_iolock);
-	if (--sc->sc_iopend == 0)
-		cv_broadcast(&sc->sc_dkdrn);
-
 	disk_unbusy(&sc->sc_dk, obp->b_bcount - obp->b_resid,
 	    obp->b_flags & B_READ);
 	mutex_exit(&sc->sc_iolock);
@@ -1865,8 +1855,9 @@ dkdump(dev_t dev, daddr_t blkno, void *va, size_t size)
  *	Find wedge corresponding to the specified parent name
  *	and offset/length.
  */
-device_t
-dkwedge_find_partition(device_t parent, daddr_t startblk, uint64_t nblks)
+static device_t
+dkwedge_find_partition_acquire(device_t parent, daddr_t startblk,
+    uint64_t nblks)
 {
 	struct dkwedge_softc *sc;
 	int i;
@@ -1874,11 +1865,11 @@ dkwedge_find_partition(device_t parent, daddr_t startblk, uint64_t nblks)
 
 	rw_enter(&dkwedges_lock, RW_READER);
 	for (i = 0; i < ndkwedges; i++) {
-		if ((sc = dkwedges[i]) == NULL)
+		if ((sc = dkwedges[i]) == NULL || sc->sc_dev == NULL)
 			continue;
 		if (strcmp(sc->sc_parent->dk_name, device_xname(parent)) == 0 &&
 		    sc->sc_offset == startblk &&
-		    dkwedge_size(sc) == nblks) {
+		    dkwedge_size(sc) >= nblks) {
 			if (wedge) {
 				printf("WARNING: double match for boot wedge "
 				    "(%s, %s)\n",
@@ -1887,6 +1878,7 @@ dkwedge_find_partition(device_t parent, daddr_t startblk, uint64_t nblks)
 				continue;
 			}
 			wedge = sc->sc_dev;
+			device_acquire(wedge);
 		}
 	}
 	rw_exit(&dkwedges_lock);
@@ -1894,6 +1886,20 @@ dkwedge_find_partition(device_t parent, daddr_t startblk, uint64_t nblks)
 	return wedge;
 }
 
+/* XXX unsafe */
+device_t
+dkwedge_find_partition(device_t parent, daddr_t startblk,
+    uint64_t nblks)
+{
+	device_t dv;
+
+	if ((dv = dkwedge_find_partition_acquire(parent, startblk, nblks))
+	    == NULL)
+		return NULL;
+	device_release(dv);
+	return dv;
+}
+
 const char *
 dkwedge_get_parent_name(dev_t dev)
 {
@@ -1903,8 +1909,11 @@ dkwedge_get_parent_name(dev_t dev)
 
 	if (major(dev) != bmaj && major(dev) != cmaj)
 		return NULL;
-	struct dkwedge_softc *sc = dkwedge_lookup(dev);
+
+	struct dkwedge_softc *const sc = dkwedge_lookup_acquire(dev);
 	if (sc == NULL)
 		return NULL;
-	return sc->sc_parent->dk_name;
+	const char *const name = sc->sc_parent->dk_name;
+	device_release(sc->sc_dev);
+	return name;
 }
diff --git a/sys/kern/subr_autoconf.c b/sys/kern/subr_autoconf.c
index e60be2d7aad8..c8cfae8ab114 100644
--- a/sys/kern/subr_autoconf.c
+++ b/sys/kern/subr_autoconf.c
@@ -1259,7 +1259,7 @@ static const char * const msgs[] = {
  * not configured, call the given `print' function and return NULL.
  */
 device_t
-config_found(device_t parent, void *aux, cfprint_t print,
+config_found_acquire(device_t parent, void *aux, cfprint_t print,
     const struct cfargs * const cfargs)
 {
 	cfdata_t cf;
@@ -1286,6 +1286,29 @@ config_found(device_t parent, void *aux, cfprint_t print,
 	return NULL;
 }
 
+/*
+ * config_found(parent, aux, print, cfargs)
+ *
+ *	Legacy entry point for callers whose use of the returned
+ *	device_t is not delimited by device_release.  This is
+ *	inherently racy -- either they must ignore the return value, or
+ *	they must be converted to config_found_acquire with a matching
+ *	device_release.
+ */
+device_t
+config_found(device_t parent, void *aux, cfprint_t print,
+    const struct cfargs * const cfargs)
+{
+	device_t dev;
+
+	dev = config_found_acquire(parent, aux, print, cfargs);
+	if (dev == NULL)
+		return NULL;
+	device_release(dev);
+
+	return dev;
+}
+
 /*
  * As above, but for root devices.
  */
@@ -1708,6 +1731,8 @@ config_add_attrib_dict(device_t dev)
 
 /*
  * Attach a found device.
+ *
+ * Returns the device referenced, to be released with device_release.
  */
 static device_t
 config_attach_internal(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
@@ -1778,6 +1803,12 @@ config_attach_internal(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
 	 */
 	config_pending_incr(dev);
 
+	/*
+	 * Prevent concurrent detach from destroying the device_t until
+	 * the caller has released the device.
+	 */
+	device_acquire(dev);
+
 	/* Call the driver's attach function.  */
 	(*dev->dv_cfattach->ca_attach)(parent, dev, aux);
 
@@ -1813,7 +1844,7 @@ config_attach_internal(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
 }
 
 device_t
-config_attach(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
+config_attach_acquire(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
     const struct cfargs *cfargs)
 {
 	struct cfargs_internal store;
@@ -1824,6 +1855,29 @@ config_attach(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
 	    cfargs_canonicalize(cfargs, &store));
 }
 
+/*
+ * config_attach(parent, cf, aux, print, cfargs)
+ *
+ *	Legacy entry point for callers whose use of the returned
+ *	device_t is not delimited by device_release.  This is
+ *	inherently racy -- either they must ignore the return value, or
+ *	they must be converted to config_attach_acquire with a matching
+ *	device_release.
+ */
+device_t
+config_attach(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
+    const struct cfargs *cfargs)
+{
+	device_t dev;
+
+	dev = config_attach_acquire(parent, cf, aux, print, cfargs);
+	if (dev == NULL)
+		return NULL;
+	device_release(dev);
+
+	return dev;
+}
+
 /*
  * As above, but for pseudo-devices.  Pseudo-devices attached in this
  * way are silently inserted into the device tree, and their children
@@ -1834,7 +1888,7 @@ config_attach(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
  * name by the attach routine.
  */
 device_t
-config_attach_pseudo(cfdata_t cf)
+config_attach_pseudo_acquire(cfdata_t cf, void *aux)
 {
 	device_t dev;
 
@@ -1867,8 +1921,14 @@ config_attach_pseudo(cfdata_t cf)
 	 */
 	config_pending_incr(dev);
 
+	/*
+	 * Prevent concurrent detach from destroying the device_t until
+	 * the caller has released the device.
+	 */
+	device_acquire(dev);
+
 	/* Call the driver's attach function.  */
-	(*dev->dv_cfattach->ca_attach)(ROOT, dev, NULL);
+	(*dev->dv_cfattach->ca_attach)(ROOT, dev, aux);
 
 	/*
 	 * Allow other threads to acquire references to the device now
@@ -1892,6 +1952,28 @@ out:	KERNEL_UNLOCK_ONE(NULL);
 	return dev;
 }
 
+/*
+ * config_attach_pseudo(cf)
+ *
+ *	Legacy entry point for callers whose use of the returned
+ *	device_t is not delimited by device_release.  This is
+ *	inherently racy -- either they must ignore the return value, or
+ *	they must be converted to config_attach_pseudo_acquire with a
+ *	matching device_release.
+ */
+device_t
+config_attach_pseudo(cfdata_t cf)
+{
+	device_t dev;
+
+	dev = config_attach_pseudo_acquire(cf, NULL);
+	if (dev == NULL)
+		return dev;
+	device_release(dev);
+
+	return dev;
+}
+
 /*
  * Caller must hold alldevs_lock.
  */
@@ -2000,9 +2082,12 @@ config_detach_exit(device_t dev)
  * Note that this code wants to be run from a process context, so
  * that the detach can sleep to allow processes which have a device
  * open to run and unwind their stacks.
+ *
+ * Caller must hold a reference with device_acquire or
+ * device_lookup_acquire.
  */
 int
-config_detach(device_t dev, int flags)
+config_detach_release(device_t dev, int flags)
 {
 	struct alldevs_foray af;
 	struct cftable *ct;
@@ -2031,6 +2116,7 @@ config_detach(device_t dev, int flags)
 	 * attached.
 	 */
 	rv = config_detach_enter(dev);
+	device_release(dev);
 	if (rv) {
 		KERNEL_UNLOCK_ONE(NULL);
 		return rv;
@@ -2185,6 +2271,22 @@ out:
 	return rv;
 }
 
+/*
+ * config_detach(dev, flags)
+ *
+ *	Legacy entry point for callers that have not acquired a
+ *	reference to dev.  This is inherently racy -- you must convert
+ *	such callers to acquire a reference and then use
+ *	config_detach_release instead.
+ */
+int
+config_detach(device_t dev, int flags)
+{
+
+	device_acquire(dev);
+	return config_detach_release(dev, flags);
+}
+
 /*
  * config_detach_commit(dev)
  *
@@ -2740,7 +2842,7 @@ retry:	if (unit < 0 || unit >= cd->cd_ndevs ||
 			mutex_enter(&alldevs_lock);
 			goto retry;
 		}
-		localcount_acquire(dv->dv_localcount);
+		device_acquire(dv);
 	}
 	mutex_exit(&alldevs_lock);
 	mutex_exit(&config_misc_lock);
@@ -2748,10 +2850,31 @@ retry:	if (unit < 0 || unit >= cd->cd_ndevs ||
 	return dv;
 }
 
+/*
+ * device_acquire:
+ *
+ *	Acquire a reference to a device.  It is the caller's
+ *	responsibility to ensure that the device's .ca_detach routine
+ *	cannot return before calling this.  Caller must release the
+ *	reference with device_release or config_detach_release.
+ */
+void
+device_acquire(device_t dv)
+{
+
+	/*
+	 * No lock because the caller has promised that this can't
+	 * change concurrently with device_acquire.
+	 */
+	KASSERTMSG(!dv->dv_detach_done, "%s",
+	    dv == NULL ? "(null)" : device_xname(dv));
+	localcount_acquire(dv->dv_localcount);
+}
+
 /*
  * device_release:
  *
- *	Release a reference to a device acquired with
+ *	Release a reference to a device acquired with device_acquire or
  *	device_lookup_acquire.
  */
 void
diff --git a/sys/kern/subr_device.c b/sys/kern/subr_device.c
index b00538790be2..073620ce1289 100644
--- a/sys/kern/subr_device.c
+++ b/sys/kern/subr_device.c
@@ -30,6 +30,8 @@
 __KERNEL_RCSID(0, "$NetBSD: subr_device.c,v 1.13 2022/03/28 12:38:59 riastradh Exp $");
 
 #include <sys/param.h>
+
+#include <sys/atomic.h>
 #include <sys/device.h>
 #include <sys/device_impl.h>
 #include <sys/systm.h>
@@ -276,7 +278,7 @@ device_private(device_t dev)
 	 * It avoids having them test for it to be NULL and only then calling
 	 * device_private.
 	 */
-	return dev == NULL ? NULL : dev->dv_private;
+	return dev == NULL ? NULL : atomic_load_consume(&dev->dv_private);
 }
 
 void
@@ -287,7 +289,7 @@ device_set_private(device_t dev, void *private)
 	    " device %s already has private set to %p",
 	    dev, private, device_xname(dev), device_private(dev));
 	KASSERT(private != NULL);
-	dev->dv_private = private;
+	atomic_store_release(&dev->dv_private, private);
 }
 
 prop_dictionary_t
diff --git a/sys/sys/device.h b/sys/sys/device.h
index 4f47e063d0a6..91d994a9ff88 100644
--- a/sys/sys/device.h
+++ b/sys/sys/device.h
@@ -554,14 +554,20 @@ device_t config_found(device_t, void *, cfprint_t, const struct cfargs *);
 device_t config_rootfound(const char *, void *);
 device_t config_attach(device_t, cfdata_t, void *, cfprint_t,
 	    const struct cfargs *);
+device_t config_found_acquire(device_t, void *, cfprint_t,
+	    const struct cfargs *);
+device_t config_attach_acquire(device_t, cfdata_t, void *, cfprint_t,
+	    const struct cfargs *);
 int	config_match(device_t, cfdata_t, void *);
 int	config_probe(device_t, cfdata_t, void *);
 
 bool	ifattr_match(const char *, const char *);
 
 device_t config_attach_pseudo(cfdata_t);
+device_t config_attach_pseudo_acquire(cfdata_t, void *);
 
 int	config_detach(device_t, int);
+int	config_detach_release(device_t, int);
 int	config_detach_children(device_t, int flags);
 void	config_detach_commit(device_t);
 bool	config_detach_all(int);
@@ -588,6 +594,7 @@ device_t	device_lookup(cfdriver_t, int);
 void		*device_lookup_private(cfdriver_t, int);
 
 device_t	device_lookup_acquire(cfdriver_t, int);
+void		device_acquire(device_t);
 void		device_release(device_t);
 
 void		device_register(device_t, void *);


Home | Main Index | Thread Index | Old Index