March 2019 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] Question about devm_get_clk_from_child()
by Kuninori Morimoto 08 Mar '19

08 Mar '19

Hi Stephen I noticed that this commit breaks my ALSA Sound playback 4472287a3b2f52f4aa53f294ccb74392dde4e07d ("clk: Introduce of_clk_get_hw_from_clkspec()") I debuged this issue, and noticed that devm_get_clk_from_child() can't get clk my pseudo DT code is like this deviceA: devA { ... device = <&devB> }; deviceB: devB { clocks = <xxxx> }; I could get clock on deviceB driver // dev = deviceB clk = clk_get(dev, NULL); clk is "clocks = <xxx>" But, I couldn't get clock from deviceA driver // node = device = devB node = ... // dev = deviceA clk = devm_get_clk_from_child(dev, node, NULL); My understanding is these can get same clock. But, am I wrong ?? Best regards --- Kuninori Morimoto

2 5

[alsa-devel] [PATCH] ASoC: soc-pcm: only apply symmetry if more than one stream is active
by Russell King 07 Mar '19

07 Mar '19

ALSA documentation indicates that the hw_params() (and prepare()) callbacks can be made multiple times during the life of an individual stream. It is not specified whether hw_free() will be called, nor is it specified whether the parameters will be the same as the last call. Since symmetry is supposed to be applied between different streams, apply symmetry only when we have more than one activation of the CPU DAI. Signed-off-by: Russell King <rmk+kernel(a)armlinux.org.uk> --- Untested, depends on "ASoC: soc-pcm: make symmetry work again" sound/soc/soc-pcm.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 2a4246dbe55f..3c7c3f79742e 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1010,9 +1010,11 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, } component = NULL; - ret = soc_pcm_params_symmetry(substream, params); - if (ret) - goto component_err; + if (cpu_dai->active > 1) { + ret = soc_pcm_params_symmetry(substream, params); + if (ret) + goto component_err; + } /* store the parameters for each DAIs */ cpu_dai->rate = params_rate(params); -- 2.7.4

1 0

[alsa-devel] [PATCH v2] ASoC: soc-pcm: make symmetry work again
by Russell King 07 Mar '19

07 Mar '19

Commit 957ce0c6b8a1 ("ASoC: soc-pcm: check symmetry after hw_params") moved the check for symmetry after we have set the cpu_dai's rate, channels and sample width to those in the params. The symmetry check verifies that the cpu dai's stored parameters are the same as those in the params. Since we have only just stored these parameters, this makes the symmetry checks always pass. Fix this by moving the symmetry check before we store the parameters, but after the parameters have been fixed up. Fixes: 957ce0c6b8a1 ("ASoC: soc-pcm: check symmetry after hw_params") Signed-off-by: Russell King <rmk+kernel(a)armlinux.org.uk> --- Not tested: this is something I stumbled over while reading the code. v2: also fixes the incorrect whitespace indentation copied from the original patch. sound/soc/soc-pcm.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 08ee163504e8..2a4246dbe55f 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1010,15 +1010,16 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, } component = NULL; + ret = soc_pcm_params_symmetry(substream, params); + if (ret) + goto component_err; + /* store the parameters for each DAIs */ cpu_dai->rate = params_rate(params); cpu_dai->channels = params_channels(params); cpu_dai->sample_bits = snd_pcm_format_physical_width(params_format(params)); - ret = soc_pcm_params_symmetry(substream, params); - if (ret) - goto component_err; out: mutex_unlock(&rtd->pcm_mutex); return ret; -- 2.7.4

1 0

[alsa-devel] [PATCH] ASoC: soc-pcm: make symmetry work again
by Russell King 07 Mar '19

07 Mar '19

