linux/sound
Kuninori Morimoto 0ced7b0502
ASoC: soc-pcm: remove soc_pcm_private_free()
soc-topology adds extra dai_link by using snd_soc_add_dai_link(),
and removes it by snd_soc_romove_dai_link().

This snd_soc_add/remove_dai_link() and/or its related
functions are unbalanced before, and now, these are balance-uped.
But, it finds the random operation issue, and it is reported by
Pierre-Louis.

When card was released, topology will call snd_soc_remove_dai_link()
via (A).

	static void soc_cleanup_card_resources(struct snd_soc_card *card)
	{
		struct snd_soc_dai_link *link, *_link;

		/* This should be called before snd_card_free() */
	(A)	soc_remove_link_components(card);

		/* free the ALSA card at first; this syncs with pending operations */
		if (card->snd_card) {
	(B)		snd_card_free(card->snd_card);
			card->snd_card = NULL;
		}

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

		for_each_card_links_safe(card, link, _link)
	(C)		snd_soc_remove_dai_link(card, link);

		...
	}

At (A), topology calls snd_soc_remove_dai_link().
Then topology rtd, and its related all data are freed.

Next, (B) is called, and then, pcm->private_free = soc_pcm_private_free()
is called.

	static void soc_pcm_private_free(struct snd_pcm *pcm)
	{
		struct snd_soc_pcm_runtime *rtd = pcm->private_data;

		/* need to sync the delayed work before releasing resources */
		flush_delayed_work(&rtd->delayed_work);
		snd_soc_pcm_component_free(rtd);
	}

Here, it gets rtd via pcm->private_data.
But, topology related rtd are already freed at (A).
Normal sound card has no damage, becase it frees rtd at (C).

These are finalizing rtd related data.
Thus, these should be called when rtd was freed, not sound card
was freed. It is very natural and understandable.

In other words, pcm->private_free = soc_pcm_private_free()
is no longer needed.

Extra issue is that there is zero chance to call
soc_remove_dai() for topology related dai at (X).
Because (A) removes rtd connection from card too, and,
(X) is based on card connected rtd.

This means, (X) need to be called before (C) (= for normal sound)
and (A) (= for topology).

Now, I want to focus this patch which is the reason why
snd_card_free() = (B) is located there.

	commit 4efda5f213
	("ASoC: Fix use-after-free at card unregistration")

Original snd_card_free() was called last of this function.
But moved to top to avoid use-after-free issue.
The issue was happen at soc_pcm_free() which was pcm->private_free,
today it is updated/renamed to soc_pcm_private_free().

In other words, (B) need to be called before (C) (= for normal sound)
and (A) (= for topology), because it needs (not yet freed) rtd.
But, (A) need to be called before (B),
because it needs card->snd_card pointer.

If we call flush_delayed_work() and snd_soc_pcm_component_free()
(= same as soc_pcm_private_free()) when rtd was freed (= (C), (A)),
there is no reason to call snd_card_free() at top of this function.
It can be called end of this function, again.

But, in such case, it will likely break unbind again, as Takashi-san
reported. When unbind is performed in a busy state, the code may
release still-in-use resources.
At least we need to call snd_card_disconnect_sync() at the first place.

The final code will be...

	static void soc_cleanup_card_resources(struct snd_soc_card *card)
	{
		struct snd_soc_dai_link *link, *_link;

		if (card->snd_card)
	(Z)		snd_card_disconnect_sync(card->snd_card);

	(X)	soc_remove_link_dais(card);
	(A)	soc_remove_link_components(card);

		for_each_card_links_safe(card, link, _link)
	(C)		snd_soc_remove_dai_link(card, link);

		...
		if (card->snd_card) {
	(B)		snd_card_free(card->snd_card);
			card->snd_card = NULL;
		}
	}

To avoid release still-in-use resources,
call snd_card_disconnect_sync() at (Z).

(X) is needed for both non-topology and topology.

    topology removes rtd via (A), and
non topology removes rtd via (C).

snd_card_free() is no longer related to use-after-free issue.
Thus, locating (B) is no problem.

Fixes: df95a16d2a ("ASoC: soc-core: fix RIP warning on card removal")
Fixes: bc7a9091e5 ("ASoC: soc-core: add soc_unbind_dai_link()")
Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/87o8xax88g.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
2019-11-19 18:23:06 +00:00
..
ac97 ALSA: ac97: Fix double free of ac97_codec_device 2019-07-23 14:16:11 +02:00
aoa ALSA: aoa: onyx: always initialize register read value 2019-07-29 09:21:39 +02:00
arm ASoC: pxa: remove snd_pcm_ops 2019-10-08 13:47:20 +01:00
atmel treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 500 2019-06-19 17:09:55 +02:00
core Merge branch 'for-5.4' of https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into asoc-5.5 2019-11-06 16:29:34 +00:00
drivers treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 500 2019-06-19 17:09:55 +02:00
firewire ALSA: dice: fix wrong packet parameter for Alesis iO26 2019-09-16 23:46:14 +02:00
hda Merge branch 'asoc-5.4' into asoc-next 2019-09-15 10:31:44 +01:00
i2c ALSA: i2c: ak4xxx-adda: Fix a possible null pointer dereference in build_adc_controls() 2019-07-26 14:25:37 +02:00
isa ALSA: sb: remove redundant assignment to variable result 2019-08-14 17:44:06 +02:00
mips treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 176 2019-05-30 11:29:19 -07:00
oss sound: dmasound_atari: Mark expected switch fall-through 2019-07-30 09:36:13 +02:00
parisc treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 176 2019-05-30 11:29:19 -07:00
pci ALSA: hda/hdmi - implement mst_no_extra_pcms flag 2019-10-29 17:31:30 +00:00
pcmcia treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 156 2019-05-30 11:26:35 -07:00
ppc ALSA: ps3: Remove Unneeded variable: "ret" 2019-07-10 11:53:31 +02:00
sh treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 273 2019-06-05 17:30:30 +02:00
soc ASoC: soc-pcm: remove soc_pcm_private_free() 2019-11-19 18:23:06 +00:00
sparc ALSA: sparc: Mark expected switch fall-throughs 2019-07-30 09:37:01 +02:00
spi treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 500 2019-06-19 17:09:55 +02:00
synth treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 156 2019-05-30 11:26:35 -07:00
usb ALSA: usb-audio: Add DSD support for EVGA NU Audio 2019-09-24 13:58:20 +02:00
x86 treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 285 2019-06-05 17:36:37 +02:00
xen ASoC: Updates for v5.3 2019-07-08 14:45:34 +02:00
ac97_bus.c treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 152 2019-05-30 11:26:32 -07:00
Kconfig treewide: Add SPDX license identifier - Makefile/Kconfig 2019-05-21 10:50:46 +02:00
last.c treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 156 2019-05-30 11:26:35 -07:00
Makefile
sound_core.c sound: fix a memory leak bug 2019-08-08 08:18:32 +02:00