thesofproject / linux

Linux kernel source tree
Other
89 stars 129 forks source link

KASAN use-after-free in snd_soc_unregister_dai() on module unload #2186

Closed plbossart closed 4 years ago

plbossart commented 4 years ago

Tested with following branch (topic/sof-dev + traces and comments)

https://github.com/plbossart/sound/tree/kasan/hdac_component_use_after_free

sof-dev-defconfig.txt

dmesg-kasan.txt

standard test: boot first with snd-sof-pci blacklisted, then modprobe snd-sof-pci and rmmod snd-sof-pci.

It seems to me that we have the same DAIs declared in multiple places for HDaudio support.

@kv2019i can you reproduce on a plain vanilla HDaudio platform?

plbossart commented 4 years ago

I get the same error on a completely different device (Broadwell Chromebook) so this looks like a core issue with our memory management and devm_ handling during module unload

dmesg-kasan-samus.txt

@ranj063 does this ring a bell?

plbossart commented 4 years ago

@ranj063 the constant on the two platforms is that the use-after-free happens after dealing with the platform-registered DAIs and before the topology-added ones. So I suspect that removing the topology might do too many things and that the devm_ release deal with data already freed.

plbossart commented 4 years ago

This seems to be consistently happening during the list_del operation in snd_soc_unregister_dai.

dmesg-kasan-list-del.txt

plbossart commented 4 years ago

Update: the use-after-free happens when using list_del for the last platform-registered DAI, before taking care of the topology-registered DAIs.

[  177.173443] sof-audio-pci 0000:00:1f.3: ASoC: Unregistered DAI 'Alt Analog CPU DAI'
[  177.173449] sof-audio-pci 0000:00:1f.3: plb: snd_soc_unregister_dai before list_del
[  177.173453] ==================================================================
[  177.173486] BUG: KASAN: use-after-free in snd_soc_unregister_dai+0x7c/0xe1 [snd_soc_core]
[  177.173492] Write of size 8 at addr ffff8884409cd1a0 by task rmmod/1421
sinahuang commented 4 years ago

Error also can be seen on GLK Chrome with onboard codec DA7219 in I2S mode with kernel https://github.com/plbossart/sound/tree/kasan/hdac_component_use_after_free branch (with original issue kconfig)+master branch (commit:700ca2ae).

[ 1253.632723] sof-audio-pci 0000:00:0e.0: display power disable
[ 1253.639393] ==================================================================
[ 1253.639434] BUG: KASAN: use-after-free in snd_soc_unregister_dai+0x3a/0xb0 [snd_soc_core]
[ 1253.639441] Write of size 8 at addr ffff888154bc00a0 by task rmmod/1908

[ 1253.639453] CPU: 1 PID: 1908 Comm: rmmod Not tainted 5.7.0-rc7-default-validation-0611-pieere-0-c5e14df #rc7
[ 1253.639457] Hardware name: Google Bobba/Bobba, BIOS Google_Bobba.11825.0.2019_03_06_2015 03/06/2019

dmesg.log

bardliao commented 4 years ago

Looks like those dai created by soc_tplg_dai_create() -> snd_soc_register_dai() will be freed too early. I won't see the issue if I change devm_kzalloc to kzalloc and devm_kstrdup to kstrdup. I don't understand why only those dais created by topology will be freed too early, but not those dais allocated by the same device.

bardliao commented 4 years ago

I think maybe we shouldn't use devm_snd_soc_register_component() if we use topology. The reason is that release_nodes will free all devres in the device in reverse order. If we see devm_snd_soc_register_component(), we can see devres_add() is called after snd_soc_registercomponent(). That is no problem in normal case since we will unregister ASoC component first, then free other devm allocated resources. But, soc_tplg_dai_create()->snd_soc_register_dai() is called after snd_soc_registercomponent(), and we use devm functions to allocate dai and dai->name there. It means they will be freed before ASoC component is unregistered and that's the reason of use-after-free.

plbossart commented 4 years ago

@sinahuang can you try PR #2188 on GLK? Thanks!

sinahuang commented 4 years ago

Error not seen on GLK codec DA7219 in I2S mode with sof-dev+ PR https://github.com/thesofproject/linux/pull/2188 and master FW(commit:a8ec8b) .