Change KASSERT() in feed_vchan16() into an explicit test and call to

panic() so that the buffer overflow just beyond this point is always
caught, even when the code is not compiled with INVARIANTS.

Change chn_setblocksize() buffer reallocation code to attempt to avoid
the feed_vchan16() buffer overflow by attempting to always keep the
bufsoft buffer at least as large as the bufhard buffer.

Print a diagnositic message
	Danger! %s bufsoft size increasing from %d to %d after CHANNEL_SETBLOCKSIZE()
if our best attempts fail.  If feed_vchan16() were to be called by
the interrupt handler while locks are dropped in chn_setblocksize()
to increase the size bufsoft to match the size of bufhard, the panic()
code in feed_vchan16() will be triggered.  If the diagnostic message
is printed, it is a warning that a panic is possible if the system
were to see events in an "unlucky" order.

Change the locking code to avoid the need for MTX_RECURSIVE mutexes.

Add the MTX_DUPOK option to the channel mutexes and change the locking
sequence to always lock the parent channel before its children to avoid
the possibility of deadlock.

Actually implement locking assertions for the channel mutexes and fix
the problems found by the resulting assertion violations.

Clean up the locking code in dsp_ioctl().

Allocate the channel buffers using the malloc() M_WAITOK option instead
of M_NOWAIT so that buffer allocation won't fail.  Drop locks across
the malloc() calls.

Add/modify KASSERTS() in attempt to detect problems early.

Abuse layering by adding a pointer to the snd_dbuf structure that points
back to the pcm_channel that owns it.  This allows sndbuf_resize() to do
proper locking without having to change the its API, which is used by
the hardware drivers.

Don't dereference a NULL pointer when setting hw.snd.maxautovchans
if a hardware driver is not loaded.  Noticed by Ryan Sommers
<ryans at gamersimpact.com>.

Tested by:	Stefan Ehmann <shoesoft AT gmx.net>
Tested by:	matk (Mathew Kanner)
Tested by:	Gordon Bergling <gbergling AT 0xfce3.net>

8 files changed, 245 insertions, 140 deletions

diff --git a/sys/dev/sound/pcm/buffer.c b/sys/dev/sound/pcm/buffer.c
index 0021c25..46759cb 100644
--- a/sys/dev/sound/pcm/buffer.c
+++ b/sys/dev/sound/pcm/buffer.c
@@ -31,13 +31,14 @@
 SND_DECLARE_FILE("$FreeBSD$");
 
 struct snd_dbuf *
-sndbuf_create(device_t dev, char *drv, char *desc)
+sndbuf_create(device_t dev, char *drv, char *desc, struct pcm_channel *channel)
 {
 	struct snd_dbuf *b;
 
 	b = malloc(sizeof(*b), M_DEVBUF, M_WAITOK | M_ZERO);
 	snprintf(b->name, SNDBUF_NAMELEN, "%s:%s", drv, desc);
 	b->dev = dev;
+	b->channel = channel;
 
 	return b;
 }
@@ -113,27 +114,38 @@ sndbuf_free(struct snd_dbuf *b)
 int
 sndbuf_resize(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz)
 {
-	u_int8_t *tmpbuf;
+	u_int8_t *tmpbuf, *f2;
+
+	chn_lock(b->channel);
 	if (b->maxsize == 0)
-		return 0;
+		goto out;
 	if (blkcnt == 0)
 		blkcnt = b->blkcnt;
 	if (blksz == 0)
 		blksz = b->blksz;
-	if (blkcnt < 2 || blksz < 16 || (blkcnt * blksz > b->maxsize))
+	if (blkcnt < 2 || blksz < 16 || (blkcnt * blksz > b->maxsize)) {
+		chn_unlock(b->channel);
 		return EINVAL;
+	}
 	if (blkcnt == b->blkcnt && blksz == b->blksz)
-		return 0;
-	b->blkcnt = blkcnt;
-	b->blksz = blksz;
-	b->bufsize = blkcnt * blksz;
+		goto out;
 
-	tmpbuf = malloc(b->bufsize, M_DEVBUF, M_NOWAIT);
+	chn_unlock(b->channel);
+	tmpbuf = malloc(blkcnt * blksz, M_DEVBUF, M_WAITOK);
 	if (tmpbuf == NULL)
 		return ENOMEM;
-	free(b->tmpbuf, M_DEVBUF);
+	chn_lock(b->channel);
+	b->blkcnt = blkcnt;
+	b->blksz = blksz;
+	b->bufsize = blkcnt * blksz;
+	f2 =  b->tmpbuf;
 	b->tmpbuf = tmpbuf;
 	sndbuf_reset(b);
+	chn_unlock(b->channel);
+	free(f2, M_DEVBUF);
+	return 0;
+out:
+	chn_unlock(b->channel);
 	return 0;
 }
 
@@ -142,21 +154,27 @@ sndbuf_remalloc(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz)
 {
         u_int8_t *buf, *tmpbuf, *f1, *f2;
         unsigned int bufsize;
+	int ret;
 
 	if (blkcnt < 2 || blksz < 16)
 		return EINVAL;
 
 	bufsize = blksz * blkcnt;
 
-	buf = malloc(bufsize, M_DEVBUF, M_NOWAIT);
-	if (buf == NULL)
-		return ENOMEM;
+	chn_unlock(b->channel);
+	buf = malloc(bufsize, M_DEVBUF, M_WAITOK);
+	if (buf == NULL) {
+		ret = ENOMEM;
+		goto out;
+	}
 
-	tmpbuf = malloc(bufsize, M_DEVBUF, M_NOWAIT);
+	tmpbuf = malloc(bufsize, M_DEVBUF, M_WAITOK);
    	if (tmpbuf == NULL) {
 		free(buf, M_DEVBUF);
-		return ENOMEM;
+		ret = ENOMEM;
+		goto out;
 	}
+	chn_lock(b->channel);
 
 	b->blkcnt = blkcnt;
 	b->blksz = blksz;
@@ -167,13 +185,18 @@ sndbuf_remalloc(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz)
 	b->buf = buf;
 	b->tmpbuf = tmpbuf;
 
+	sndbuf_reset(b);
+
+	chn_unlock(b->channel);
       	if (f1)
 		free(f1, M_DEVBUF);
       	if (f2)
 		free(f2, M_DEVBUF);
 
-	sndbuf_reset(b);
-	return 0;
+	ret = 0;
+out:
+	chn_lock(b->channel);
+	return ret;
 }
 
 void
