[RFC PATCH v3 07/13] ASoC: soc-pcm: protect for_each_dpcm_be() loops
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Wed Oct 13 16:30:44 CEST 2021
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 <pierre-louis.bossart at linux.intel.com>
---
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
More information about the Alsa-devel
mailing list