Commit 957ce0c6b8a1 ("ASoC: soc-pcm: check symmetry after hw_params") moved the check for symmetry after we have set the cpu_dai's rate, channels and sample width to those in the params. The symmetry check verifies that the cpu dai's stored parameters are the same as those in the params. Since we have only just stored these parameters, this makes the symmetry checks always pass. Fix this by moving the symmetry check before we store the parameters, but after the parameters have been fixed up. Fixes: 957ce0c6b8a1 ("ASoC: soc-pcm: check symmetry after hw_params") Signed-off-by: Russell King <rmk+kernel(a)armlinux.org.uk> --- Not tested: this is something I stumbled over while reading the code. sound/soc/soc-pcm.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 08ee163504e8..dc3c3e31d2ef 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1010,15 +1010,16 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, } component = NULL; + ret = soc_pcm_params_symmetry(substream, params); + if (ret) + goto component_err; + /* store the parameters for each DAIs */ cpu_dai->rate = params_rate(params); cpu_dai->channels = params_channels(params); cpu_dai->sample_bits = snd_pcm_format_physical_width(params_format(params)); - ret = soc_pcm_params_symmetry(substream, params); - if (ret) - goto component_err; out: mutex_unlock(&rtd->pcm_mutex); return ret; -- 2.7.4

1 0

[alsa-devel] [PATCH v2] ASoC: Intel: Boards: Add Maxim98373 support
by Jenny TC 07 Mar '19

07 Mar '19

Reuse kbl_da7219_max98927 machine driver to support max98373 v1: First patchset V2: Updated commit message and Acked-by Jenny TC (1): ASoC: Intel: Boards: Add Maxim98373 support sound/soc/intel/boards/Kconfig | 1 + sound/soc/intel/boards/kbl_da7219_max98927.c | 203 +++++++++++++++++++--- sound/soc/intel/common/soc-acpi-intel-kbl-match.c | 19 ++ 3 files changed, 200 insertions(+), 23 deletions(-) -- 1.9.1

4 4

[alsa-devel] ChromeBook machine will lose MIC JD(again)
by Kailang 07 Mar '19

07 Mar '19

Hi Takashi, Attach patch will solve the issue for Mic JD lose. BR, Kailang

1 0

[alsa-devel] [PATCH linux-next v1 0/2] misc fix in src.c
by Jiada Wang 07 Mar '19

07 Mar '19

two issues were found due to linux-next commit 7674bec4fc09 ("ASoC: rsnd: update BSDSR/BSDISR handling") this patch-set address these two issues Jiada Wang (2): ASoC: rsnd: src: Avoid a potential deadlock ASoC: rsnd: src: fix compiler warnings sound/soc/sh/rcar/core.c | 3 +++ sound/soc/sh/rcar/rsnd.h | 5 +++++ sound/soc/sh/rcar/src.c | 21 +++++++-------------- 3 files changed, 15 insertions(+), 14 deletions(-) -- 2.19.2

2 4

[alsa-devel] [PATCH v2] ASoC: dpcm: prevent snd_soc_dpcm use after free
by KaiChieh Chuang 07 Mar '19

07 Mar '19