diff --git a/sys/dev/sound/pcm/buffer.h b/sys/dev/sound/pcm/buffer.h
index 31d9e2b..48981b5 100644
--- a/sys/dev/sound/pcm/buffer.h
+++ b/sys/dev/sound/pcm/buffer.h
@@ -53,10 +53,11 @@ struct snd_dbuf {
 	bus_dma_tag_t dmatag;
 	u_int32_t buf_addr;
 	struct selinfo sel;
+	struct pcm_channel *channel;
 	char name[SNDBUF_NAMELEN];
 };
 
-struct snd_dbuf *sndbuf_create(device_t dev, char *drv, char *desc);
+struct snd_dbuf *sndbuf_create(device_t dev, char *drv, char *desc, struct pcm_channel *channel);
 void sndbuf_destroy(struct snd_dbuf *b);
 
 void sndbuf_dump(struct snd_dbuf *b, char *s, u_int32_t what);
diff --git a/sys/dev/sound/pcm/channel.c b/sys/dev/sound/pcm/channel.c
index e5b0c30..610f69c 100644
--- a/sys/dev/sound/pcm/channel.c
+++ b/sys/dev/sound/pcm/channel.c
@@ -70,9 +70,9 @@ static void
 chn_lockinit(struct pcm_channel *c, int dir)
 {
 	if (dir == PCMDIR_PLAY)
-		c->lock = snd_mtxcreate(c->name, "pcm play channel");
+		c->lock = snd_chnmtxcreate(c->name, "pcm play channel");
 	else
-		c->lock = snd_mtxcreate(c->name, "pcm record channel");
+		c->lock = snd_chnmtxcreate(c->name, "pcm record channel");
 }
 
 static void
@@ -205,16 +205,19 @@ chn_wrfeed(struct pcm_channel *c)
 	unsigned int ret, amt;
 
 	CHN_LOCKASSERT(c);
