From patchwork Wed Mar 6 08:01:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: KaiChieh Chuang X-Patchwork-Id: 10840515 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 74310139A for ; Wed, 6 Mar 2019 08:01:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5FDFF2D274 for ; Wed, 6 Mar 2019 08:01:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5E4662D2B9; Wed, 6 Mar 2019 08:01:40 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 19C232D274 for ; Wed, 6 Mar 2019 08:01:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:Subject:To :From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=oCDQFOXcsiIdahAIskGKR14QBQWptDSkW1Eg5FdH7h8=; b=XOudiyKHdS6a9P lcEj0fQVmgxaAIiGHz08Qfh126aRVIZsx1jacxFTcOjvtISqFcJ+L69mzRjKvnbPxWrsgElkkFta/ aVzjLVr1Libu1BplV1jgqMA1M3i+WAr4fIvhRFKlXmJbpSUQYnBc47njAaCV3lVMNdQ/5Ufk1WmZ0 x90bZ9nwCEm31f6OzghHZhrXpZ2OjYutzDOqWtwqWud2GjH6VVJzp11by03WDGQRDSfMQh4FMU3Ps 0FiP6ZsCaaHWIRFUJPSGFIo3AnyqykrI999XA2x3glj5/mqVTZjVQLaw35sGft4/wqG84dR4HKOet yOtVWpcyNDEF5vC40m5Q==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h1RUX-0000AQ-R2; Wed, 06 Mar 2019 08:01:29 +0000 Received: from mailgw01.mediatek.com ([216.200.240.184]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h1RUV-00009w-N8 for linux-mediatek@lists.infradead.org; Wed, 06 Mar 2019 08:01:29 +0000 X-UUID: dc5ddb0e965947f6b05caba91df76297-20190306 X-UUID: dc5ddb0e965947f6b05caba91df76297-20190306 Received: from mtkcas68.mediatek.inc [(172.29.94.19)] by mailgw01.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLS) with ESMTP id 493704093; Wed, 06 Mar 2019 00:01:19 -0800 Received: from mtkmbs03n2.mediatek.inc (172.21.101.182) by MTKMBS62N2.mediatek.inc (172.29.193.42) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Wed, 6 Mar 2019 00:01:17 -0800 Received: from mtkcas07.mediatek.inc (172.21.101.84) by mtkmbs03n2.mediatek.inc (172.21.101.182) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Wed, 6 Mar 2019 16:01:15 +0800 Received: from mtksdaap41.mediatek.inc (172.21.77.4) by mtkcas07.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1395.4 via Frontend Transport; Wed, 6 Mar 2019 16:01:15 +0800 From: KaiChieh Chuang To: Subject: [RFC PATCH] ASoC: dpcm: prevent snd_soc_dpcm use after free Date: Wed, 6 Mar 2019 16:01:13 +0800 Message-ID: <1551859273-9928-1-git-send-email-kaichieh.chuang@mediatek.com> X-Mailer: git-send-email 1.9.1 MIME-Version: 1.0 X-TM-SNTS-SMTP: 7AF4376639CD56B882201F8B60AFE478553A1506C52365A939BCBE221E6F52462000:8 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190306_000127_771445_993FD3B2 X-CRM114-Status: GOOD ( 11.34 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: michael.hsiao@mediatek.com, alsa-devel@alsa-project.org, hochi.huang@mediatek.com, linux-mediatek@lists.infradead.org, kaichieh.chuang@mediatek.com Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+patchwork-linux-mediatek=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP the dpcm get from fe_clients/be_clients may be free before use possible race condition between void dpcm_be_disconnect( ... list_del(&dpcm->list_be); list_del(&dpcm->list_fe); kfree(dpcm); ... and for_each_dpcm_fe() for_each_dpcm_be*() race condition example Thread 1: snd_soc_dapm_mixer_update_power() -> soc_dpcm_runtime_update() -> dpcm_be_disconnect() -> kfree(dpcm); Thread 2: dpcm_fe_dai_trigger() -> dpcm_be_dai_trigger() -> snd_soc_dpcm_can_be_free_stop() -> if (dpcm->fe == fe) Excpetion Scenario: two FE link to same BE FE1 -> BE FE2 -> Thread 1: switch of mixer between FE2 -> BE Thread 2: pcm_stop FE1 Add a spin lock at snd_soc_card level, to protect the dpcm instance. The lock may be used in atomic context, so use spin lock. Exception: Unable to handle kernel paging request at virtual address dead0000000000e0 pc=<> [] dpcm_be_dai_trigger+0x29c/0x47c sound/soc/soc-pcm.c:3226 if (dpcm->fe == fe) lr=<> [] dpcm_fe_dai_do_trigger+0x94/0x26c Backtrace: [] notify_die+0x68/0xb8 [] die+0x118/0x2a8 [] __do_kernel_fault+0x13c/0x14c [] do_translation_fault+0x64/0xa0 [] do_mem_abort+0x4c/0xd0 [] el1_da+0x24/0x40 [] dpcm_be_dai_trigger+0x29c/0x47c [] dpcm_fe_dai_do_trigger+0x94/0x26c [] dpcm_fe_dai_trigger+0x3c/0x44 [] snd_pcm_do_stop+0x50/0x5c [] snd_pcm_action+0xb4/0x13c [] snd_pcm_drop+0xa0/0x128 [] snd_pcm_common_ioctl+0x9d8/0x30f0 [] snd_pcm_ioctl_compat+0x29c/0x2f14 [] compat_SyS_ioctl+0x128/0x244 [] el0_svc_naked+0x34/0x38 [] 0xffffffffffffffff Signed-off-by: KaiChieh Chuang --- include/sound/soc.h | 2 ++ sound/soc/soc-core.c | 1 + sound/soc/soc-pcm.c | 21 ++++++++++++++++++--- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index eb7db605955b..1e2be35ed36f 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1083,6 +1083,8 @@ struct snd_soc_card { struct mutex mutex; struct mutex dapm_mutex; + spinlock_t dpcm_lock; + bool instantiated; bool topology_shortname_created; diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 5a5764dba147..d88757659729 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2820,6 +2820,7 @@ int snd_soc_register_card(struct snd_soc_card *card) card->instantiated = 0; mutex_init(&card->mutex); mutex_init(&card->dapm_mutex); + spin_lock_init(&card->dpcm_lock); return snd_soc_bind_card(card); } diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index a5b40e82dea4..ac3f773ca768 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1294,9 +1294,11 @@ void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) #ifdef CONFIG_DEBUG_FS debugfs_remove(dpcm->debugfs_state); #endif + spin_lock(&fe->card->dpcm_lock); list_del(&dpcm->list_be); list_del(&dpcm->list_fe); kfree(dpcm); + spin_unlock(&fe->card->dpcm_lock); } } @@ -1548,9 +1550,11 @@ void dpcm_clear_pending_state(struct snd_soc_pcm_runtime *fe, int stream) { struct snd_soc_dpcm *dpcm; + spin_lock(&fe->card->dpcm_lock); for_each_dpcm_be(fe, stream, dpcm) dpcm->be->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO; + spin_unlock(&fe->card->dpcm_lock); } static void dpcm_be_dai_startup_unwind(struct snd_soc_pcm_runtime *fe, @@ -2640,11 +2644,13 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream) dpcm_be_dai_shutdown(fe, stream); disconnect: /* disconnect any non started BEs */ + spin_lock(&fe->card->dpcm_lock); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE; } + spin_unlock(&fe->card->dpcm_lock); return ret; } @@ -3221,6 +3227,7 @@ int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, struct snd_soc_dpcm *dpcm; int state; + spin_lock(&fe->card->dpcm_lock); for_each_dpcm_fe(be, stream, dpcm) { if (dpcm->fe == fe) @@ -3229,9 +3236,12 @@ int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, state = dpcm->fe->dpcm[stream].state; if (state == SND_SOC_DPCM_STATE_START || state == SND_SOC_DPCM_STATE_PAUSED || - state == SND_SOC_DPCM_STATE_SUSPEND) + state == SND_SOC_DPCM_STATE_SUSPEND) { + spin_unlock(&fe->card->dpcm_lock); return 0; + } } + spin_unlock(&fe->card->dpcm_lock); /* it's safe to free/stop this BE DAI */ return 1; @@ -3248,6 +3258,7 @@ int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, struct snd_soc_dpcm *dpcm; int state; + spin_lock(&fe->card->dpcm_lock); for_each_dpcm_fe(be, stream, dpcm) { if (dpcm->fe == fe) @@ -3257,9 +3268,12 @@ int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, if (state == SND_SOC_DPCM_STATE_START || state == SND_SOC_DPCM_STATE_PAUSED || state == SND_SOC_DPCM_STATE_SUSPEND || - state == SND_SOC_DPCM_STATE_PREPARE) + state == SND_SOC_DPCM_STATE_PREPARE) { + spin_unlock(&fe->card->dpcm_lock); return 0; + } } + spin_unlock(&fe->card->dpcm_lock); /* it's safe to change hw_params */ return 1; @@ -3328,6 +3342,7 @@ static ssize_t dpcm_show_state(struct snd_soc_pcm_runtime *fe, goto out; } + spin_lock(&fe->card->dpcm_lock); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; params = &dpcm->hw_params; @@ -3348,7 +3363,7 @@ static ssize_t dpcm_show_state(struct snd_soc_pcm_runtime *fe, params_channels(params), params_rate(params)); } - + spin_unlock(&fe->card->dpcm_lock); out: return offset; }