The dpcm get from fe_clients/be_clients may be free before use 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. 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 Exception: Unable to handle kernel paging request at virtual address dead0000000000e0 pc=<> [<ffffff8960e2cd10>] dpcm_be_dai_trigger+0x29c/0x47c sound/soc/soc-pcm.c:3226 if (dpcm->fe == fe) lr=<> [<ffffff8960e2f694>] dpcm_fe_dai_do_trigger+0x94/0x26c Backtrace: [<ffffff89602dba80>] notify_die+0x68/0xb8 [<ffffff896028c7dc>] die+0x118/0x2a8 [<ffffff89602a2f84>] __do_kernel_fault+0x13c/0x14c [<ffffff89602a27f4>] do_translation_fault+0x64/0xa0 [<ffffff8960280cf8>] do_mem_abort+0x4c/0xd0 [<ffffff8960282ad0>] el1_da+0x24/0x40 [<ffffff8960e2cd10>] dpcm_be_dai_trigger+0x29c/0x47c [<ffffff8960e2f694>] dpcm_fe_dai_do_trigger+0x94/0x26c [<ffffff8960e2edec>] dpcm_fe_dai_trigger+0x3c/0x44 [<ffffff8960de5588>] snd_pcm_do_stop+0x50/0x5c [<ffffff8960dded24>] snd_pcm_action+0xb4/0x13c [<ffffff8960ddfdb4>] snd_pcm_drop+0xa0/0x128 [<ffffff8960de69bc>] snd_pcm_common_ioctl+0x9d8/0x30f0 [<ffffff8960de1cac>] snd_pcm_ioctl_compat+0x29c/0x2f14 [<ffffff89604c9d60>] compat_SyS_ioctl+0x128/0x244 [<ffffff8960283740>] el0_svc_naked+0x34/0x38 [<ffffffffffffffff>] 0xffffffffffffffff Signed-off-by: KaiChieh Chuang <kaichieh.chuang(a)mediatek.com> --- include/sound/soc.h | 2 ++ sound/soc/soc-core.c | 1 + sound/soc/soc-pcm.c | 33 ++++++++++++++++++++++++++------- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index eb7db60..1e2be35 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 5a5764d..d887576 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 a5b40e8..9020a06 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1228,8 +1228,10 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, dpcm->fe = fe; be->dpcm[stream].runtime = fe->dpcm[stream].runtime; dpcm->state = SND_SOC_DPCM_LINK_STATE_NEW; + spin_lock(&fe->card->dpcm_lock); list_add(&dpcm->list_be, &fe->dpcm[stream].be_clients); list_add(&dpcm->list_fe, &be->dpcm[stream].fe_clients); + spin_unlock(&fe->card->dpcm_lock); dev_dbg(fe->dev, "connected new DPCM %s path %s %s %s\n", stream ? "capture" : "playback", fe->dai_link->name, @@ -1294,8 +1296,10 @@ 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); + spin_unlock(&fe->card->dpcm_lock); kfree(dpcm); } } @@ -1548,9 +1552,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 +2646,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; } @@ -3220,7 +3228,9 @@ int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, { struct snd_soc_dpcm *dpcm; int state; + int ret = 1; + spin_lock(&fe->card->dpcm_lock); for_each_dpcm_fe(be, stream, dpcm) { if (dpcm->fe == fe) @@ -3229,12 +3239,15 @@ 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) - return 0; + state == SND_SOC_DPCM_STATE_SUSPEND) { + ret = 0; + break; + } } + spin_unlock(&fe->card->dpcm_lock); /* it's safe to free/stop this BE DAI */ - return 1; + return ret; } EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_free_stop); @@ -3247,7 +3260,9 @@ int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, { struct snd_soc_dpcm *dpcm; int state; + int ret = 1; + spin_lock(&fe->card->dpcm_lock); for_each_dpcm_fe(be, stream, dpcm) { if (dpcm->fe == fe) @@ -3257,12 +3272,15 @@ 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) - return 0; + state == SND_SOC_DPCM_STATE_PREPARE) { + ret = 0; + break; + } } + spin_unlock(&fe->card->dpcm_lock); /* it's safe to change hw_params */ - return 1; + return ret; } EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_params); @@ -3328,6 +3346,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 +3367,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; } -- 1.9.1

1 0

[alsa-devel] [PATCH] ASoC: dpcm: prevent snd_soc_dpcm use after free
by KaiChieh Chuang 07 Mar '19