-    	DEB(
+/*    	DEB(
 	if (c->flags & CHN_F_CLOSING) {
 		sndbuf_dump(b, "b", 0x02);
 		sndbuf_dump(bs, "bs", 0x02);
-	})
+	}) */
 
 	if (c->flags & CHN_F_MAPPED)
 		sndbuf_acquire(bs, NULL, sndbuf_getfree(bs));
 
 	amt = sndbuf_getfree(b);
+	KASSERT(amt <= sndbuf_getsize(bs),
+	    ("%s(%s): amt %d > source size %d, flags 0x%x", __func__, c->name,
+	   amt, sndbuf_getsize(bs), c->flags));
 	if (sndbuf_getready(bs) < amt)
 		c->xruns++;
 
@@ -746,6 +749,16 @@ chn_init(struct pcm_channel *c, void *devinfo, int dir)
 	c->devinfo = NULL;
 	c->feeder = NULL;
 
+	ret = ENOMEM;
+	b = sndbuf_create(c->dev, c->name, "primary", c);
+	if (b == NULL)
+		goto out;
+	bs = sndbuf_create(c->dev, c->name, "secondary", c);
+	if (bs == NULL)
+		goto out;
+
+	CHN_LOCK(c);
+
 	ret = EINVAL;
 	fc = feeder_getclass(NULL);
 	if (fc == NULL)
@@ -753,21 +766,23 @@ chn_init(struct pcm_channel *c, void *devinfo, int dir)
 	if (chn_addfeeder(c, fc, NULL))
 		goto out;
 
-	ret = ENOMEM;
-	b = sndbuf_create(c->dev, c->name, "primary");
-	if (b == NULL)
-		goto out;
-	bs = sndbuf_create(c->dev, c->name, "secondary");
-	if (bs == NULL)
-		goto out;
+	/*
+	 * XXX - sndbuf_setup() & sndbuf_resize() expect to be called
+	 *	 with the channel unlocked because they are also called
+	 *	 from driver methods that don't know about locking
+	 */
+	CHN_UNLOCK(c);
 	sndbuf_setup(bs, NULL, 0);
+	CHN_LOCK(c);
 	c->bufhard = b;
 	c->bufsoft = bs;
 	c->flags = 0;
 	c->feederflags = 0;
 
 	ret = ENODEV;
+	CHN_UNLOCK(c); /* XXX - Unlock for CHANNEL_INIT() malloc() call */
 	c->devinfo = CHANNEL_INIT(c->methods, devinfo, b, c, dir);
+	CHN_LOCK(c);
 	if (c->devinfo == NULL)
 		goto out;
 
@@ -789,6 +804,7 @@ chn_init(struct pcm_channel *c, void *devinfo, int dir)
 
 
 out:
+	CHN_UNLOCK(c);
 	if (ret) {
 		if (c->devinfo) {
 			if (CHANNEL_FREE(c->methods, c->devinfo))
@@ -971,11 +987,17 @@ chn_setblocksize(struct pcm_channel *c, int blkcnt, int blksz)
 {
 	struct snd_dbuf *b = c->bufhard;
 	struct snd_dbuf *bs = c->bufsoft;
-	int bufsz, irqhz, tmp, ret;
+	int irqhz, tmp, ret, maxsize, reqblksz, tmpblksz;
 
 	CHN_LOCKASSERT(c);
-	if (!CANCHANGE(c) || (c->flags & CHN_F_MAPPED))
+	if (!CANCHANGE(c) || (c->flags & CHN_F_MAPPED)) {
+		KASSERT(sndbuf_getsize(bs) ==  0 ||
+		    sndbuf_getsize(bs) >= sndbuf_getsize(b),
+		    ("%s(%s): bufsoft size %d < bufhard size %d", __func__,
+		    c->name, sndbuf_getsize(bs), sndbuf_getsize(b)));
 		return EINVAL;
+	}
+	c->flags |= CHN_F_SETBLOCKSIZE;
 
 	ret = 0;
 	DEB(printf("%s(%d, %d)\n", __func__, blkcnt, blksz));
@@ -1007,36 +1029,70 @@ chn_setblocksize(struct pcm_channel *c, int blkcnt, int blksz)
 		c->flags |= CHN_F_HAS_SIZE;
 	}
 
-	bufsz = blkcnt * blksz;
-
-	ret = sndbuf_remalloc(bs, blkcnt, blksz);
-	if (ret)
-		goto out;
+	reqblksz = blksz;
 
 	/* adjust for different hw format/speed */
-	irqhz = (sndbuf_getbps(bs) * sndbuf_getspd(bs)) / sndbuf_getblksz(bs);
+	irqhz = (sndbuf_getbps(bs) * sndbuf_getspd(bs)) / blksz;
 	DEB(printf("%s: soft bps %d, spd %d, irqhz == %d\n", __func__, sndbuf_getbps(bs), sndbuf_getspd(bs), irqhz));
 	RANGE(irqhz, 16, 512);
 
-	sndbuf_setblksz(b, (sndbuf_getbps(b) * sndbuf_getspd(b)) / irqhz);
+	tmpblksz = (sndbuf_getbps(b) * sndbuf_getspd(b)) / irqhz;
 
 	/* round down to 2^x */
 	blksz = 32;
-	while (blksz <= sndbuf_getblksz(b))
+	while (blksz <= tmpblksz)
 		blksz <<= 1;
 	blksz >>= 1;
 
 	/* round down to fit hw buffer size */
-	RANGE(blksz, 16, sndbuf_getmaxsize(b) / 2);
+	if (sndbuf_getmaxsize(b) > 0)
+		RANGE(blksz, 16, sndbuf_getmaxsize(b) / 2);
+	else
+		/* virtual channels don't appear to allocate bufhard */
+		RANGE(blksz, 16, CHN_2NDBUFMAXSIZE / 2);
 	DEB(printf("%s: hard blksz requested %d (maxsize %d), ", __func__, blksz, sndbuf_getmaxsize(b)));
 
+	/* Increase the size of bufsoft if before increasing bufhard. */
+	maxsize = sndbuf_getsize(b);
+	if (sndbuf_getsize(bs) > maxsize)
+		maxsize = sndbuf_getsize(bs);
+	if (reqblksz * blkcnt > maxsize)
+		maxsize = reqblksz * blkcnt;
+	if (sndbuf_getsize(bs) != maxsize || sndbuf_getblksz(bs) != reqblksz) {
+		ret = sndbuf_remalloc(bs, maxsize/reqblksz, reqblksz);
+		if (ret)
+			goto out1;
+	}
+
+	CHN_UNLOCK(c);
 	sndbuf_setblksz(b, CHANNEL_SETBLOCKSIZE(c->methods, c->devinfo, blksz));
+	CHN_LOCK(c);
+
+	/* Decrease the size of bufsoft after decreasing bufhard. */
+	maxsize = sndbuf_getsize(b);
+	if (reqblksz * blkcnt > maxsize)
+		maxsize = reqblksz * blkcnt;
+	if (maxsize > sndbuf_getsize(bs))
+		printf("Danger! %s bufsoft size increasing from %d to %d after CHANNEL_SETBLOCKSIZE()\n",
+		    c->name, sndbuf_getsize(bs), maxsize);
+	if (sndbuf_getsize(bs) != maxsize || sndbuf_getblksz(bs) != reqblksz) {
+		ret = sndbuf_remalloc(bs, maxsize/reqblksz, reqblksz);
+		if (ret)
+			goto out1;
+	}
 
 	irqhz = (sndbuf_getbps(b) * sndbuf_getspd(b)) / sndbuf_getblksz(b);
 	DEB(printf("got %d, irqhz == %d\n", sndbuf_getblksz(b), irqhz));
 
 	chn_resetbuf(c);
+out1:
+	KASSERT(sndbuf_getsize(bs) ==  0 ||
+	    sndbuf_getsize(bs) >= sndbuf_getsize(b),
+	    ("%s(%s): bufsoft size %d < bufhard size %d, reqblksz=%d blksz=%d maxsize=%d blkcnt=%d",
+	    __func__, c->name, sndbuf_getsize(bs), sndbuf_getsize(b), reqblksz,
+	    blksz, maxsize, blkcnt));
 out:
+	c->flags &= ~CHN_F_SETBLOCKSIZE;
 	return ret;
 }
 
@@ -1216,8 +1272,12 @@ chn_notify(struct pcm_channel *c, u_int32_t flags)
 	struct pcm_channel *child;
 	int run;
 
-	if (SLIST_EMPTY(&c->children))
+	CHN_LOCK(c);
+
+	if (SLIST_EMPTY(&c->children)) {
+		CHN_UNLOCK(c);
 		return ENODEV;
+	}
 
 	run = (c->flags & CHN_F_TRIGGERED)? 1 : 0;
 	/*
@@ -1253,8 +1313,10 @@ chn_notify(struct pcm_channel *c, u_int32_t flags)
 		blksz = sndbuf_getmaxsize(c->bufhard) / 2;
 		SLIST_FOREACH(pce, &c->children, link) {
 			child = pce->channel;
+			CHN_LOCK(child);
 			if (sndbuf_getblksz(child->bufhard) < blksz)
 				blksz = sndbuf_getblksz(child->bufhard);
+			CHN_UNLOCK(child);
 		}
 		chn_setblocksize(c, 2, blksz);
 	}
@@ -1268,13 +1330,28 @@ chn_notify(struct pcm_channel *c, u_int32_t flags)
 		nrun = 0;
 		SLIST_FOREACH(pce, &c->children, link) {
 			child = pce->channel;
+			CHN_LOCK(child);
 			if (child->flags & CHN_F_TRIGGERED)
 				nrun = 1;
+			CHN_UNLOCK(child);
 		}
 		if (nrun && !run)
 			chn_start(c, 1);
 		if (!nrun && run)
 			chn_abort(c);
 	}
+	CHN_UNLOCK(c);
 	return 0;
 }
+
+void
+chn_lock(struct pcm_channel *c)
+{
+	CHN_LOCK(c);
+}
+
+void
+chn_unlock(struct pcm_channel *c)
+{
+	CHN_UNLOCK(c);
+}
diff --git a/sys/dev/sound/pcm/channel.h b/sys/dev/sound/pcm/channel.h
index 57302f0..5a9492a 100644
--- a/sys/dev/sound/pcm/channel.h
+++ b/sys/dev/sound/pcm/channel.h
@@ -99,11 +99,13 @@ void chn_wrupdate(struct pcm_channel *c);
 void chn_rdupdate(struct pcm_channel *c);
 
 int chn_notify(struct pcm_channel *c, u_int32_t flags);
+void chn_lock(struct pcm_channel *c);
+void chn_unlock(struct pcm_channel *c);
 
 #ifdef	USING_MUTEX
 #define CHN_LOCK(c) mtx_lock((struct mtx *)((c)->lock))
 #define CHN_UNLOCK(c) mtx_unlock((struct mtx *)((c)->lock))
-#define CHN_LOCKASSERT(c)
+#define CHN_LOCKASSERT(c) mtx_assert((struct mtx *)((c)->lock), MA_OWNED)
 #else
 #define CHN_LOCK(c)
 #define CHN_UNLOCK(c)
@@ -134,6 +136,7 @@ int fmtvalid(u_int32_t fmt, u_int32_t *fmtlist);
 #define CHN_F_MAPPED		0x00010000  /* has been mmap()ed */
 #define CHN_F_DEAD		0x00020000
 #define CHN_F_BADSETTING	0x00040000
+#define CHN_F_SETBLOCKSIZE	0x00080000
 
 #define	CHN_F_VIRTUAL		0x10000000  /* not backed by hardware */
 
diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c
index 91fbb6d..cffbe6b 100644
--- a/sys/dev/sound/pcm/dsp.c
+++ b/sys/dev/sound/pcm/dsp.c
@@ -113,8 +113,8 @@ getchns(dev_t dev, struct pcm_channel **rdch, struct pcm_channel **wrch, u_int32
 
 	flags = dsp_get_flags(dev);
 	d = dsp_get_info(dev);
-	pcm_lock(d);
 	pcm_inprog(d, 1);
+	pcm_lock(d);
 	KASSERT((flags & SD_F_PRIO_SET) != SD_F_PRIO_SET, \
 		("getchns: read and write both prioritised"));
 
@@ -159,9 +159,7 @@ relchns(dev_t dev, struct pcm_channel *rdch, struct pcm_channel *wrch, u_int32_t
 		CHN_UNLOCK(wrch);
 	if (rdch && rdch != pcm_getfakechan(d) && (prio & SD_F_PRIO_RD))
 		CHN_UNLOCK(rdch);
-	pcm_lock(d);
 	pcm_inprog(d, -1);
-	pcm_unlock(d);
 }
 
 static int
@@ -476,6 +474,11 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 		wrch = NULL;
 	if (kill & 2)
 		rdch = NULL;
+	
+	if (rdch != NULL)
+		CHN_LOCK(rdch);
+	if (wrch != NULL)
+		CHN_LOCK(wrch);
 
     	switch(cmd) {
 #ifdef OLDPCM_IOCTL
@@ -497,16 +500,12 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 			p->play_size = 0;
 			p->rec_size = 0;
 	    		if (wrch) {
-				CHN_LOCK(wrch);
 				chn_setblocksize(wrch, 2, p->play_size);
 				p->play_size = sndbuf_getblksz(wrch->bufsoft);
-				CHN_UNLOCK(wrch);
 			}
 	    		if (rdch) {
-				CHN_LOCK(rdch);
 				chn_setblocksize(rdch, 2, p->rec_size);
 				p->rec_size = sndbuf_getblksz(rdch->bufsoft);
-				CHN_UNLOCK(rdch);
 			}
 		}
 		break;
@@ -526,16 +525,12 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 	    		snd_chan_param *p = (snd_chan_param *)arg;
 
 	    		if (wrch) {
-				CHN_LOCK(wrch);
 				chn_setformat(wrch, p->play_format);
 				chn_setspeed(wrch, p->play_rate);
-				CHN_UNLOCK(wrch);
 	    		}
 	    		if (rdch) {
-				CHN_LOCK(rdch);
 				chn_setformat(rdch, p->rec_format);
 				chn_setspeed(rdch, p->rec_rate);
-				CHN_UNLOCK(rdch);
 	    		}
 		}
 		/* FALLTHROUGH */
@@ -557,14 +552,10 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 			struct pcmchan_caps *pcaps = NULL, *rcaps = NULL;
 			dev_t pdev;
 
-			if (rdch) {
-				CHN_LOCK(rdch);
+			if (rdch)
 				rcaps = chn_getcaps(rdch);
-			}
-			if (wrch) {
-				CHN_LOCK(wrch);
+			if (wrch)
 				pcaps = chn_getcaps(wrch);
-			}
 	    		p->rate_min = max(rcaps? rcaps->minspeed : 0,
 	                      		  pcaps? pcaps->minspeed : 0);
 	    		p->rate_max = min(rcaps? rcaps->maxspeed : 1000000,
@@ -580,10 +571,6 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 	    		p->mixers = 1; /* default: one mixer */
 	    		p->inputs = pdev->si_drv1? mix_getdevs(pdev->si_drv1) : 0;
 	    		p->left = p->right = 100;
-			if (wrch)
-				CHN_UNLOCK(wrch);
-			if (rdch)
-				CHN_UNLOCK(rdch);
 		}
 		break;
 
@@ -646,59 +633,42 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 
     	case SNDCTL_DSP_SETBLKSIZE:
 		RANGE(*arg_i, 16, 65536);
-		if (wrch) {
-			CHN_LOCK(wrch);
+		if (wrch)
 			chn_setblocksize(wrch, 2, *arg_i);
-			CHN_UNLOCK(wrch);
-		}
-		if (rdch) {
-			CHN_LOCK(rdch);
+		if (rdch)
 			chn_setblocksize(rdch, 2, *arg_i);
-			CHN_UNLOCK(rdch);
-		}
 		break;
 
     	case SNDCTL_DSP_RESET:
 		DEB(printf("dsp reset\n"));
 		if (wrch) {
-			CHN_LOCK(wrch);
 			chn_abort(wrch);
 			chn_resetbuf(wrch);
-			CHN_UNLOCK(wrch);
 		}
 		if (rdch) {
-			CHN_LOCK(rdch);
 			chn_abort(rdch);
 			chn_resetbuf(rdch);
-			CHN_UNLOCK(rdch);
 		}
 		break;
 
     	case SNDCTL_DSP_SYNC:
 		DEB(printf("dsp sync\n"));
 		/* chn_sync may sleep */
-		if (wrch) {
-			CHN_LOCK(wrch);
+		if (wrch)
 			chn_sync(wrch, sndbuf_getsize(wrch->bufsoft) - 4);
-			CHN_UNLOCK(wrch);
-		}
 		break;
 
     	case SNDCTL_DSP_SPEED:
 		/* chn_setspeed may sleep */
 		tmp = 0;
 		if (wrch) {
-			CHN_LOCK(wrch);
 			ret = chn_setspeed(wrch, *arg_i);
 			tmp = wrch->speed;
-			CHN_UNLOCK(wrch);
 		}
 		if (rdch && ret == 0) {
-			CHN_LOCK(rdch);
 			ret = chn_setspeed(rdch, *arg_i);
 			if (tmp == 0)
 				tmp = rdch->speed;
-			CHN_UNLOCK(rdch);
 		}
 		*arg_i = tmp;
 		break;
@@ -711,17 +681,13 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 		tmp = -1;
 		*arg_i = (*arg_i)? AFMT_STEREO : 0;
 		if (wrch) {
-			CHN_LOCK(wrch);
 			ret = chn_setformat(wrch, (wrch->format & ~AFMT_STEREO) | *arg_i);
 			tmp = (wrch->format & AFMT_STEREO)? 1 : 0;
-			CHN_UNLOCK(wrch);
 		}
 		if (rdch && ret == 0) {
-			CHN_LOCK(rdch);
 			ret = chn_setformat(rdch, (rdch->format & ~AFMT_STEREO) | *arg_i);
 			if (tmp == -1)
 				tmp = (rdch->format & AFMT_STEREO)? 1 : 0;
-			CHN_UNLOCK(rdch);
 		}
 		*arg_i = tmp;
 		break;
@@ -732,22 +698,17 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 			tmp = 0;
 			*arg_i = (*arg_i != 1)? AFMT_STEREO : 0;
 	  		if (wrch) {
-				CHN_LOCK(wrch);
 				ret = chn_setformat(wrch, (wrch->format & ~AFMT_STEREO) | *arg_i);
 				tmp = (wrch->format & AFMT_STEREO)? 2 : 1;
-				CHN_UNLOCK(wrch);
 			}
 			if (rdch && ret == 0) {
-				CHN_LOCK(rdch);
 				ret = chn_setformat(rdch, (rdch->format & ~AFMT_STEREO) | *arg_i);
 				if (tmp == 0)
 					tmp = (rdch->format & AFMT_STEREO)? 2 : 1;
-				CHN_UNLOCK(rdch);
 			}
 			*arg_i = tmp;
-		} else {
+		} else
 			*arg_i = ((wrch? wrch->format : rdch->format) & AFMT_STEREO)? 2 : 1;
-		}
 		break;
 
     	case SOUND_PCM_READ_CHANNELS:
