From nobody Sun Apr 28 19:49:01 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 4VSH8Y6sgVz5JqcN; Sun, 28 Apr 2024 19:49:01 +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 4VSH8Y4nhLz42NH; Sun, 28 Apr 2024 19:49:01 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1714333741; 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=vvEY0UmoMmefBr2/v8gLpsJlZrhyZUxhJdF/YHVCTa8=; b=UQzBWtEGZMgEwR+mB5SVjBomzpYMt50aQ4d8l2uCkSL7FKlp/L1NXSl4qdAL2Kmh5lg38O M9Jg0srg88YywODSEI6blxFztOEnrJr9A29Hv6TeZakLvhd8xUQjzxdFxFiJEuiJxnakKc E2tQ6A6iNEhoISkeW7UrnwXMF8NwLOLEK4xB08PatwjWMGEwiAbkR4FK8/HSaKKDgCBJqO RYorix8v8bekQIXesGWbs7no3GTTKxfLX6XZTkJbr7Ux9St+br2LpJSazgpWCLu200mODE HV6rTRKBpqAP59aJlgoRLycxcHEztDO2Ue4PsihHeSJqnrGTZMEeVzPf/aIDXg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1714333741; a=rsa-sha256; cv=none; b=sKmKTc9NKilwgG/5RHCqpSo50wjysJ0KymNIiKUvMJsLnqN+wi00vODswakkLCrjLVZtI4 CfvHmkSGAhLZ0fe8m/PgawTIgK3jAM6Jk5GetHxfgTfqiYpy+bGG3jHn4qMX6fOMbCq9KV iLI9CRlYUVCNWxKYHv0dqu0cZfAhFkzYS1qTkCauYeguXpygedxUpgrTH4+D3uxYuAyA6O +gPbOsAla/rPm+c9M9K5DQdh4WhWVYCUHLUJNhqkxZRSLB433Mcio/kilmM3OSN8wrH5j1 NRSacD/Q/OsTjqtyOX5QOBElMYoULGNqO9NmVTDHla8aakFQ2WGMMppH5q4dEQ== 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=1714333741; 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=vvEY0UmoMmefBr2/v8gLpsJlZrhyZUxhJdF/YHVCTa8=; b=VWzfMHs038HSaVskd70NQoyknaR9c+lY5dcfvyVqeeWAhTvDNlmHWY7KrMlsklwkP93FAi WOlc1s8HWo/U8q0lZ3lYZLori8xbhbaAdQAhUsO1e7f/t1lhIYRpL9BQr4bKYJEtONUtJb SLKTlwlSpXjiuZJEd530ikOUjXGS8rrK1FO1Hh25pKmZk0BwPgzAIKvLJRKtMrvMFuJYo5 G88ZBnLE0hdz61bkGBNfV45reFIvKtlq6/xa8yxro41tYDQa+3EnOXGSkHd+p1q34GuyIh Ug/ieAxLA1q6kJa2BwT0J39mAzqHb3+E/94hCMs9HkTL6srO2kD6pS0NbXohzA== 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 4VSH8Y3s0nzf6C; Sun, 28 Apr 2024 19:49:01 +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 43SJn1uj063240; Sun, 28 Apr 2024 19:49:01 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 43SJn1Nq063237; Sun, 28 Apr 2024 19:49:01 GMT (envelope-from git) Date: Sun, 28 Apr 2024 19:49:01 GMT Message-Id: <202404281949.43SJn1Nq063237@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Christos Margiolis Subject: git: 03614fcba25b - main - 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/main X-Git-Reftype: branch X-Git-Commit: 03614fcba25b9de99e69819bc4690f66a3d24438 Auto-Submitted: auto-generated The branch main has been updated by christos: URL: https://cgit.FreeBSD.org/src/commit/?id=03614fcba25b9de99e69819bc4690f66a3d24438 commit 03614fcba25b9de99e69819bc4690f66a3d24438 Author: Christos Margiolis AuthorDate: 2024-04-28 19:40:40 +0000 Commit: Christos Margiolis CommitDate: 2024-04-28 19:48:24 +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 --- 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 faf4bd1c7dce..0a88d732def7 100644 --- a/sys/dev/sound/pcm/sound.c +++ b/sys/dev/sound/pcm/sound.c @@ -674,23 +674,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 @@ -1000,15 +1027,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); } @@ -1033,8 +1056,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);