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
--
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/20230430113944.7D3856050D%40jupiter.mumble.net.
diff --git a/sys/dev/dkwedge/dk.c b/sys/dev/dkwedge/dk.c
index e244eb6d1bc3..7cd825b9691d 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]
*
@@ -1268,8 +1264,8 @@ dkopen(dev_t dev, int flags, int fmt, struct lwp *l)
if (sc == NULL)
return ENXIO;
- if (sc->sc_state != DKW_STATE_RUNNING)
- return ENXIO;
+ KASSERT(sc->sc_dev != NULL);
+ KASSERT(sc->sc_state == DKW_STATE_RUNNING);
/*
* We go through a complicated little dance to only open the parent
@@ -1383,10 +1379,16 @@ dkclose(dev_t dev, int flags, int fmt, struct lwp *l)
{
struct dkwedge_softc *sc = dkwedge_lookup(dev);
+ /*
+ * dkclose can be called even if dkopen didn't succeed, so we
+ * have to handle the same possibility that the wedge may not
+ * exist.
+ */
if (sc == NULL)
return ENXIO;
- if (sc->sc_state != DKW_STATE_RUNNING)
- return ENXIO;
+ KASSERT(sc->sc_dev != NULL);
+ KASSERT(sc->sc_state != DKW_STATE_LARVAL);
+ KASSERT(sc->sc_state != DKW_STATE_DEAD);
mutex_enter(&sc->sc_dk.dk_openlock);
mutex_enter(&sc->sc_parent->dk_rawlock);
@@ -1450,6 +1452,7 @@ dkstrategy(struct buf *bp)
uint64_t p_size, p_offset;
KASSERT(sc != NULL);
+ KASSERT(sc->sc_dev != NULL);
KASSERT(sc->sc_state != DKW_STATE_LARVAL);
KASSERT(sc->sc_state != DKW_STATE_DEAD);
KASSERT(sc->sc_parent->dk_rawvp != NULL);
@@ -1470,7 +1473,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 +1502,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;
@@ -1581,15 +1581,15 @@ dkiodone(struct buf *bp)
struct buf *obp = bp->b_private;
struct dkwedge_softc *sc = dkwedge_lookup(obp->b_dev);
+ KASSERT(sc != NULL);
+ KASSERT(sc->sc_dev != NULL);
+
if (bp->b_error != 0)
obp->b_error = bp->b_error;
obp->b_resid = bp->b_resid;
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);
@@ -1625,6 +1625,9 @@ dkminphys(struct buf *bp)
struct dkwedge_softc *sc = dkwedge_lookup(bp->b_dev);
dev_t dev;
+ KASSERT(sc != NULL);
+ KASSERT(sc->sc_dev != NULL);
+
dev = bp->b_dev;
bp->b_dev = sc->sc_pdev;
if (sc->sc_parent->dk_driver && sc->sc_parent->dk_driver->d_minphys)
@@ -1645,6 +1648,7 @@ dkread(dev_t dev, struct uio *uio, int flags)
struct dkwedge_softc *sc __diagused = dkwedge_lookup(dev);
KASSERT(sc != NULL);
+ KASSERT(sc->sc_dev != NULL);
KASSERT(sc->sc_state != DKW_STATE_LARVAL);
KASSERT(sc->sc_state != DKW_STATE_DEAD);
@@ -1662,6 +1666,7 @@ dkwrite(dev_t dev, struct uio *uio, int flags)
struct dkwedge_softc *sc __diagused = dkwedge_lookup(dev);
KASSERT(sc != NULL);
+ KASSERT(sc->sc_dev != NULL);
KASSERT(sc->sc_state != DKW_STATE_LARVAL);
KASSERT(sc->sc_state != DKW_STATE_DEAD);
@@ -1680,6 +1685,7 @@ dkioctl(dev_t dev, u_long cmd, void *data, int flag, struct lwp *l)
int error = 0;
KASSERT(sc != NULL);
+ KASSERT(sc->sc_dev != NULL);
KASSERT(sc->sc_state != DKW_STATE_LARVAL);
KASSERT(sc->sc_state != DKW_STATE_DEAD);
KASSERT(sc->sc_parent->dk_rawvp != NULL);
@@ -1755,6 +1761,7 @@ dkdiscard(dev_t dev, off_t pos, off_t len)
int error;
KASSERT(sc != NULL);
+ KASSERT(sc->sc_dev != NULL);
KASSERT(sc->sc_state != DKW_STATE_LARVAL);
KASSERT(sc->sc_state != DKW_STATE_DEAD);
KASSERT(sc->sc_parent->dk_rawvp != NULL);
@@ -1792,6 +1799,11 @@ dkdiscard(dev_t dev, off_t pos, off_t len)
static int
dksize(dev_t dev)
{
+ /*
+ * Don't bother taking a reference because this is only used
+ * either (a) while the device is open (for swap), or (b) while
+ * any multiprocessing is quiescent (for crash dumps).
+ */
struct dkwedge_softc *sc = dkwedge_lookup(dev);
uint64_t p_size;
int rv = -1;
@@ -1823,6 +1835,10 @@ dksize(dev_t dev)
static int
dkdump(dev_t dev, daddr_t blkno, void *va, size_t size)
{
+ /*
+ * Don't bother taking a reference because this is only used
+ * while any multiprocessing is quiescent.
+ */
struct dkwedge_softc *sc = dkwedge_lookup(dev);
const struct bdevsw *bdev;
uint64_t p_size, p_offset;
@@ -1865,8 +1881,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 +1891,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 +1904,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 +1912,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 +1935,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