* [alsa-devel] [PATCH 1/3] ASoC: SOF: fix fault at driver unload after failed probe
2019-12-04 21:04 [alsa-devel] [PATCH 0/3] ASoC: SOF: fixes for kernel oopses/use-after-free Pierre-Louis Bossart
@ 2019-12-04 21:04 ` Pierre-Louis Bossart
2019-12-04 21:04 ` [alsa-devel] [PATCH 2/3] ASoC: SOF: Intel: hda: hda-dai: fix oops on hda_link .hw_free Pierre-Louis Bossart
2019-12-04 21:04 ` [alsa-devel] [PATCH 3/3] ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime() Pierre-Louis Bossart
2 siblings, 0 replies; 6+ messages in thread
From: Pierre-Louis Bossart @ 2019-12-04 21:04 UTC (permalink / raw)
To: alsa-devel
Cc: tiwai, broonie, Kai Vehmanen, Kuninori Morimoto, Pierre-Louis Bossart
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;
--
2.20.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [alsa-devel] [PATCH 2/3] ASoC: SOF: Intel: hda: hda-dai: fix oops on hda_link .hw_free
2019-12-04 21:04 [alsa-devel] [PATCH 0/3] ASoC: SOF: fixes for kernel oopses/use-after-free Pierre-Louis Bossart
2019-12-04 21:04 ` [alsa-devel] [PATCH 1/3] ASoC: SOF: fix fault at driver unload after failed probe Pierre-Louis Bossart
@ 2019-12-04 21:04 ` Pierre-Louis Bossart
2019-12-04 21:04 ` [alsa-devel] [PATCH 3/3] ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime() Pierre-Louis Bossart
2 siblings, 0 replies; 6+ messages in thread
From: Pierre-Louis Bossart @ 2019-12-04 21:04 UTC (permalink / raw)
To: alsa-devel
Cc: Kuninori Morimoto, Kai Vehmanen, tiwai, Pierre-Louis Bossart,
Ranjani Sridharan, broonie
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 */
--
2.20.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [alsa-devel] [PATCH 3/3] ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime()
2019-12-04 21:04 [alsa-devel] [PATCH 0/3] ASoC: SOF: fixes for kernel oopses/use-after-free Pierre-Louis Bossart
2019-12-04 21:04 ` [alsa-devel] [PATCH 1/3] ASoC: SOF: fix fault at driver unload after failed probe Pierre-Louis Bossart
2019-12-04 21:04 ` [alsa-devel] [PATCH 2/3] ASoC: SOF: Intel: hda: hda-dai: fix oops on hda_link .hw_free Pierre-Louis Bossart
@ 2019-12-04 21:04 ` Pierre-Louis Bossart
2019-12-05 0:11 ` Kuninori Morimoto
2019-12-25 0:08 ` [alsa-devel] Applied "ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime()" to the asoc tree Mark Brown
2 siblings, 2 replies; 6+ messages in thread
From: Pierre-Louis Bossart @ 2019-12-04 21:04 UTC (permalink / raw)
To: alsa-devel
Cc: Dragos Tarcatu, Kuninori Morimoto, tiwai, Ranjani Sridharan,
Pierre-Louis Bossart, broonie
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);
}
--
2.20.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [alsa-devel] [PATCH 3/3] ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime()
2019-12-04 21:04 ` [alsa-devel] [PATCH 3/3] ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime() Pierre-Louis Bossart
@ 2019-12-05 0:11 ` Kuninori Morimoto
2019-12-25 0:08 ` [alsa-devel] Applied "ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime()" to the asoc tree Mark Brown
1 sibling, 0 replies; 6+ messages in thread
From: Kuninori Morimoto @ 2019-12-05 0:11 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: tiwai, Dragos Tarcatu, alsa-devel, broonie, Ranjani Sridharan
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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [alsa-devel] Applied "ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime()" to the asoc tree
2019-12-04 21:04 ` [alsa-devel] [PATCH 3/3] ASoC: topology: Prevent use-after-free in snd_soc_get_pcm_runtime() Pierre-Louis Bossart
2019-12-05 0:11 ` Kuninori Morimoto
@ 2019-12-25 0:08 ` Mark Brown
1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2019-12-25 0:08 UTC (permalink / raw)
To: Dragos Tarcatu
Cc: alsa-devel, Kuninori Morimoto, tiwai, Pierre-Louis Bossart,
Ranjani Sridharan, Mark Brown
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.intel.com
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);
}
--
2.20.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread