| Message ID | 20190607141745.759-1-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: Fix freeing of incompletely initialized snd_soc_dapm_context | expand |
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=for-next&id=70fc53734e71ce51f46dfcfd1a1c319e1cfe080c), 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=for-next&id=adb76b5b9c4740a11f6ad6c68764515961ae8ade). But, the conflict was not fixed correctly (https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?h=for-next&id=a8e992342ce4cd173d437d0aa4eecc9e30489f72), 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.
On 6/7/19 17:02, Tzung-Bi Shih wrote: > Based on current for-next branch, we could simply remove the > soc_cleanup_card_resources() call. Thank you for looking into this, I will post a patch removing that unnecessary call instead.
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);
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(-) -- 2.17.1