From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 69528C433F5 for ; Wed, 13 Oct 2021 14:32:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5475B610C8 for ; Wed, 13 Oct 2021 14:32:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237572AbhJMOe3 (ORCPT ); Wed, 13 Oct 2021 10:34:29 -0400 Received: from mga12.intel.com ([192.55.52.136]:5091 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237000AbhJMOdn (ORCPT ); Wed, 13 Oct 2021 10:33:43 -0400 X-IronPort-AV: E=McAfee;i="6200,9189,10135"; a="207554856" X-IronPort-AV: E=Sophos;i="5.85,371,1624345200"; d="scan'208";a="207554856" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Oct 2021 07:31:18 -0700 X-IronPort-AV: E=Sophos;i="5.85,371,1624345200"; d="scan'208";a="524649537" Received: from ssafavib-mobl.amr.corp.intel.com (HELO pbossart-mobl3.intel.com) ([10.209.164.198]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Oct 2021 07:31:16 -0700 From: Pierre-Louis Bossart To: alsa-devel@alsa-project.org Cc: tiwai@suse.de, broonie@kernel.org, vkoul@kernel.org, Sameer Pujar , Gyeongtaek Lee , Peter Ujfalusi , Kuninori Morimoto , Pierre-Louis Bossart , Liam Girdwood , Jaroslav Kysela , Takashi Iwai , linux-kernel@vger.kernel.org (open list) Subject: [RFC PATCH v3 07/13] ASoC: soc-pcm: protect for_each_dpcm_be() loops Date: Wed, 13 Oct 2021 09:30:44 -0500 Message-Id: <20211013143050.244444-8-pierre-louis.bossart@linux.intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211013143050.244444-1-pierre-louis.bossart@linux.intel.com> References: <20211013143050.244444-1-pierre-louis.bossart@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The D in DPCM stands for 'dynamic', which means that connections between FE and BE can evolve. Commit a97648697790 ("ASoC: dpcm: prevent snd_soc_dpcm use after free") started to protect some of the for_each_dpcm_be() loops, but there are still many cases that were not modified. This patch adds protection for all the remaining loops, with the notable exception of the dpcm_be_dai_trigger(), where the lock is already taken at a higher level, e.g. in snd_pcm_period_elapsed(). Signed-off-by: Pierre-Louis Bossart --- sound/soc/soc-pcm.c | 86 ++++++++++++++++++++------------------------- 1 file changed, 39 insertions(+), 47 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 797f0d114c83..5f2368059e14 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -42,15 +42,12 @@ void snd_soc_dpcm_fe_unlock_irq(struct snd_soc_pcm_runtime *fe, int stream) EXPORT_SYMBOL_GPL(snd_soc_dpcm_fe_unlock_irq); /* can this BE stop and free */ -static int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, - struct snd_soc_pcm_runtime *be, int stream); - static int snd_soc_dpcm_can_be_free_stop_locked(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream); /* can this BE perform a hw_params() */ -static int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, - struct snd_soc_pcm_runtime *be, int stream); +static int snd_soc_dpcm_can_be_params_locked(struct snd_soc_pcm_runtime *fe, + struct snd_soc_pcm_runtime *be, int stream); /* is the current PCM operation for this BE ? */ static int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe, @@ -335,6 +332,7 @@ int dpcm_dapm_stream_event(struct snd_soc_pcm_runtime *fe, int dir, { struct snd_soc_dpcm *dpcm; + snd_soc_dpcm_fe_lock_irq(fe, dir); for_each_dpcm_be(fe, dir, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; @@ -348,6 +346,8 @@ int dpcm_dapm_stream_event(struct snd_soc_pcm_runtime *fe, int dir, snd_soc_dapm_stream_event(be, dir, event); } + snd_soc_dpcm_fe_unlock_irq(fe, dir); + snd_soc_dapm_stream_event(fe, dir, event); @@ -1237,6 +1237,7 @@ void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) { struct snd_soc_dpcm *dpcm, *d; + snd_soc_dpcm_fe_lock_irq(fe, stream); for_each_dpcm_be_safe(fe, stream, dpcm, d) { dev_dbg(fe->dev, "ASoC: BE %s disconnect check for %s\n", stream ? "capture" : "playback", @@ -1254,12 +1255,11 @@ void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) dpcm_remove_debugfs_state(dpcm); - snd_soc_dpcm_fe_lock_irq(fe, stream); list_del(&dpcm->list_be); list_del(&dpcm->list_fe); - snd_soc_dpcm_fe_unlock_irq(fe, stream); kfree(dpcm); } + snd_soc_dpcm_fe_unlock_irq(fe, stream); } /* get BE for DAI widget and stream */ @@ -1386,6 +1386,7 @@ static int dpcm_prune_paths(struct snd_soc_pcm_runtime *fe, int stream, int prune = 0; /* Destroy any old FE <--> BE connections */ + snd_soc_dpcm_fe_lock_irq(fe, stream); for_each_dpcm_be(fe, stream, dpcm) { if (dpcm_be_is_active(dpcm, stream, *list_)) continue; @@ -1397,6 +1398,7 @@ static int dpcm_prune_paths(struct snd_soc_pcm_runtime *fe, int stream, dpcm_set_be_update_state(dpcm->be, stream, SND_SOC_DPCM_UPDATE_BE); prune++; } + snd_soc_dpcm_fe_unlock_irq(fe, stream); dev_dbg(fe->dev, "ASoC: found %d old BE paths for pruning\n", prune); return prune; @@ -1496,13 +1498,16 @@ void dpcm_be_dai_stop(struct snd_soc_pcm_runtime *fe, int stream, struct snd_soc_dpcm *dpcm; /* disable any enabled and non active backends */ + snd_soc_dpcm_fe_lock_irq(fe, stream); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; struct snd_pcm_substream *be_substream = snd_soc_dpcm_get_substream(be, stream); - if (dpcm == last) + if (dpcm == last) { + snd_soc_dpcm_fe_unlock_irq(fe, stream); return; + } /* is this op for this BE ? */ if (!snd_soc_dpcm_be_can_update(fe, be, stream)) @@ -1532,6 +1537,7 @@ void dpcm_be_dai_stop(struct snd_soc_pcm_runtime *fe, int stream, be_substream->runtime = NULL; be->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE; } + snd_soc_dpcm_fe_unlock_irq(fe, stream); } int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) @@ -1541,6 +1547,7 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) int err, count = 0; /* only startup BE DAIs that are either sinks or sources to this FE DAI */ + snd_soc_dpcm_fe_lock_irq(fe, stream); for_each_dpcm_be(fe, stream, dpcm) { struct snd_pcm_substream *be_substream; @@ -1591,11 +1598,13 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) be->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN; count++; } + snd_soc_dpcm_fe_unlock_irq(fe, stream); return count; unwind: dpcm_be_dai_startup_rollback(fe, stream, dpcm); + snd_soc_dpcm_fe_unlock_irq(fe, stream); dev_err(fe->dev, "ASoC: %s() failed at %s (%d)\n", __func__, be->dai_link->name, err); @@ -1650,6 +1659,7 @@ static void dpcm_runtime_setup_be_format(struct snd_pcm_substream *substream) * if FE want to use it (= dpcm_merged_format) */ + snd_soc_dpcm_fe_lock_irq(fe, stream); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; struct snd_soc_pcm_stream *codec_stream; @@ -1668,6 +1678,7 @@ static void dpcm_runtime_setup_be_format(struct snd_pcm_substream *substream) soc_pcm_hw_update_format(hw, codec_stream); } } + snd_soc_dpcm_fe_unlock_irq(fe, stream); } static void dpcm_runtime_setup_be_chan(struct snd_pcm_substream *substream) @@ -1685,7 +1696,7 @@ static void dpcm_runtime_setup_be_chan(struct snd_pcm_substream *substream) * It returns merged BE codec channel; * if FE want to use it (= dpcm_merged_chan) */ - + snd_soc_dpcm_fe_lock_irq(fe, stream); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; struct snd_soc_pcm_stream *cpu_stream; @@ -1716,6 +1727,7 @@ static void dpcm_runtime_setup_be_chan(struct snd_pcm_substream *substream) soc_pcm_hw_update_chan(hw, codec_stream); } } + snd_soc_dpcm_fe_unlock_irq(fe, stream); } static void dpcm_runtime_setup_be_rate(struct snd_pcm_substream *substream) @@ -1733,7 +1745,7 @@ static void dpcm_runtime_setup_be_rate(struct snd_pcm_substream *substream) * It returns merged BE codec channel; * if FE want to use it (= dpcm_merged_chan) */ - + snd_soc_dpcm_fe_lock_irq(fe, stream); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; struct snd_soc_pcm_stream *pcm; @@ -1753,6 +1765,7 @@ static void dpcm_runtime_setup_be_rate(struct snd_pcm_substream *substream) soc_pcm_hw_update_rate(hw, pcm); } } + snd_soc_dpcm_fe_unlock_irq(fe, stream); } static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream, @@ -1775,6 +1788,7 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream, } /* apply symmetry for BE */ + snd_soc_dpcm_fe_lock_irq(fe, stream); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; struct snd_pcm_substream *be_substream = @@ -1800,6 +1814,7 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream, } } error: + snd_soc_dpcm_fe_unlock_irq(fe, stream); if (err < 0) dev_err(fe->dev, "ASoC: %s failed (%d)\n", __func__, err); @@ -1875,6 +1890,7 @@ void dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream) /* only hw_params backends that are either sinks or sources * to this frontend DAI */ + snd_soc_dpcm_fe_lock_irq(fe, stream); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; @@ -1886,7 +1902,7 @@ void dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream) continue; /* only free hw when no longer used - check all FEs */ - if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) + if (!snd_soc_dpcm_can_be_free_stop_locked(fe, be, stream)) continue; /* do not free hw if this BE is used by other FE */ @@ -1908,6 +1924,7 @@ void dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream) be->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_FREE; } + snd_soc_dpcm_fe_unlock_irq(fe, stream); } static int dpcm_fe_dai_hw_free(struct snd_pcm_substream *substream) @@ -1941,6 +1958,7 @@ int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream) struct snd_soc_dpcm *dpcm; int ret; + snd_soc_dpcm_fe_lock_irq(fe, stream); for_each_dpcm_be(fe, stream, dpcm) { be = dpcm->be; be_substream = snd_soc_dpcm_get_substream(be, stream); @@ -1963,7 +1981,7 @@ int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream) sizeof(struct snd_pcm_hw_params)); /* only allow hw_params() if no connected FEs are running */ - if (!snd_soc_dpcm_can_be_params(fe, be, stream)) + if (!snd_soc_dpcm_can_be_params_locked(fe, be, stream)) continue; if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_OPEN) && @@ -1980,6 +1998,7 @@ int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream) be->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_PARAMS; } + snd_soc_dpcm_fe_unlock_irq(fe, stream); return 0; unwind: @@ -1995,7 +2014,7 @@ int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream) continue; /* only allow hw_free() if no connected FEs are running */ - if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream)) + if (!snd_soc_dpcm_can_be_free_stop_locked(fe, be, stream)) continue; if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_OPEN) && @@ -2006,6 +2025,7 @@ int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream) soc_pcm_hw_free(be_substream); } + snd_soc_dpcm_fe_unlock_irq(fe, stream); return ret; } @@ -2290,6 +2310,7 @@ int dpcm_be_dai_prepare(struct snd_soc_pcm_runtime *fe, int stream) struct snd_soc_dpcm *dpcm; int ret = 0; + snd_soc_dpcm_fe_lock_irq(fe, stream); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; @@ -2315,6 +2336,7 @@ int dpcm_be_dai_prepare(struct snd_soc_pcm_runtime *fe, int stream) be->dpcm[stream].state = SND_SOC_DPCM_STATE_PREPARE; } + snd_soc_dpcm_fe_unlock_irq(fe, stream); if (ret < 0) dev_err(fe->dev, "ASoC: %s() failed (%d)\n", __func__, ret); @@ -2588,8 +2610,10 @@ static void dpcm_fe_dai_cleanup(struct snd_pcm_substream *fe_substream) int stream = fe_substream->stream; /* mark FE's links ready to prune */ + snd_soc_dpcm_fe_lock_irq(fe, stream); for_each_dpcm_be(fe, stream, dpcm) dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE; + snd_soc_dpcm_fe_unlock_irq(fe, stream); dpcm_be_disconnect(fe, stream); @@ -2905,22 +2929,6 @@ static int snd_soc_dpcm_check_state_locked(struct snd_soc_pcm_runtime *fe, return ret; } -static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe, - struct snd_soc_pcm_runtime *be, - int stream, - const enum snd_soc_dpcm_state *states, - int num_states) -{ - int ret; - - snd_soc_dpcm_fe_lock_irq(fe, stream); - ret = snd_soc_dpcm_check_state_locked(fe, be, stream, states, num_states); - snd_soc_dpcm_fe_unlock_irq(fe, stream); - - /* it's safe to do this BE DAI */ - return ret; -} - /* * We can only hw_free, stop, pause or suspend a BE DAI if any of it's FE * are not running, paused or suspended for the specified stream direction. @@ -2937,27 +2945,11 @@ static int snd_soc_dpcm_can_be_free_stop_locked(struct snd_soc_pcm_runtime *fe, return snd_soc_dpcm_check_state_locked(fe, be, stream, state, ARRAY_SIZE(state)); } -/* - * We can only hw_free, stop, pause or suspend a BE DAI if any of it's FE - * are not running, paused or suspended for the specified stream direction. - */ -static int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, - struct snd_soc_pcm_runtime *be, int stream) -{ - const enum snd_soc_dpcm_state state[] = { - SND_SOC_DPCM_STATE_START, - SND_SOC_DPCM_STATE_PAUSED, - SND_SOC_DPCM_STATE_SUSPEND, - }; - - return snd_soc_dpcm_check_state(fe, be, stream, state, ARRAY_SIZE(state)); -} - /* * We can only change hw params a BE DAI if any of it's FE are not prepared, * running, paused or suspended for the specified stream direction. */ -static int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, +static int snd_soc_dpcm_can_be_params_locked(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream) { const enum snd_soc_dpcm_state state[] = { @@ -2967,5 +2959,5 @@ static int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, SND_SOC_DPCM_STATE_PREPARE, }; - return snd_soc_dpcm_check_state(fe, be, stream, state, ARRAY_SIZE(state)); + return snd_soc_dpcm_check_state_locked(fe, be, stream, state, ARRAY_SIZE(state)); } -- 2.25.1