@@ -763,17 +724,13 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 		if ((*arg_i != AFMT_QUERY)) {
 			tmp = 0;
 			if (wrch) {
-				CHN_LOCK(wrch);
 				ret = chn_setformat(wrch, (*arg_i) | (wrch->format & AFMT_STEREO));
 				tmp = wrch->format & ~AFMT_STEREO;
-				CHN_UNLOCK(wrch);
 			}
 			if (rdch && ret == 0) {
-				CHN_LOCK(rdch);
 				ret = chn_setformat(rdch, (*arg_i) | (rdch->format & AFMT_STEREO));
 				if (tmp == 0)
 					tmp = rdch->format & ~AFMT_STEREO;
-				CHN_UNLOCK(rdch);
 			}
 			*arg_i = tmp;
 		} else
@@ -800,18 +757,14 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 
 			DEB(printf("SNDCTL_DSP_SETFRAGMENT %d frags, %d sz\n", maxfrags, fragsz));
 		    	if (rdch) {
-				CHN_LOCK(rdch);
 				ret = chn_setblocksize(rdch, maxfrags, fragsz);
 				maxfrags = sndbuf_getblkcnt(rdch->bufsoft);
 				fragsz = sndbuf_getblksz(rdch->bufsoft);
-				CHN_UNLOCK(rdch);
 			}
 		    	if (wrch && ret == 0) {
-				CHN_LOCK(wrch);
 				ret = chn_setblocksize(wrch, maxfrags, fragsz);
  				maxfrags = sndbuf_getblkcnt(wrch->bufsoft);
 				fragsz = sndbuf_getblksz(wrch->bufsoft);
-				CHN_UNLOCK(wrch);
 			}
 
 			fragln = 0;
