From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: BUG: double free of PCM structure upon module removal Date: Tue, 23 May 2017 20:14:07 +0200 Message-ID: References: <87k2571px9.fsf@belgarion.home> <20170523171405.iyahe4zfxerd7rn7@sirena.org.uk> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 63130267111 for ; Tue, 23 May 2017 20:14:07 +0200 (CEST) In-Reply-To: <20170523171405.iyahe4zfxerd7rn7@sirena.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Mark Brown Cc: alsa-devel@alsa-project.org, Robert Jarzmik List-Id: alsa-devel@alsa-project.org 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? 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. 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 @@ -2047,6 +2047,8 @@ int snd_soc_set_dmi_name(struct snd_soc_card *card, const char *flavour) EXPORT_SYMBOL_GPL(snd_soc_set_dmi_name); #endif /* CONFIG_DMI */ +static void soc_cleanup_card_resources(struct snd_card *scard); + static int snd_soc_instantiate_card(struct snd_soc_card *card) { struct snd_soc_codec *codec; @@ -2228,6 +2230,8 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) } card->instantiated = 1; + card->snd_card->private_data = card; + card->snd_card->private_free = soc_cleanup_card_resources; snd_soc_dapm_sync(&card->dapm); mutex_unlock(&card->mutex); mutex_unlock(&client_mutex); @@ -2278,8 +2282,9 @@ static int soc_probe(struct platform_device *pdev) return snd_soc_register_card(card); } -static int soc_cleanup_card_resources(struct snd_soc_card *card) +static void soc_cleanup_card_resources(struct snd_card *scard) { + struct snd_soc_card *card = scard->private_data; struct snd_soc_pcm_runtime *rtd; /* make sure any delayed work runs */ @@ -2299,10 +2304,6 @@ static int soc_cleanup_card_resources(struct snd_soc_card *card) /* remove the card */ if (card->remove) card->remove(card); - - snd_card_free(card->snd_card); - return 0; - } /* removes a socdev */ @@ -2887,7 +2888,7 @@ int snd_soc_unregister_card(struct snd_soc_card *card) if (card->instantiated) { card->instantiated = false; snd_soc_dapm_shutdown(card); - soc_cleanup_card_resources(card); + snd_card_free(card->snd_card); dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card->name); }