On 6/21/22 06:57, Cezary Rojewski wrote:

Flows leading to link->init() and link->exit() are not symmetric. Currently the relevant part of card probe sequence goes as:

for_each_card_rtds(card, rtd) for_each_rtd_components(rtd, i, component) component->probe() for_each_card_rtds(card, rtd) for_each_rtd_dais(rtd, i, dai) dai->probe() for_each_card_rtds(card, rtd) rtd->init()

On the other side, equivalent remove sequence goes as:

for_each_card_rtds(card, rtd) for_each_rtd_dais(rtd, i, dai) dai->remove() for_each_card_rtds(card, rtd) for_each_rtd_components(rtd, i, component) component->remove() for_each_card_rtds(card, rtd) rtd->exit()

what can lead to errors as link->exit() may still operate on resources owned by its components despite the probability of them being freed during the component->remove().

This change modifies the remove sequence to:

for_each_card_rtds(card, rtd) rtd->exit() for_each_card_rtds(card, rtd) for_each_rtd_dais(rtd, i, dai) dai->remove() for_each_card_rtds(card, rtd) for_each_rtd_components(rtd, i, component) component->remove()

so code found in link->exit() is safe to touch any component stuff as component->remove() has not been called yet.

This looks harmless as described, but...

Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Reviewed-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com

---

sound/soc/soc-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 4a3fca50a536..638e781df3b0 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -935,9 +935,6 @@ void snd_soc_remove_pcm_runtime(struct snd_soc_card *card, { lockdep_assert_held(&client_mutex);

• /* release machine specific resources */
• snd_soc_link_exit(rtd);
• /*
  • Notify the machine driver for extra destruction
  */

.... what is not shown here is the

soc_free_pcm_runtime(rtd);

which will call device_unregister(rtd->dev);

....

@@ -1888,6 +1885,9 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)

snd_soc_dapm_shutdown(card);

• /* release machine specific resources */
• for_each_card_rtds(card, rtd)

• snd_soc_link_exit(rtd);
  

... so there's still a risk that the link exit refers to memory that's been released already.

We would need to make sure rtd->dev is not used in any of the existing callbacks - or other functions such as e.g. snd_soc_close_delayed_work() which makes use of rtd->dev

/* remove and free each DAI */ soc_remove_link_dais(card); soc_remove_link_components(card);