[PATCH 0/2] Add sanity checks for topology API calls

[PATCH 0/2] Add sanity checks for topology API calls

[Amadeusz Sławiński]

Topology API exposes just 2 function calls, to load and unload topology.
This adds sanity checks to make sure that it behaves well when some of
parameters are set incoorectly or not needed.

This makes developer live easier by failing early instead of proceeding
on and then failing in unexpected ways.

As loading and unloading topology usually happens one the overhead of
additional checks should be negligible.

Amadeusz Sławiński (2):
  ASoC: topology: Ensure that needed parameters are set
  ASoC: topology: Check if ops is set before dereference

 sound/soc/soc-topology.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH 1/2] ASoC: topology: Ensure that needed parameters are set

[Amadeusz Sławiński]

As snd_soc_tplg_component_load is exported function, which means it is
part of API, there should be checks if it is called with proper
parameters.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 950c45008e24..0d182a190c98 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -2659,8 +2659,14 @@ int snd_soc_tplg_component_load(struct snd_soc_component *comp,
 	struct soc_tplg tplg;
 	int ret;
 
-	/* component needs to exist to keep and reference data while parsing */
-	if (!comp)
+	/*
+	 * check if we have sane parameters:
+	 * comp - needs to exist to keep and reference data while parsing
+	 * comp->dev - used for resource management and prints
+	 * comp->card - used for setting card related parameters
+	 * fw - we need it, as it is the very thing we parse
+	 */
+	if (!comp || !comp->dev || !comp->card || !fw)
 		return -EINVAL;
 
 	/* setup parsing context */
-- 
2.25.1

[PATCH 2/2] ASoC: topology: Check if ops is set before dereference

[Amadeusz Sławiński]

Topology can be created without ops overrides, in that case trying to
assign any value would lead to dereferencing NULL pointer.

Other places in code have either checks for tplg->ops or loop using
*_count variables, hence they can't dereference NULL pointer and there
is no need to add more checks.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 0d182a190c98..5476854c12b0 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -2674,11 +2674,13 @@ int snd_soc_tplg_component_load(struct snd_soc_component *comp,
 	tplg.fw = fw;
 	tplg.dev = comp->dev;
 	tplg.comp = comp;
-	tplg.ops = ops;
-	tplg.io_ops = ops->io_ops;
-	tplg.io_ops_count = ops->io_ops_count;
-	tplg.bytes_ext_ops = ops->bytes_ext_ops;
-	tplg.bytes_ext_ops_count = ops->bytes_ext_ops_count;
+	if (ops) {
+		tplg.ops = ops;
+		tplg.io_ops = ops->io_ops;
+		tplg.io_ops_count = ops->io_ops_count;
+		tplg.bytes_ext_ops = ops->bytes_ext_ops;
+		tplg.bytes_ext_ops_count = ops->bytes_ext_ops_count;
+	}
 
 	ret = soc_tplg_load(&tplg);
 	/* free the created components if fail to load topology */
-- 
2.25.1

Re: [PATCH 0/2] Add sanity checks for topology API calls

[Mark Brown]

On Thu, 14 Jan 2021 11:36:00 -0500, Amadeusz Sławiński wrote:
> Topology API exposes just 2 function calls, to load and unload topology.
> This adds sanity checks to make sure that it behaves well when some of
> parameters are set incoorectly or not needed.
> 
> This makes developer live easier by failing early instead of proceeding
> on and then failing in unexpected ways.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: topology: Ensure that needed parameters are set
      commit: d40ab86f7db3612074d08a317bdb1eb8ba06a37e
[2/2] ASoC: topology: Check if ops is set before dereference
      commit: 9c88a9838352c43550ab18080c924824bc660546

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