Repository navigation
Topology fixes - #1598
Topology fixes#1598
Conversation
snd_soc_add_dai_link() might fail. This situation occurs for instance in a very specific use case where a PCM device and a Back End DAI link are given identical names in the topology. When this happens, soc_new_pcm_runtime() fails and then snd_soc_add_dai_link() returns -ENOMEM when called from soc_tplg_fe_link_create(). Because of that, the link will not get added into the card list, so any attempt to remove it later ends up in a panic. Fix that by checking the return status and free the memory in case of an error. Signed-off-by: Dragos Tarcatu <dragos_tarcatu@mentor.com>
The return value of soc_tplg_pcm_create() is currently not checked in soc_tplg_pcm_elems_load(). If an error is to occur there, the topology ignores it and continues loading. Fix that by checking the status and rejecting the topology on error. Signed-off-by: Dragos Tarcatu <dragos_tarcatu@mentor.com>
| ret = soc_tplg_pcm_create(tplg, _pcm); | ||
| if (ret < 0) { | ||
| if (!abi_match) | ||
| kfree(_pcm); |
There was a problem hiding this comment.
@dragosht I'm not too sure about this one. The DAI_DRV that is created in soc_tplg_pcm_create could still have been added to the comp->dobj_list and when you attempt to unload the dai, it will point to a freed pcm. Is this correct?
There was a problem hiding this comment.
@ranj063 I see that topology _pcm copy is freed right after pcm_create() anyway, dai_drv doesn't seem to keep any references to it, so it shouldn't be used: https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/soc-topology.c#L2090 . I did the same thing here just to prevent it leak away ...
There was a problem hiding this comment.
@dragosht dai_drv does not keep the reference but the SOF driver does in its spcm. So when dai_unload happens, we will try to access a freed pcm?
There was a problem hiding this comment.
@ranj063 Actually the sof spcm seems to keep its own copy of tplg_pcm, not a pointer to it: https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/sof/sof-audio.h#L47 and it creates that in dai_load: https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/sof/topology.c#L2455
There was a problem hiding this comment.
@dragosht yes you are correct. Thanks for clarifying.
This is related to Issue #1143
Leaving the sysfs naming collisions aside, there seems to be a more subtle issue there as snd_soc_add_dai_link() silently fails and its return code is never checked. Because of that, the dai link is not added into the card list, but if something else is supposed to go wrong while loading it (e.g SSP configuration fails) this would end up in a kernel panic.
For instance this particular topology ends up in a panic: sof-apl-nocodec-ssp5.m4.txt
[ 11.808810] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 11.816593] #PF: supervisor read access in kernel mode
[ 11.822327] #PF: error_code(0x0000) - not-present page
[ 11.828060] PGD 0 P4D 0
[ 11.830888] Oops: 0000 [#1] SMP KASAN
[ 11.834977] CPU: 3 PID: 2414 Comm: systemd-udevd Tainted: G B 5.4.0-rc8-quilt+ #117
[ 11.844987] RIP: 0010:__list_del_entry_valid+0x4a/0xe0
[ 11.850725] Code: ff 4c 8b 2b 48 b8 00 01 00 00 00 00 ad de 49 39 c5 74 3e 48 b8 22 01 00 00 00 00 ad de 49 39 c4 74 4c 4c 89 e7 e8 f6 c5 b9 f8
[ 11.871693] RSP: 0018:ffff88822d25ec40 EFLAGS: 00010246
[ 11.877524] RAX: 0000000000000000 RBX: ffff888222fe8c80 RCX: ffffffffbb1c0a18
[ 11.885493] RDX: 1ffffffff7af13bc RSI: ffffffffbb435e37 RDI: ffffffffbc4fb47c
[ 11.893460] RBP: ffff88822d25ec58 R08: fffffbfff7af13bd R09: fffffbfff7af13bd
[ 11.901426] R10: 0000000000000001 R11: fffffbfff7af13bc R12: 0000000000000000
[ 11.909393] R13: 0000000000000000 R14: ffff888222fe8c88 R15: ffff888222fe8ca0
[ 11.917359] FS: 00007f19b911b240(0000) GS:ffff888238380000(0000) knlGS:0000000000000000
[ 11.926396] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 11.932812] CR2: 0000000000000000 CR3: 0000000231f9c000 CR4: 00000000003406e0
[ 11.940777] Call Trace:
[ 11.943511] snd_soc_remove_dai_link+0x56/0x100
[ 11.948571] snd_soc_tplg_component_remove+0x41f/0x970
[ 11.954309] snd_soc_tplg_component_load+0x1a8/0x1b0
[ 11.959853] ? soc_tplg_process_headers+0x4520/0x4520
[ 11.965511] snd_sof_load_topology+0xcb/0x190 [snd_sof]
[ 11.971350] ? devm_kvasprintf+0x100/0x100
[ 11.975939] ? sof_dbg_comp_config+0x90/0x90 [snd_sof]
[ 11.981689] sof_pcm_probe+0xac/0xf0 [snd_sof]
[ 11.986643] ? mutex_unlock+0x22/0x40
[ 11.990734] snd_soc_component_probe+0x38/0x50
[ 11.995699] soc_probe_component+0x2d4/0x690