07 Mar '19

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=<> [<ffffff8960e2cd10>] dpcm_be_dai_trigger+0x29c/0x47c sound/soc/soc-pcm.c:3226 if (dpcm->fe == fe) lr=<> [<ffffff8960e2f694>] dpcm_fe_dai_do_trigger+0x94/0x26c Backtrace: [<ffffff89602dba80>] notify_die+0x68/0xb8 [<ffffff896028c7dc>] die+0x118/0x2a8 [<ffffff89602a2f84>] __do_kernel_fault+0x13c/0x14c [<ffffff89602a27f4>] do_translation_fault+0x64/0xa0 [<ffffff8960280cf8>] do_mem_abort+0x4c/0xd0 [<ffffff8960282ad0>] el1_da+0x24/0x40 [<ffffff8960e2cd10>] dpcm_be_dai_trigger+0x29c/0x47c [<ffffff8960e2f694>] dpcm_fe_dai_do_trigger+0x94/0x26c [<ffffff8960e2edec>] dpcm_fe_dai_trigger+0x3c/0x44 [<ffffff8960de5588>] snd_pcm_do_stop+0x50/0x5c [<ffffff8960dded24>] snd_pcm_action+0xb4/0x13c [<ffffff8960ddfdb4>] snd_pcm_drop+0xa0/0x128 [<ffffff8960de69bc>] snd_pcm_common_ioctl+0x9d8/0x30f0 [<ffffff8960de1cac>] snd_pcm_ioctl_compat+0x29c/0x2f14 [<ffffff89604c9d60>] compat_SyS_ioctl+0x128/0x244 [<ffffff8960283740>] el0_svc_naked+0x34/0x38 [<ffffffffffffffff>] 0xffffffffffffffff Signed-off-by: KaiChieh Chuang <kaichieh.chuang(a)mediatek.com> --- include/sound/soc.h | 2 ++ sound/soc/soc-core.c | 1 + sound/soc/soc-pcm.c | 33 ++++++++++++++++++++++++++------- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index eb7db60..1e2be35 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 5a5764d..d887576 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 a5b40e8..9020a06 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1228,8 +1228,10 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, dpcm->fe = fe; be->dpcm[stream].runtime = fe->dpcm[stream].runtime; dpcm->state = SND_SOC_DPCM_LINK_STATE_NEW; + spin_lock(&fe->card->dpcm_lock); list_add(&dpcm->list_be, &fe->dpcm[stream].be_clients); list_add(&dpcm->list_fe, &be->dpcm[stream].fe_clients); + spin_unlock(&fe->card->dpcm_lock); dev_dbg(fe->dev, "connected new DPCM %s path %s %s %s\n", stream ? "capture" : "playback", fe->dai_link->name, @@ -1294,8 +1296,10 @@ 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); + spin_unlock(&fe->card->dpcm_lock); kfree(dpcm); } } @@ -1548,9 +1552,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 +2646,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; } @@ -3220,7 +3228,9 @@ int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, { struct snd_soc_dpcm *dpcm; int state; + int ret = 1; + spin_lock(&fe->card->dpcm_lock); for_each_dpcm_fe(be, stream, dpcm) { if (dpcm->fe == fe) @@ -3229,12 +3239,15 @@ 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) - return 0; + state == SND_SOC_DPCM_STATE_SUSPEND) { + ret = 0; + break; + } } + spin_unlock(&fe->card->dpcm_lock); /* it's safe to free/stop this BE DAI */ - return 1; + return ret; } EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_free_stop); @@ -3247,7 +3260,9 @@ int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, { struct snd_soc_dpcm *dpcm; int state; + int ret = 1; + spin_lock(&fe->card->dpcm_lock); for_each_dpcm_fe(be, stream, dpcm) { if (dpcm->fe == fe) @@ -3257,12 +3272,15 @@ 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) - return 0; + state == SND_SOC_DPCM_STATE_PREPARE) { + ret = 0; + break; + } } + spin_unlock(&fe->card->dpcm_lock); /* it's safe to change hw_params */ - return 1; + return ret; } EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_params); @@ -3328,6 +3346,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 +3367,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; } -- 1.9.1

1 0

[alsa-devel] [PATCH] ASoC: dpcm: prevent snd_soc_dpcm use after free
by KaiChieh Chuang 07 Mar '19

