[alsa-devel] [PATCH] ASoC: Fix freeing of incompletely initialized snd_soc_dapm_context

When soc_init_dai_link() call at the beginning of snd_soc_instantiate_card function fails soc_cleanup_card_resources() and then snd_soc_dapm_free() gets called with an incompletely initialized card->dapm. In particular card->dapm.card is NULL and it gets dereferenced in dapm_free_widgets(). Also dapm->list is invalid and there is an invalid pointer dereference from list_del().
The function call stack (deferred probing) on Chromebook Snow where this issue has shown up in todays -next:
snd_soc_dapm_free soc_cleanup_card_resources snd_soc_instantiate_card snd_soc_register_card devm_snd_soc_register_card snow_probe
Fix this by testing dapm->card before attempting to free dapm widgets.
Fixes: 70fc53734e71 ("ASoC: core: move DAI pre-links initiation to snd_soc_instantiate_card") Reported-by: Marek Szyprowski m.szyprowski@samsung.com Signed-off-by: Sylwester Nawrocki s.nawrocki@samsung.com --- sound/soc/soc-dapm.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 6b44b4a78b8e..5774cbd393fe 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -4658,8 +4658,11 @@ EXPORT_SYMBOL_GPL(snd_soc_dapm_ignore_suspend); void snd_soc_dapm_free(struct snd_soc_dapm_context *dapm) { dapm_debugfs_cleanup(dapm); - dapm_free_widgets(dapm); - list_del(&dapm->list); + + if (dapm->card) { + dapm_free_widgets(dapm); + list_del(&dapm->list); + } } EXPORT_SYMBOL_GPL(snd_soc_dapm_free);
-- 2.17.1

On Fri, Jun 7, 2019 at 10:18 PM Sylwester Nawrocki s.nawrocki@samsung.com wrote:
When soc_init_dai_link() call at the beginning of snd_soc_instantiate_card function fails soc_cleanup_card_resources() and then snd_soc_dapm_free() gets called with an incompletely initialized card->dapm. In particular card->dapm.card is NULL and it gets dereferenced in dapm_free_widgets(). Also dapm->list is invalid and there is an invalid pointer dereference from list_del().
You don't need to do this. In my original patch (https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?h=...), soc_cleanup_card_resources() should not be called if soc_init_dai_link() returns fail.
I found there is a merge conflict. Kuninori Morimoto removed some legacy code (i.e. soc_cleanup_platform() -> soc_cleanup_legacy()) at the same time (https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?h=...).
But, the conflict was not fixed correctly (https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?h=...), the soc_cleanup_platform() turns to soc_cleanup_card_resources().
Based on current for-next branch, we could simply remove the soc_cleanup_card_resources() call.
participants (2)
-
Sylwester Nawrocki
-
Tzung-Bi Shih