@@ -830,12 +783,10 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 	    		if (rdch) {
 	        		struct snd_dbuf *bs = rdch->bufsoft;
 
-				CHN_LOCK(rdch);
 				a->bytes = sndbuf_getready(bs);
 	        		a->fragments = a->bytes / sndbuf_getblksz(bs);
 	        		a->fragstotal = sndbuf_getblkcnt(bs);
 	        		a->fragsize = sndbuf_getblksz(bs);
-				CHN_UNLOCK(rdch);
 	    		}
 		}
 		break;
@@ -847,13 +798,11 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 	    		if (wrch) {
 	        		struct snd_dbuf *bs = wrch->bufsoft;
 
-				CHN_LOCK(wrch);
 				chn_wrupdate(wrch);
 				a->bytes = sndbuf_getfree(bs);
 	        		a->fragments = a->bytes / sndbuf_getblksz(bs);
 	        		a->fragstotal = sndbuf_getblkcnt(bs);
 	        		a->fragsize = sndbuf_getblksz(bs);
-				CHN_UNLOCK(wrch);
 	    		}
 		}
 		break;
@@ -864,13 +813,11 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 	    		if (rdch) {
 	        		struct snd_dbuf *bs = rdch->bufsoft;
 
-				CHN_LOCK(rdch);
 				chn_rdupdate(rdch);
 	        		a->bytes = sndbuf_gettotal(bs);
 	        		a->blocks = sndbuf_getblocks(bs) - rdch->blocks;
 	        		a->ptr = sndbuf_getreadyptr(bs);
 				rdch->blocks = sndbuf_getblocks(bs);
-				CHN_UNLOCK(rdch);
 	    		} else
 				ret = EINVAL;
 		}