07 Mar '19

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=<> [<ffffff8960e2cd10>] dpcm_be_dai_trigger+0x29c/0x47c sound/soc/soc-pcm.c:3226 if (dpcm->fe == fe) lr=<> [<ffffff8960e2f694>] dpcm_fe_dai_do_trigger+0x94/0x26c Backtrace: [<ffffff89602dba80>] notify_die+0x68/0xb8 [<ffffff896028c7dc>] die+0x118/0x2a8 [<ffffff89602a2f84>] __do_kernel_fault+0x13c/0x14c [<ffffff89602a27f4>] do_translation_fault+0x64/0xa0 [<ffffff8960280cf8>] do_mem_abort+0x4c/0xd0 [<ffffff8960282ad0>] el1_da+0x24/0x40 [<ffffff8960e2cd10>] dpcm_be_dai_trigger+0x29c/0x47c [<ffffff8960e2f694>] dpcm_fe_dai_do_trigger+0x94/0x26c [<ffffff8960e2edec>] dpcm_fe_dai_trigger+0x3c/0x44 [<ffffff8960de5588>] snd_pcm_do_stop+0x50/0x5c [<ffffff8960dded24>] snd_pcm_action+0xb4/0x13c [<ffffff8960ddfdb4>] snd_pcm_drop+0xa0/0x128 [<ffffff8960de69bc>] snd_pcm_common_ioctl+0x9d8/0x30f0 [<ffffff8960de1cac>] snd_pcm_ioctl_compat+0x29c/0x2f14 [<ffffff89604c9d60>] compat_SyS_ioctl+0x128/0x244 [<ffffff8960283740>] el0_svc_naked+0x34/0x38 [<ffffffffffffffff>] 0xffffffffffffffff Signed-off-by: KaiChieh Chuang <kaichieh.chuang(a)mediatek.com> --- include/sound/soc.h | 2 ++ sound/soc/soc-core.c | 1 + sound/soc/soc-pcm.c | 33 ++++++++++++++++++++++++++------- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index eb7db60..1e2be35 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 5a5764d..d887576 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 a5b40e8..e99346e 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1228,8 +1228,10 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, dpcm->fe = fe; be->dpcm[stream].runtime = fe->dpcm[stream].runtime; dpcm->state = SND_SOC_DPCM_LINK_STATE_NEW; + spin_lock(&fe->card->dpcm_lock); list_add(&dpcm->list_be, &fe->dpcm[stream].be_clients); list_add(&dpcm->list_fe, &be->dpcm[stream].fe_clients); + spin_unlock(&fe->card->dpcm_lock); dev_dbg(fe->dev, "connected new DPCM %s path %s %s %s\n", stream ? "capture" : "playback", fe->dai_link->name, @@ -1294,8 +1296,10 @@ 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); + spin_unlock(&fe->card->dpcm_lock); kfree(dpcm); } } @@ -1548,9 +1552,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 +2646,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; } @@ -3220,7 +3228,9 @@ int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, { struct snd_soc_dpcm *dpcm; int state; + int ret = 1; + spin_lock(&fe->card->dpcm_lock); for_each_dpcm_fe(be, stream, dpcm) { if (dpcm->fe == fe) @@ -3229,12 +3239,15 @@ 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) - return 0; + state == SND_SOC_DPCM_STATE_SUSPEND) { + ret = 0; + break; + } } + spin_unlock(&fe->card->dpcm_lock); /* it's safe to free/stop this BE DAI */ - return 1; + return ret; } EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_free_stop); @@ -3247,7 +3260,9 @@ int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, { struct snd_soc_dpcm *dpcm; int state; + int ret = 1; + spin_lock(&fe->card->dpcm_lock); for_each_dpcm_fe(be, stream, dpcm) { if (dpcm->fe == fe) @@ -3257,12 +3272,15 @@ 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) - return 0; + state == SND_SOC_DPCM_STATE_PREPARE) { + ret = 0 + break; + } } + spin_unlock(&fe->card->dpcm_lock); /* it's safe to change hw_params */ - return 1; + return ret; } EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_params); @@ -3328,6 +3346,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 +3367,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; } -- 1.9.1

1 0