Re: BUG: double free of PCM structure upon module removal

[Takashi Iwai <tiwai@suse.de>]

On Wed, 24 May 2017 08:31:39 +0200,
Robert Jarzmik wrote:
> 
> Takashi Iwai <tiwai@suse.de> writes:
> 
> > On Tue, 23 May 2017 19:14:05 +0200,
> > Mark Brown wrote:
> >> 
> >> On Tue, May 23, 2017 at 06:46:58PM +0200, Robert Jarzmik wrote:
> >> 
> >> > Did you notice the same behavior on other platforms, and if not would you have a
> >> > hint why it happens to me ?
> >> 
> >> This is the only report I've seen, sorry.  It's not ringing any bells
> >> immediately either - I'll have a think.
> >
> > What's the problem?  soc_free_pcm_runtime() just calls kfree() of rtd,
> > but it's not called in snd_pcm_free(), isn't it?
> You're right, it's not called, but it's dereferenced.
> The way I see it, what happens is :
>  - soc_free_pcm_runtime() calls kfree(rtd)
>    - ... guess ...
>    - some other call does a kmalloc() which gets the same bucket as rtd
>    - this other call puts data in the previous rtd
>      => in my backtrace, this data is 35343848, which looks like the string
>      "548H"
>  - snd_pcm_free() calls soc_pcm_free()
>  - soc_pcm_free() uses in the for_each loop rtd->card->component_dev_list, and
>    triggers the BUG.

I see, so my patch should cure such a case.

> > In anyway the calls there look in a wrong order.  Basically we should
> > start with snd_card_free() to sync with the whole operation finishes,
> > then release everything.  Below is an untested patch to do that.
> With you patch, the bug is gone in my first 3 tries.

I think the patch below is simpler and has the same effect.
If this works for you, I'll submit the proper patch.


thanks,

Takashi

---
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2286,6 +2286,8 @@ static int soc_cleanup_card_resources(struct snd_soc_card *card)
 	list_for_each_entry(rtd, &card->rtd_list, list)
 		flush_delayed_work(&rtd->delayed_work);
 
+	snd_card_free(card->snd_card);
+
 	/* remove and free each DAI */
 	soc_remove_dai_links(card);
 	soc_remove_pcm_runtimes(card);
@@ -2300,7 +2302,6 @@ static int soc_cleanup_card_resources(struct snd_soc_card *card)
 	if (card->remove)
 		card->remove(card);
 
-	snd_card_free(card->snd_card);
 	return 0;
 
 }