| Message ID | 20190530073229.121032-1-tzungbi@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: core: move DAI pre-links initiation to snd_soc_instantiate_card | expand |
Please ignore this patch. On Thu, May 30, 2019 at 3:32 PM Tzung-Bi Shih <tzungbi@google.com> wrote: > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > index 2d3520fca613..82ff384753c7 100644 > --- a/sound/soc/soc-core.c > +++ b/sound/soc/soc-core.c > @@ -2072,6 +2072,15 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) > mutex_lock(&client_mutex); > mutex_lock_nested(&card->mutex, SND_SOC_CARD_CLASS_INIT); > > + for_each_card_prelinks(card, i, dai_link) { > + ret = soc_init_dai_link(card, dai_link); > + if (ret) { > + dev_err(card->dev, "ASoC: failed to init link %s: %d\n", > + dai_link->name, ret); > + goto probe_end; > + } > + } > + > card->dapm.bias_level = SND_SOC_BIAS_OFF; > card->dapm.dev = card->dev; > card->dapm.card = card; > @@ -2241,7 +2250,7 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) > snd_soc_dapm_sync(&card->dapm); > > probe_end: > - if (ret < 0) > + if (ret < 0 && ret != -EPROBE_DEFER) > soc_cleanup_card_resources(card); Should not call soc_cleanup_card_resources() if soc_init_dai_link() returns fail. Some context has not initialized yet in the case. > > mutex_unlock(&card->mutex);
> > probe_end: > > - if (ret < 0) > > + if (ret < 0 && ret != -EPROBE_DEFER) > > soc_cleanup_card_resources(card); > Should not call soc_cleanup_card_resources() if soc_init_dai_link() > returns fail. Some context has not initialized yet in the case. Why not? You need to clean up the platform naming if links fails which will causes a use-after-free bug if you don't clean it up.
On Tue, Jun 4, 2019 at 1:10 AM Curtis Malainey <cujomalainey@google.com> wrote: > > > > probe_end: > > > - if (ret < 0) > > > + if (ret < 0 && ret != -EPROBE_DEFER) > > > soc_cleanup_card_resources(card); > > Should not call soc_cleanup_card_resources() if soc_init_dai_link() > > returns fail. Some context has not initialized yet in the case. > Why not? You need to clean up the platform naming if links fails which > will causes a use-after-free bug if you don't clean it up. Some context may have not initialized if soc_init_dai_link() returns fail. See v2 https://patchwork.kernel.org/patch/10974149/ if it would help.
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 2d3520fca613..82ff384753c7 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2072,6 +2072,15 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) mutex_lock(&client_mutex); mutex_lock_nested(&card->mutex, SND_SOC_CARD_CLASS_INIT); + for_each_card_prelinks(card, i, dai_link) { + ret = soc_init_dai_link(card, dai_link); + if (ret) { + dev_err(card->dev, "ASoC: failed to init link %s: %d\n", + dai_link->name, ret); + goto probe_end; + } + } + card->dapm.bias_level = SND_SOC_BIAS_OFF; card->dapm.dev = card->dev; card->dapm.card = card; @@ -2241,7 +2250,7 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) snd_soc_dapm_sync(&card->dapm); probe_end: - if (ret < 0) + if (ret < 0 && ret != -EPROBE_DEFER) soc_cleanup_card_resources(card); mutex_unlock(&card->mutex); @@ -2794,26 +2803,9 @@ static int snd_soc_bind_card(struct snd_soc_card *card) */ int snd_soc_register_card(struct snd_soc_card *card) { - int i, ret; - struct snd_soc_dai_link *link; - if (!card->name || !card->dev) return -EINVAL; - mutex_lock(&client_mutex); - for_each_card_prelinks(card, i, link) { - - ret = soc_init_dai_link(card, link); - if (ret) { - soc_cleanup_platform(card); - dev_err(card->dev, "ASoC: failed to init link %s\n", - link->name); - mutex_unlock(&client_mutex); - return ret; - } - } - mutex_unlock(&client_mutex); - dev_set_drvdata(card->dev, card); snd_soc_initialize_card_lists(card);
Kernel crashes when an ASoC component rebinding. As an example, by using the following commands: - echo -n max98357a > /sys/bus/platform/drivers/max98357a/unbind - echo -n max98357a > /sys/bus/platform/drivers/max98357a/bind Got the error message: "Unable to handle kernel NULL pointer dereference at virtual address". The call trace: snd_soc_is_matching_component+0x30/0x6c soc_bind_dai_link+0x16c/0x240 snd_soc_bind_card+0x1e4/0xb10 snd_soc_add_component+0x270/0x300 snd_soc_register_component+0x54/0x6c devm_snd_soc_register_component+0x68/0xac max98357a_platform_probe+0x7c/0x94 The dai_link->platforms has been reset to NULL by soc_cleanup_platform() in soc_cleanup_card_resources() when un-registering component. However, it has no chance to re-allocate the dai_link->platforms when registering the component again. Move the DAI pre-links initiation from snd_soc_register_card() to snd_soc_instantiate_card() to make sure all DAI pre-links get initiated when component rebinding. Signed-off-by: Tzung-Bi Shih <tzungbi@google.com> --- sound/soc/soc-core.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-)