@@ -882,13 +829,11 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 	    		if (wrch) {
 	        		struct snd_dbuf *bs = wrch->bufsoft;
 
-				CHN_LOCK(wrch);
 				chn_wrupdate(wrch);
 	        		a->bytes = sndbuf_gettotal(bs);
 	        		a->blocks = sndbuf_getblocks(bs) - wrch->blocks;
 	        		a->ptr = sndbuf_getreadyptr(bs);
 				wrch->blocks = sndbuf_getblocks(bs);
-				CHN_UNLOCK(wrch);
 	    		} else
 				ret = EINVAL;
 		}
@@ -906,22 +851,18 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 
     	case SNDCTL_DSP_SETTRIGGER:
 		if (rdch) {
-			CHN_LOCK(rdch);
 			rdch->flags &= ~(CHN_F_TRIGGERED | CHN_F_NOTRIGGER);
 		    	if (*arg_i & PCM_ENABLE_INPUT)
 				chn_start(rdch, 1);
 			else
 				rdch->flags |= CHN_F_NOTRIGGER;
-			CHN_UNLOCK(rdch);
 		}
 		if (wrch) {
-			CHN_LOCK(wrch);
 			wrch->flags &= ~(CHN_F_TRIGGERED | CHN_F_NOTRIGGER);
 		    	if (*arg_i & PCM_ENABLE_OUTPUT)
 				chn_start(wrch, 1);
 			else
 				wrch->flags |= CHN_F_NOTRIGGER;
-		 	CHN_UNLOCK(wrch);
 		}
 		break;
 
@@ -938,20 +879,16 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 			struct snd_dbuf *b = wrch->bufhard;
 	        	struct snd_dbuf *bs = wrch->bufsoft;
 
-			CHN_LOCK(wrch);
 			chn_wrupdate(wrch);
 			*arg_i = sndbuf_getready(b) + sndbuf_getready(bs);
-			CHN_UNLOCK(wrch);
 		} else
 			ret = EINVAL;
 		break;
 
     	case SNDCTL_DSP_POST:
 		if (wrch) {
-			CHN_LOCK(wrch);
 			wrch->flags &= ~CHN_F_NOTRIGGER;
 			chn_start(wrch, 1);
-			CHN_UNLOCK(wrch);
 		}
 		break;
 
@@ -969,6 +906,10 @@ dsp_ioctl(dev_t i_dev, u_long cmd, caddr_t arg, int mode, struct thread *td)
 		ret = EINVAL;
 		break;
     	}
+	if (rdch != NULL)
+		CHN_UNLOCK(rdch);
+	if (wrch != NULL)
+		CHN_UNLOCK(wrch);
 	relchns(i_dev, rdch, wrch, 0);
 	splx(s);
     	return ret;
diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index 3ba4636..edb765e 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -75,7 +75,23 @@ snd_mtxcreate(const char *desc, const char *type)
 	m = malloc(sizeof(*m), M_DEVBUF, M_WAITOK | M_ZERO);
 	if (m == NULL)
 		return NULL;
-	mtx_init(m, desc, type, MTX_DEF | MTX_RECURSE);
+	mtx_init(m, desc, type, MTX_DEF);
+	return m;
+#else
+	return (void *)0xcafebabe;
+#endif
+}
+
+void *
+snd_chnmtxcreate(const char *desc, const char *type)
+{
+#ifdef USING_MUTEX
+	struct mtx *m;
+
+	m = malloc(sizeof(*m), M_DEVBUF, M_WAITOK | M_ZERO);
+	if (m == NULL)
+		return NULL;
+	mtx_init(m, desc, type, MTX_DEF | MTX_DUPOK);
 	return m;
 #else
 	return (void *)0xcafebabe;
@@ -188,13 +204,16 @@ pcm_chnalloc(struct snddev_info *d, int direction, pid_t pid, int chnum)
 			/* try to create a vchan */
 			SLIST_FOREACH(sce, &d->channels, link) {
 				c = sce->channel;
+				CHN_LOCK(c);
 				if (!SLIST_EMPTY(&c->children)) {
 					err = vchan_create(c);
+					CHN_UNLOCK(c);
 					if (!err)
 						return pcm_chnalloc(d, direction, pid, -1);
 					else
 						device_printf(d->dev, "vchan_create(%s) == %d\n", c->name, err);
-				}
+				} else
+					CHN_UNLOCK(c);
 			}
 		}
 	}
