From 5317480967bfc8bf678e4da3fce81bcb3f5b7836 Mon Sep 17 00:00:00 2001
From: Christos Margiolis <christos@FreeBSD.org>
Date: Tue, 26 Nov 2024 15:48:36 +0100
Subject: [PATCH] sound: Remove CHN_F_SLEEPING

The KASSERT in chn_sleep() can be triggered if more than one thread
wants to sleep on a given channel at the same time. While this is not
really a common scenario, tools such as stress2, which use fork() and
the child process(es) inherit the parent's FDs as a result, we can end
up triggering such scenarios.

Fix this by removing CHN_F_SLEEPING altogether, which is not very useful
in the first place:
- CHN_BROADCAST() checks cv_waiters already, so there is no need to
  check CHN_F_SLEEPING as well.
- We can check whether cv_waiters is 0 in pcm_killchans(), instead of
  whether CHN_F_SLEEPING is not set.

Reported by:	dougm, pho (stress2)
Sponsored by:	The FreeBSD Foundation
MFC after:	2 days
Reviewed by:	dev_submerge.ch, markj
Differential Revision:	https://reviews.freebsd.org/D47559
---
 sys/dev/sound/pcm/channel.c | 13 +------------
 sys/dev/sound/pcm/channel.h |  4 ++--
 sys/dev/sound/pcm/sound.c   |  4 ++--
 3 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/sys/dev/sound/pcm/channel.c b/sys/dev/sound/pcm/channel.c
index 59cdb8cd07f5..f619ddd4bc35 100644
--- a/sys/dev/sound/pcm/channel.c
+++ b/sys/dev/sound/pcm/channel.c
@@ -309,14 +309,7 @@ chn_wakeup(struct pcm_channel *c)
 	if (CHN_EMPTY(c, children.busy)) {
 		if (SEL_WAITING(sndbuf_getsel(bs)) && chn_polltrigger(c))
 			selwakeuppri(sndbuf_getsel(bs), PRIBIO);
-		if (c->flags & CHN_F_SLEEPING) {
-			/*
-			 * Ok, I can just panic it right here since it is
-			 * quite obvious that we never allow multiple waiters
-			 * from userland. I'm too generous...
-			 */
-			CHN_BROADCAST(&c->intr_cv);
-		}
+		CHN_BROADCAST(&c->intr_cv);
 	} else {
 		CHN_FOREACH(ch, c, children.busy) {
 			CHN_LOCK(ch);
@@ -332,15 +325,11 @@ chn_sleep(struct pcm_channel *c, int timeout)
 	int ret;
 
 	CHN_LOCKASSERT(c);
-	KASSERT((c->flags & CHN_F_SLEEPING) == 0,
-	    ("%s(): entered with CHN_F_SLEEPING", __func__));
 
 	if (c->flags & CHN_F_DEAD)
 		return (EINVAL);
 
-	c->flags |= CHN_F_SLEEPING;
 	ret = cv_timedwait_sig(&c->intr_cv, c->lock, timeout);
-	c->flags &= ~CHN_F_SLEEPING;
 
 	return ((c->flags & CHN_F_DEAD) ? EINVAL : ret);
 }
diff --git a/sys/dev/sound/pcm/channel.h b/sys/dev/sound/pcm/channel.h
index 79a8d35b22f7..d226adfba06b 100644
--- a/sys/dev/sound/pcm/channel.h
+++ b/sys/dev/sound/pcm/channel.h
@@ -354,7 +354,7 @@ enum {
 #define CHN_F_RUNNING		0x00000004  /* dma is running */
 #define CHN_F_TRIGGERED		0x00000008
 #define CHN_F_NOTRIGGER		0x00000010
-#define CHN_F_SLEEPING		0x00000020
+/* unused			0x00000020 */
 
 #define CHN_F_NBIO              0x00000040  /* do non-blocking i/o */
 #define CHN_F_MMAP		0x00000080  /* has been mmap()ed */
@@ -381,8 +381,8 @@ enum {
 				"\002ABORTING"				\
 				"\003RUNNING"				\
 				"\004TRIGGERED"				\
+				/* \006 */				\
 				"\005NOTRIGGER"				\
-				"\006SLEEPING"				\
 				"\007NBIO"				\
 				"\010MMAP"				\
 				"\011BUSY"				\
diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index 4b7bcc64397f..218147b73db0 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -221,8 +221,8 @@ pcm_killchans(struct snddev_info *d)
 		/* Make sure all channels are stopped. */
 		CHN_FOREACH(ch, d, channels.pcm) {
 			CHN_LOCK(ch);
-			if ((ch->flags & CHN_F_SLEEPING) == 0 &&
-			    CHN_STOPPED(ch) && ch->inprog == 0) {
+			if (ch->intr_cv.cv_waiters == 0 && CHN_STOPPED(ch) &&
+			    ch->inprog == 0) {
 				CHN_UNLOCK(ch);
 				continue;
 			}