[alsa-devel] [PATCH 0/3] ASoC: SOF: fixes for kernel oopses/use-after-free
More stability fixes for corner cases.
It wasn't straightforward to add a Fixes tag for the two Intel patches, it's likely issues that can be applied to 5.3, possibly earlier. For Dragos' patch Ranjani mentioned this may be due to da704f26ba376 ('ASoC: soc-core: merge snd_soc_remove_dai_link() and soc_unbind_dai_link()'), but Morimoto-san may need to confirm.
Dragos Tarcatu (1): ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime()
Kai Vehmanen (1): ASoC: SOF: fix fault at driver unload after failed probe
Pierre-Louis Bossart (1): ASoC: SOF: Intel: hda: hda-dai: fix oops on hda_link .hw_free
sound/soc/soc-topology.c | 6 +++--- sound/soc/sof/intel/hda-dai.c | 11 +++++++++-- sound/soc/sof/ipc.c | 3 +++ 3 files changed, 15 insertions(+), 5 deletions(-)
From: Kai Vehmanen kai.vehmanen@linux.intel.com
If sof_machine_check() fails during driver probe, the IPC state is not initialized and this will lead to a NULL dereference at driver unload. Example log is as follows:
[ 1535.980630] sof-audio-pci 0000:00:1f.3: error: no matching ASoC machine driver found - aborting probe [ 1535.980631] sof-audio-pci 0000:00:1f.3: error: failed to get machine info -19 [ 1535.980632] sof-audio-pci 0000:00:1f.3: error: sof_probe_work failed err: -19 [ 1550.798373] BUG: kernel NULL pointer dereference, address: 0000000000000008 ... [ 1550.798393] Call Trace: [ 1550.798397] snd_sof_ipc_free+0x15/0x30 [snd_sof] [ 1550.798399] snd_sof_device_remove+0x29/0xa0 [snd_sof] [ 1550.798400] sof_pci_remove+0x10/0x30 [snd_sof_pci]
Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/ipc.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index 5994e1073364..5fdfbaa8c4ed 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -826,6 +826,9 @@ void snd_sof_ipc_free(struct snd_sof_dev *sdev) { struct snd_sof_ipc *ipc = sdev->ipc;
+ if (!ipc) + return; + /* disable sending of ipc's */ mutex_lock(&ipc->tx_mutex); ipc->disable_ipc_tx = true;
When the PCM_PARAM IPC fails while configuring the FE, the kernel oopses in the HDaudio link DMA .hw_free operation. The root cause is a NULL dma_data since the BE .hw_params was never called by the SOC core.
This error can also happen if the HDaudio link DMA configuration IPC fails in the BE .hw_params.
This patches makes sure the dma_data is properly saved in .hw_params, and tested before being use in hw_free.
GitHub issue: https://github.com/thesofproject/linux/issues/1417
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dai.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 8796f385be76..896d21984b73 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -216,6 +216,8 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream, link_dev = hda_link_stream_assign(bus, substream); if (!link_dev) return -EBUSY; + + snd_soc_dai_set_dma_data(dai, substream, (void *)link_dev); }
stream_tag = hdac_stream(link_dev)->stream_tag; @@ -228,8 +230,6 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream, if (ret < 0) return ret;
- snd_soc_dai_set_dma_data(dai, substream, (void *)link_dev); - link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name); if (!link) return -EINVAL; @@ -361,6 +361,13 @@ static int hda_link_hw_free(struct snd_pcm_substream *substream, bus = hstream->bus; rtd = snd_pcm_substream_chip(substream); link_dev = snd_soc_dai_get_dma_data(dai, substream); + + if (!link_dev) { + dev_dbg(dai->dev, + "%s: link_dev is not assigned\n", __func__); + return -EINVAL; + } + hda_stream = hstream_to_sof_hda_stream(link_dev);
/* free the link DMA channel in the FW */
From: Dragos Tarcatu dragos_tarcatu@mentor.com
remove_link() is currently calling snd_soc_remove_dai_link() after it has already freed the memory for the link name. But this is later read from snd_soc_get_pcm_runtime() causing a KASAN use-after-free warning. Reorder the cleanups to fix this issue.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Dragos Tarcatu dragos_tarcatu@mentor.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/soc-topology.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 81d2af000a5c..248530d028a6 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -548,12 +548,12 @@ static void remove_link(struct snd_soc_component *comp, if (dobj->ops && dobj->ops->link_unload) dobj->ops->link_unload(comp, dobj);
+ list_del(&dobj->list); + snd_soc_remove_dai_link(comp->card, link); + kfree(link->name); kfree(link->stream_name); kfree(link->cpus->dai_name); - - list_del(&dobj->list); - snd_soc_remove_dai_link(comp->card, link); kfree(link); }
Hi
From: Dragos Tarcatu dragos_tarcatu@mentor.com
remove_link() is currently calling snd_soc_remove_dai_link() after it has already freed the memory for the link name. But this is later read from snd_soc_get_pcm_runtime() causing a KASAN use-after-free warning. Reorder the cleanups to fix this issue.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Dragos Tarcatu dragos_tarcatu@mentor.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/soc-topology.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 81d2af000a5c..248530d028a6 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -548,12 +548,12 @@ static void remove_link(struct snd_soc_component *comp, if (dobj->ops && dobj->ops->link_unload) dobj->ops->link_unload(comp, dobj);
- list_del(&dobj->list);
- snd_soc_remove_dai_link(comp->card, link);
- kfree(link->name); kfree(link->stream_name); kfree(link->cpus->dai_name);
- list_del(&dobj->list);
- snd_soc_remove_dai_link(comp->card, link); kfree(link);
}
Yeah, indeed this is needed, I think.
Reviewed-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Thank you for your help !! Best regards --- Kuninori Morimoto
The patch
ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime()
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From dd836ddf4e4e1c7f1eb2ae44783ccd70872ef24e Mon Sep 17 00:00:00 2001
From: Dragos Tarcatu dragos_tarcatu@mentor.com Date: Wed, 4 Dec 2019 15:04:47 -0600 Subject: [PATCH] ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime()
remove_link() is currently calling snd_soc_remove_dai_link() after it has already freed the memory for the link name. But this is later read from snd_soc_get_pcm_runtime() causing a KASAN use-after-free warning. Reorder the cleanups to fix this issue.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Dragos Tarcatu dragos_tarcatu@mentor.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Link: https://lore.kernel.org/r/20191204210447.11701-4-pierre-louis.bossart@linux.... Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-topology.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index b28613149b0c..92e4f4d08bfa 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -548,12 +548,12 @@ static void remove_link(struct snd_soc_component *comp, if (dobj->ops && dobj->ops->link_unload) dobj->ops->link_unload(comp, dobj);
+ list_del(&dobj->list); + snd_soc_remove_dai_link(comp->card, link); + kfree(link->name); kfree(link->stream_name); kfree(link->cpus->dai_name); - - list_del(&dobj->list); - snd_soc_remove_dai_link(comp->card, link); kfree(link); }
participants (3)
-
Kuninori Morimoto -
Mark Brown -
Pierre-Louis Bossart