@@ -250,15 +269,19 @@ pcm_setmaxautovchans(struct snddev_info *d, int num)
 	if (num > 0 && d->vchancount == 0) {
 		SLIST_FOREACH(sce, &d->channels, link) {
 			c = sce->channel;
+			CHN_LOCK(c);
 			if ((c->direction == PCMDIR_PLAY) && !(c->flags & CHN_F_BUSY)) {
 				c->flags |= CHN_F_BUSY;
 				err = vchan_create(c);
 				if (err) {
-					device_printf(d->dev, "vchan_create(%s) == %d\n", c->name, err);
 					c->flags &= ~CHN_F_BUSY;
-				}
+					CHN_UNLOCK(c);
+					device_printf(d->dev, "vchan_create(%s) == %d\n", c->name, err);
+				} else
+					CHN_UNLOCK(c);
 				return;
 			}
+			CHN_UNLOCK(c);
 		}
 	}
 	if (num == 0 && d->vchancount > 0) {
@@ -313,7 +336,7 @@ sysctl_hw_snd_maxautovchans(SYSCTL_HANDLER_ARGS)
 	v = snd_maxautovchans;
 	error = sysctl_handle_int(oidp, &v, sizeof(v), req);
 	if (error == 0 && req->newptr != NULL) {
-		if (v < 0 || v >= SND_MAXVCHANS)
+		if (v < 0 || v >= SND_MAXVCHANS || pcm_devclass == NULL)
 			return EINVAL;
 		if (v != snd_maxautovchans) {
 			for (i = 0; i < devclass_get_maxunit(pcm_devclass); i++) {
@@ -529,20 +552,23 @@ pcm_addchan(device_t dev, int dir, kobj_class_t cls, void *devinfo)
 	err = pcm_chn_add(d, ch);
 	if (err) {
 		device_printf(d->dev, "pcm_chn_add(%s) failed, err=%d\n", ch->name, err);
-		snd_mtxunlock(d->lock);
 		pcm_chn_destroy(ch);
 		return err;
 	}
 
+	CHN_LOCK(ch);
 	if (snd_maxautovchans > 0 && (d->flags & SD_F_AUTOVCHAN) &&
 	    ch->direction == PCMDIR_PLAY && d->vchancount == 0) {
 		ch->flags |= CHN_F_BUSY;
 		err = vchan_create(ch);
 		if (err) {
-			device_printf(d->dev, "vchan_create(%s) == %d\n", ch->name, err);
 			ch->flags &= ~CHN_F_BUSY;
+			CHN_UNLOCK(ch);
+			device_printf(d->dev, "vchan_create(%s) == %d\n", ch->name, err);
+			return err;
 		}
 	}
+	CHN_UNLOCK(ch);
 
 	return err;
 }
@@ -866,11 +892,13 @@ sysctl_hw_snd_vchans(SYSCTL_HANDLER_ARGS)
 	cnt = 0;
 	SLIST_FOREACH(sce, &d->channels, link) {
 		c = sce->channel;
+		CHN_LOCK(c);
 		if ((c->direction == PCMDIR_PLAY) && (c->flags & CHN_F_VIRTUAL)) {
 			cnt++;
 			if (c->flags & CHN_F_BUSY)
 				busy++;
 		}
+		CHN_UNLOCK(c);
 	}
 
 	newcnt = cnt;
@@ -888,23 +916,28 @@ sysctl_hw_snd_vchans(SYSCTL_HANDLER_ARGS)
 			/* add new vchans - find a parent channel first */
 			SLIST_FOREACH(sce, &d->channels, link) {
 				c = sce->channel;
+				CHN_LOCK(c);
 				/* not a candidate if not a play channel */
 				if (c->direction != PCMDIR_PLAY)
-					continue;
+					goto next;
 				/* not a candidate if a virtual channel */
 				if (c->flags & CHN_F_VIRTUAL)
-					continue;
+					goto next;
 				/* not a candidate if it's in use */
-				if ((c->flags & CHN_F_BUSY) && (SLIST_EMPTY(&c->children)))
-					continue;
-				/*
-				 * if we get here we're a nonvirtual play channel, and either
-				 * 1) not busy
-				 * 2) busy with children, not directly open
-				 *
-				 * thus we can add children
-				 */
-				goto addok;
+				if (!(c->flags & CHN_F_BUSY) ||
+				    !(SLIST_EMPTY(&c->children)))
+					/*
+					 * if we get here we're a nonvirtual
+					 * play channel, and either
+					 * 1) not busy
+					 * 2) busy with children, not directly
+					 *    open
+					 *
+					 * thus we can add children
+					 */
+					goto addok;
+next:
+				CHN_UNLOCK(c);
 			}
 			pcm_inprog(d, -1);
 			return EBUSY;
@@ -917,6 +950,7 @@ addok:
 			}
 			if (SLIST_EMPTY(&c->children))
 				c->flags &= ~CHN_F_BUSY;
+			CHN_UNLOCK(c);
 		} else if (newcnt < cnt) {
 			if (busy > newcnt) {
 				printf("cnt %d, newcnt %d, busy %d\n", cnt, newcnt, busy);
@@ -928,13 +962,17 @@ addok:
 			while (err == 0 && newcnt < cnt) {
 				SLIST_FOREACH(sce, &d->channels, link) {
 					c = sce->channel;
+					CHN_LOCK(c);
 					if ((c->flags & (CHN_F_BUSY | CHN_F_VIRTUAL)) == CHN_F_VIRTUAL)
 						goto remok;
+
+					CHN_UNLOCK(c);
 				}
 				snd_mtxunlock(d->lock);
 				pcm_inprog(d, -1);
 				return EINVAL;
 remok:
+				CHN_UNLOCK(c);
 				err = vchan_destroy(c);
 				if (err == 0)
 					cnt--;
diff --git a/sys/dev/sound/pcm/sound.h b/sys/dev/sound/pcm/sound.h
index 29ddd44..aea9dc5 100644
--- a/sys/dev/sound/pcm/sound.h
+++ b/sys/dev/sound/pcm/sound.h
@@ -238,6 +238,7 @@ int snd_setup_intr(device_t dev, struct resource *res, int flags,
 		   driver_intr_t hand, void *param, void **cookiep);
 
 void *snd_mtxcreate(const char *desc, const char *type);
+void *snd_chnmtxcreate(const char *desc, const char *type);
 void snd_mtxfree(void *m);
 void snd_mtxassert(void *m);
 #define	snd_mtxlock(m) mtx_lock(m)
diff --git a/sys/dev/sound/pcm/vchan.c b/sys/dev/sound/pcm/vchan.c
index 0c5867b..ce67fb1 100644
--- a/sys/dev/sound/pcm/vchan.c
+++ b/sys/dev/sound/pcm/vchan.c
@@ -77,7 +77,9 @@ feed_vchan_s16(struct pcm_feeder *f, struct pcm_channel *c, u_int8_t *b, u_int32
 	int16_t *tmp, *dst;
 	unsigned int cnt;
 
-	KASSERT(sndbuf_getsize(src) >= count, ("bad bufsize"));
+	if (sndbuf_getsize(src) < count)
+		panic("feed_vchan_s16(%s): tmp buffer size %d < count %d, flags = 0x%x",
+		    c->name, sndbuf_getsize(src), count, c->flags);
 	count &= ~1;
 	bzero(b, count);
 
@@ -92,12 +94,14 @@ feed_vchan_s16(struct pcm_feeder *f, struct pcm_channel *c, u_int8_t *b, u_int32
 	bzero(tmp, count);
 	SLIST_FOREACH(cce, &c->children, link) {
 		ch = cce->channel;
+   		CHN_LOCK(ch);
 		if (ch->flags & CHN_F_TRIGGERED) {
 			if (ch->flags & CHN_F_MAPPED)
 				sndbuf_acquire(ch->bufsoft, NULL, sndbuf_getfree(ch->bufsoft));
 			cnt = FEEDER_FEED(ch->feeder, ch, (u_int8_t *)tmp, count, ch->bufsoft);
 			vchan_mix_s16(dst, tmp, cnt / 2);
 		}
+   		CHN_UNLOCK(ch);
 	}
 
 	return count;
@@ -145,13 +149,16 @@ vchan_setformat(kobj_t obj, void *data, u_int32_t format)
 {
 	struct vchinfo *ch = data;
 	struct pcm_channel *parent = ch->parent;
+	struct pcm_channel *channel = ch->channel;
 
 	ch->fmt = format;
 	ch->bps = 1;
 	ch->bps <<= (ch->fmt & AFMT_STEREO)? 1 : 0;
 	ch->bps <<= (ch->fmt & AFMT_16BIT)? 1 : 0;
 	ch->bps <<= (ch->fmt & AFMT_32BIT)? 2 : 0;
+   	CHN_UNLOCK(channel);
 	chn_notify(parent, CHN_N_FORMAT);
+   	CHN_LOCK(channel);
 	return 0;
 }
 
@@ -160,9 +167,12 @@ vchan_setspeed(kobj_t obj, void *data, u_int32_t speed)
 {
 	struct vchinfo *ch = data;
 	struct pcm_channel *parent = ch->parent;
+	struct pcm_channel *channel = ch->channel;
 
 	ch->spd = speed;
+   	CHN_UNLOCK(channel);
 	chn_notify(parent, CHN_N_RATE);
+   	CHN_LOCK(channel);
 	return speed;
 }
 
@@ -171,14 +181,19 @@ vchan_setblocksize(kobj_t obj, void *data, u_int32_t blocksize)
 {
 	struct vchinfo *ch = data;
 	struct pcm_channel *parent = ch->parent;
+	/* struct pcm_channel *channel = ch->channel; */
 	int prate, crate;
 
 	ch->blksz = blocksize;
+   	/* CHN_UNLOCK(channel); */
 	chn_notify(parent, CHN_N_BLOCKSIZE);
+   	CHN_LOCK(parent);
+   	/* CHN_LOCK(channel); */
 
 	crate = ch->spd * ch->bps;
 	prate = sndbuf_getspd(parent->bufhard) * sndbuf_getbps(parent->bufhard);
 	blocksize = sndbuf_getblksz(parent->bufhard);
+   	CHN_UNLOCK(parent);
 	blocksize *= prate;
 	blocksize /= crate;
 
@@ -190,12 +205,15 @@ vchan_trigger(kobj_t obj, void *data, int go)
 {
 	struct vchinfo *ch = data;
 	struct pcm_channel *parent = ch->parent;
+	struct pcm_channel *channel = ch->channel;
 
 	if (go == PCMTRIG_EMLDMAWR || go == PCMTRIG_EMLDMARD)
 		return 0;
 
 	ch->run = (go == PCMTRIG_START)? 1 : 0;
+   	CHN_UNLOCK(channel);
 	chn_notify(parent, CHN_N_TRIGGER);
+   	CHN_LOCK(channel);
 
 	return 0;
 }
@@ -235,8 +253,11 @@ vchan_create(struct pcm_channel *parent)
 	struct pcm_channel *child;
 	int err, first;
 
+   	CHN_UNLOCK(parent);
+
 	pce = malloc(sizeof(*pce), M_DEVBUF, M_WAITOK | M_ZERO);
 	if (!pce) {
+   		CHN_LOCK(parent);
 		return ENOMEM;
 	}
 
@@ -244,14 +265,13 @@ vchan_create(struct pcm_channel *parent)
 	child = pcm_chn_create(d, parent, &vchan_class, PCMDIR_VIRTUAL, parent);
 	if (!child) {
 		free(pce, M_DEVBUF);
+   		CHN_LOCK(parent);
 		return ENODEV;
 	}
 
    	CHN_LOCK(parent);
-	if (!(parent->flags & CHN_F_BUSY)) {
-		CHN_UNLOCK(parent);
+	if (!(parent->flags & CHN_F_BUSY))
 		return EBUSY;
-	}
 
 	first = SLIST_EMPTY(&parent->children);
 	/* add us to our parent channel's children */
@@ -269,6 +289,7 @@ vchan_create(struct pcm_channel *parent)
 		free(pce, M_DEVBUF);
 	}
 
+   	CHN_LOCK(parent);
 	/* XXX gross ugly hack, murder death kill */
 	if (first && !err) {
 		err = chn_reset(parent, AFMT_STEREO | AFMT_S16_LE);