[alsa-devel] [PATCH v2 0/3] 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.
Changes since v1: Fixed conflict in 3rd patch with Morimoto-san changes (rename of snd_soc_remove_dai_link)
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 | 8 ++++---- sound/soc/sof/intel/hda-dai.c | 11 +++++++++-- sound/soc/sof/ipc.c | 3 +++ 3 files changed, 16 insertions(+), 6 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 293c5ae8e882..1cede16aa7d8 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -830,6 +830,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;
The patch
ASoC: SOF: fix fault at driver unload after failed probe
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.5
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 b06e46427f987bf83dcb6a69516b57276eb8ec0c Mon Sep 17 00:00:00 2001
From: Kai Vehmanen kai.vehmanen@linux.intel.com Date: Tue, 17 Dec 2019 18:05:16 -0600 Subject: [PATCH] ASoC: SOF: fix fault at driver unload after failed probe
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 Link: https://lore.kernel.org/r/20191218000518.5830-2-pierre-louis.bossart@linux.i... Signed-off-by: Mark Brown broonie@kernel.org --- 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 2d9ac0035bd2..7d2903a67503 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -217,6 +217,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; @@ -229,8 +231,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; @@ -362,6 +362,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 */
The patch
ASoC: SOF: Intel: hda: hda-dai: fix oops on hda_link .hw_free
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.5
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 921162c81a089aa2f442103290f1af9ba281fc9f Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Date: Tue, 17 Dec 2019 18:05:17 -0600 Subject: [PATCH] ASoC: SOF: Intel: hda: hda-dai: fix oops on hda_link .hw_free
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 Link: https://lore.kernel.org/r/20191218000518.5830-3-pierre-louis.bossart@linux.i... Signed-off-by: Mark Brown broonie@kernel.org --- 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_pcm_runtime() 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 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index f8bd406c6198..2b3c74a0b126 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -548,14 +548,14 @@ static void remove_link(struct snd_soc_component *comp, if (dobj->ops && dobj->ops->link_unload) dobj->ops->link_unload(comp, dobj);
- kfree(link->name); - kfree(link->stream_name); - kfree(link->cpus->dai_name); - list_del(&dobj->list);
snd_soc_remove_pcm_runtime(comp->card, snd_soc_get_pcm_runtime(comp->card, link)); + + kfree(link->name); + kfree(link->stream_name); + kfree(link->cpus->dai_name); kfree(link); }
Hi Pierre-Louis
From: Dragos Tarcatu dragos_tarcatu@mentor.com
remove_link() is currently calling snd_soc_remove_pcm_runtime() 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 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index f8bd406c6198..2b3c74a0b126 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -548,14 +548,14 @@ static void remove_link(struct snd_soc_component *comp, if (dobj->ops && dobj->ops->link_unload) dobj->ops->link_unload(comp, dobj);
kfree(link->name);
kfree(link->stream_name);
kfree(link->cpus->dai_name);
list_del(&dobj->list);
snd_soc_remove_pcm_runtime(comp->card, snd_soc_get_pcm_runtime(comp->card, link));
- kfree(link->name);
- kfree(link->stream_name);
- kfree(link->cpus->dai_name); kfree(link);
}
Acked-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 for-5.6
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 72b46612d06b83851e2e4f7b538a0bbeb69c10de Mon Sep 17 00:00:00 2001
From: Dragos Tarcatu dragos_tarcatu@mentor.com Date: Tue, 17 Dec 2019 18:05:18 -0600 Subject: [PATCH] ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime()
remove_link() is currently calling snd_soc_remove_pcm_runtime() 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 Acked-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Link: https://lore.kernel.org/r/20191218000518.5830-4-pierre-louis.bossart@linux.i... Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-topology.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index f8bd406c6198..2b3c74a0b126 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -548,14 +548,14 @@ static void remove_link(struct snd_soc_component *comp, if (dobj->ops && dobj->ops->link_unload) dobj->ops->link_unload(comp, dobj);
- kfree(link->name); - kfree(link->stream_name); - kfree(link->cpus->dai_name); - list_del(&dobj->list);
snd_soc_remove_pcm_runtime(comp->card, snd_soc_get_pcm_runtime(comp->card, link)); + + kfree(link->name); + kfree(link->stream_name); + kfree(link->cpus->dai_name); kfree(link); }
participants (3)
-
Kuninori Morimoto -
Mark Brown -
Pierre-Louis Bossart