From nobody Mon Apr 29 10:43:45 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4VSg0y2xpNz5J3P7; Mon, 29 Apr 2024 10:43:46 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4VSg0y0scYz4pQ5; Mon, 29 Apr 2024 10:43:46 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1714387426; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=8Y2QaOigreXUSI9HzxUW450qJU+6gQGe5OFH9oiGALc=; b=Boc9huRVApRzWqiE5p2hUUJ7j1yiCI5qu7Omf+deCRPQUNhHgW5MW9cKSy26BrwIJEVW+w ITZRpDP0DHRb7xRTnjrLZMdxq4Jl93b3pq4zvreDpigbe9o6tSqnhIeNL/LgVUZlJQ/k8c t6Me8XeJXvvNerSRWCTM979iqMBP4i1nvQA5AvciMxfPsuuQ8WqaOHwYnVd1LEm1MSWsgD i88pADHTkmlWvX/ldAIABUArv6EIH0QoAPnFNo6R7cgOOheiSb3WztsuNcVZctUb4qZFp0 s2dJd3kUDjD7+9crCqWzPa/xl8aPDvHOYkBr3BsF6pWsIvo1jmKpxsM6jngsWg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1714387426; a=rsa-sha256; cv=none; b=wTMO9Qj2ye0D6CHQZv8nan0KhhbHxZyYkixUB9xKsP70Y6Tdy++XZDHYkFlsmyWFPTNpOZ QlrPVxj7kNKsBIWCDcQT8P+P8vDcieLAkcQy6iww9PrPrJoHVptGpMPP51FpD7hTjKUUSx HBqgVL/V+qr57AAeByhnAjmiP/jlEpVzmTtMptFq6fEI0IokDFD38E4j/c+M6nzmZB6bfj OBI5dOH5ztEjigjAueOIQueIYJZks/Styf1Z6bQLqdsO+71kSAf8KbdE/t7mpVodmfvmeT flmjDKqAJMsYCMFccr7AoDSGM8STogd8MetlZtCaxvyV8cOGquVnJL+PwOy18Q== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1714387426; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=8Y2QaOigreXUSI9HzxUW450qJU+6gQGe5OFH9oiGALc=; b=WMvi34JyKsTplMQ7Vd/c/m4oxCqlsRJSYoaeP0aPxAGp9W8uGLNHgM8QwrFWigBpjNDhcn xcWcflTj47Zzxlf9gvB6qCDAUAldI7CP1+Z0nwEAfjJ7o7yyCSOnvo8nBJYddiqjJNShbI j+TlgLxt/jUILxguxzNOJ66x27Nurp5wVu6WPwBP8LpdC3hLIXpJW1OaBzoivxaXwCJVzW CHKbxXdElVqH/rTdM+Pa/PAWX52BVERVovKcCMDNRL2IQ/suizdjklpbsoh3PTDFNqQAnH qQrbOX1JjxYJ1Ym6wQ5iafiv3jUbtO9UV5SkGzj+VAZ85OU6r2IjWRJPWiwN3g== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4VSg0y0SZ8z159j; Mon, 29 Apr 2024 10:43:46 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 43TAhjxR088439; Mon, 29 Apr 2024 10:43:45 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 43TAhjql088436; Mon, 29 Apr 2024 10:43:45 GMT (envelope-from git) Date: Mon, 29 Apr 2024 10:43:45 GMT Message-Id: <202404291043.43TAhjql088436@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Christos Margiolis Subject: git: d8d7907826cc - stable/14 - sound: Fix panic caused by sleeping-channel destruction during asynchronous detach List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: christos X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: d8d7907826cc0799d29b9ee9c23688704f619e86 Auto-Submitted: auto-generated The branch stable/14 has been updated by christos: URL: https://cgit.FreeBSD.org/src/commit/?id=d8d7907826cc0799d29b9ee9c23688704f619e86 commit d8d7907826cc0799d29b9ee9c23688704f619e86 Author: Christos Margiolis AuthorDate: 2024-04-28 19:40:40 +0000 Commit: Christos Margiolis CommitDate: 2024-04-29 10:43:30 +0000 sound: Fix panic caused by sleeping-channel destruction during asynchronous detach Currently we are force-destroying all channels unconditionally in pcm_killchan(). However, since asynchronous audio device detach is possible as of 44e128fe9d92, if we do not check whether the channel is sleeping or not and forcefully kill it, we will get a panic from cv_timedwait_sig() (called from chn_sleep()), because it will try to use a freed lock/cv. Modify pcm_killchan() (renamed to pcm_killchans() since that's a more appropriate name now) to loop through the channel list and destroy only the channels that are awake, otherwise wake up the sleeping thread and try again. This loop is repeated until all channels are awakened and destroyed. To reduce code duplication, implement chn_shutdown() which wakes up the channel and sets CHN_F_DEAD, and use it in pcm_unregister() and pcm_killchans(). Reported by: KASAN Fixes: 44e128fe9d92 ("sound: Implement asynchronous device detach") Sponsored by: The FreeBSD Foundation MFC after: 1 day Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D44923 (cherry picked from commit 03614fcba25b9de99e69819bc4690f66a3d24438) --- sys/dev/sound/pcm/channel.c | 9 +++++++ sys/dev/sound/pcm/channel.h | 1 + sys/dev/sound/pcm/sound.c | 64 ++++++++++++++++++++++++++++++--------------- 3 files changed, 53 insertions(+), 21 deletions(-) diff --git a/sys/dev/sound/pcm/channel.c b/sys/dev/sound/pcm/channel.c index b4872fdb8037..cf9239839aca 100644 --- a/sys/dev/sound/pcm/channel.c +++ b/sys/dev/sound/pcm/channel.c @@ -1301,6 +1301,15 @@ chn_kill(struct pcm_channel *c) return (0); } +void +chn_shutdown(struct pcm_channel *c) +{ + CHN_LOCKASSERT(c); + + chn_wakeup(c); + c->flags |= CHN_F_DEAD; +} + int chn_setvolume_multi(struct pcm_channel *c, int vc, int left, int right, int center) diff --git a/sys/dev/sound/pcm/channel.h b/sys/dev/sound/pcm/channel.h index 21007454584e..c8d33c583188 100644 --- a/sys/dev/sound/pcm/channel.h +++ b/sys/dev/sound/pcm/channel.h @@ -264,6 +264,7 @@ int chn_poll(struct pcm_channel *c, int ev, struct thread *td); int chn_init(struct pcm_channel *c, void *devinfo, int dir, int direction); int chn_kill(struct pcm_channel *c); +void chn_shutdown(struct pcm_channel *c); int chn_reset(struct pcm_channel *c, u_int32_t fmt, u_int32_t spd); int chn_setvolume_multi(struct pcm_channel *c, int vc, int left, int right, int center); diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c index 7de099cfe5ca..57cbf37005a7 100644 --- a/sys/dev/sound/pcm/sound.c +++ b/sys/dev/sound/pcm/sound.c @@ -678,23 +678,50 @@ pcm_addchan(device_t dev, int dir, kobj_class_t cls, void *devinfo) return (err); } -static int -pcm_killchan(device_t dev) +static void +pcm_killchans(struct snddev_info *d) { - struct snddev_info *d = device_get_softc(dev); struct pcm_channel *ch; int error; + bool found; PCM_BUSYASSERT(d); + do { + found = false; + CHN_FOREACH(ch, d, channels.pcm) { + CHN_LOCK(ch); + /* + * Make sure no channel has went to sleep in the + * meantime. + */ + chn_shutdown(ch); + /* + * We have to give a thread sleeping in chn_sleep() a + * chance to observe that the channel is dead. + */ + if ((ch->flags & CHN_F_SLEEPING) == 0) { + found = true; + CHN_UNLOCK(ch); + break; + } + CHN_UNLOCK(ch); + } - ch = CHN_FIRST(d, channels.pcm); + /* + * All channels are still sleeping. Sleep for a bit and try + * again to see if any of them is awake now. + */ + if (!found) { + pause_sbt("pcmkillchans", SBT_1MS * 5, 0, 0); + continue; + } - PCM_LOCK(d); - error = pcm_chn_remove(d, ch); - PCM_UNLOCK(d); - if (error) - return (error); - return (pcm_chn_destroy(ch)); + PCM_LOCK(d); + error = pcm_chn_remove(d, ch); + PCM_UNLOCK(d); + if (error == 0) + pcm_chn_destroy(ch); + } while (!CHN_EMPTY(d, channels.pcm)); } static int @@ -1005,15 +1032,11 @@ pcm_unregister(device_t dev) CHN_FOREACH(ch, d, channels.pcm) { CHN_LOCK(ch); - if (ch->flags & CHN_F_SLEEPING) { - /* - * We are detaching, so do not wait for the timeout in - * chn_read()/chn_write(). Wake up the thread and kill - * the channel immediately. - */ - CHN_BROADCAST(&ch->intr_cv); - ch->flags |= CHN_F_DEAD; - } + /* + * Do not wait for the timeout in chn_read()/chn_write(). Wake + * up the sleeping thread and kill the channel. + */ + chn_shutdown(ch); chn_abort(ch); CHN_UNLOCK(ch); } @@ -1038,8 +1061,7 @@ pcm_unregister(device_t dev) dsp_destroy_dev(dev); (void)mixer_uninit(dev); - while (!CHN_EMPTY(d, channels.pcm)) - pcm_killchan(dev); + pcm_killchans(d); PCM_LOCK(d); PCM_RELEASE(d);