[for,v6.3-rc] ASoC: SOF: avoid a NULL dereference with unsupported widgets

Commit Message

Peter Ujfalusi March 29, 2023, 11:38 a.m. UTC

From: Guennadi Liakhovetski <[e-mail]>

If an IPC4 topology contains an unsupported widget, its .module_info
field won't be set, then sof_ipc4_route_setup() will cause a kernel
Oops trying to dereference it. Add a check for such cases.

Cc: [e-mail] # 6.2
Signed-off-by: Guennadi Liakhovetski <[e-mail]>
Signed-off-by: Peter Ujfalusi <[e-mail]>
---
Hi Mark,

This patch is generated on top of 6.3-rc4, it will have conflict with asoc-next
because we have ChainDMA scheduled for 6.4 in there.
I should have taken this patch a faster track, but missed it when arranging the
patches, features.
We noticed this when trying to use our development IPC4 topologies with mainline
which does not yet able to handle the process module types (slated fro 6.4).
IPC4 is still evolving so it is not rare that fw/tplg/kernel needs to be
lock-stepped, but NULL pointer dereference should not happen.

This is how the merge conflict resolution should end up between 6.3 and 6.4:

int ret;

/* no route set up if chain DMA is used */
if (src_pipeline->use_chain_dma || sink_pipeline->use_chain_dma) {
	if (!src_pipeline->use_chain_dma || !sink_pipeline->use_chain_dma) {
		dev_err(sdev->dev,
			"use_chain_dma must be set for both src %s and sink %s pipelines\n",
			src_widget->widget->name, sink_widget->widget->name);
		return -EINVAL;
	}
	return 0;
}

if (!src_fw_module || !sink_fw_module) {
	/* The NULL module will print as "(efault)" */
	dev_err(sdev->dev, "source %s or sink %s widget weren't set up properly\n",
		src_fw_module->man4_module_entry.name,
		sink_fw_module->man4_module_entry.name);
	return -ENODEV;
}

sroute->src_queue_id = sof_ipc4_get_queue_id(src_widget, sink_widget,
					     SOF_PIN_TYPE_SOURCE);


Can you send this patch for 6.3 cycle?

Thank you,
Peter

 sound/soc/sof/ipc4-topology.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Mark Brown March 29, 2023, 4:50 p.m. UTC | #1

On Wed, 29 Mar 2023 14:38:28 +0300, Peter Ujfalusi wrote:
> If an IPC4 topology contains an unsupported widget, its .module_info
> field won't be set, then sof_ipc4_route_setup() will cause a kernel
> Oops trying to dereference it. Add a check for such cases.
> 
> 

Applied to

   broonie/sound.git for-6.3

Thanks!

[1/1] ASoC: SOF: avoid a NULL dereference with unsupported widgets
      commit: e3720f92e0237921da537e47a0b24e27899203f8

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

Patch

diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c
index a623707c8ffc..669b99a4f76e 100644
--- a/sound/soc/sof/ipc4-topology.c
+++ b/sound/soc/sof/ipc4-topology.c
@@ -1805,6 +1805,14 @@  static int sof_ipc4_route_setup(struct snd_sof_dev *sdev, struct snd_sof_route *
 	u32 header, extension;
 	int ret;
 
+	if (!src_fw_module || !sink_fw_module) {
+		/* The NULL module will print as "(efault)" */
+		dev_err(sdev->dev, "source %s or sink %s widget weren't set up properly\n",
+			src_fw_module->man4_module_entry.name,
+			sink_fw_module->man4_module_entry.name);
+		return -ENODEV;
+	}
+
 	sroute->src_queue_id = sof_ipc4_get_queue_id(src_widget, sink_widget,
 						     SOF_PIN_TYPE_SOURCE);
 	if (sroute->src_queue_id < 0) {