diff mbox series

[1/2] ASoC: simple-card-utils: Propagate errors on too many links

Message ID 20210416071147.2149109-1-thierry.reding@gmail.com (mailing list archive)
State Accepted
Commit 0f687d826736a5b4eee03170382fe54d413b912a
Headers show
Series [1/2] ASoC: simple-card-utils: Propagate errors on too many links | expand

Commit Message

Thierry Reding April 16, 2021, 7:11 a.m. UTC
From: Thierry Reding <treding@nvidia.com>

The DAI counting code doesn't propagate errors when the number of
maximum links is exceeded, which causes subsequent initialization code
to continue to run and that eventually leads to memory corruption with
the code trying to access memory that is out of bounds.

Fix this by propgating errors when the maximum number of links is
reached, which ensures that the driver fails to load and prevents the
memory corruption.

Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 sound/soc/generic/audio-graph-card.c | 19 +++++++++++--------
 sound/soc/generic/simple-card.c      | 17 ++++++++++-------
 2 files changed, 21 insertions(+), 15 deletions(-)

Comments

Jon Hunter April 16, 2021, 2:49 p.m. UTC | #1
On 16/04/2021 08:11, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> The DAI counting code doesn't propagate errors when the number of
> maximum links is exceeded, which causes subsequent initialization code
> to continue to run and that eventually leads to memory corruption with
> the code trying to access memory that is out of bounds.
> 
> Fix this by propgating errors when the maximum number of links is

s/propgating/propagating

> reached, which ensures that the driver fails to load and prevents the
> memory corruption.
> 
> Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  sound/soc/generic/audio-graph-card.c | 19 +++++++++++--------
>  sound/soc/generic/simple-card.c      | 17 ++++++++++-------
>  2 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
> index 0582fe296471..2401212281c2 100644
> --- a/sound/soc/generic/audio-graph-card.c
> +++ b/sound/soc/generic/audio-graph-card.c
> @@ -539,8 +539,8 @@ static int graph_for_each_link(struct asoc_simple_priv *priv,
>  	return ret;
>  }
>  
> -static void graph_get_dais_count(struct asoc_simple_priv *priv,
> -				 struct link_info *li);
> +static int graph_get_dais_count(struct asoc_simple_priv *priv,
> +				struct link_info *li);
>  
>  int audio_graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
>  {
> @@ -552,7 +552,10 @@ int audio_graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
>  	card->dev = dev;
>  
>  	memset(&li, 0, sizeof(li));
> -	graph_get_dais_count(priv, &li);
> +	ret = graph_get_dais_count(priv, &li);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (!li.link)
>  		return -EINVAL;
>  
> @@ -657,8 +660,8 @@ static int graph_count_dpcm(struct asoc_simple_priv *priv,
>  	return 0;
>  }
>  
> -static void graph_get_dais_count(struct asoc_simple_priv *priv,
> -				 struct link_info *li)
> +static int graph_get_dais_count(struct asoc_simple_priv *priv,
> +				struct link_info *li)
>  {
>  	/*
>  	 * link_num :	number of links.
> @@ -706,9 +709,9 @@ static void graph_get_dais_count(struct asoc_simple_priv *priv,
>  	 *	=> 4 DAIs  = 2xCPU + 2xCodec
>  	 *	=> 1 ccnf  = 1xdummy-Codec
>  	 */
> -	graph_for_each_link(priv, li,
> -			    graph_count_noml,
> -			    graph_count_dpcm);
> +	return graph_for_each_link(priv, li,
> +				   graph_count_noml,
> +				   graph_count_dpcm);
>  }
>  
>  int audio_graph_card_probe(struct snd_soc_card *card)
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index bf5ddf1ea65f..1d1c4309b582 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -526,8 +526,8 @@ static int simple_count_dpcm(struct asoc_simple_priv *priv,
>  	return 0;
>  }
>  
> -static void simple_get_dais_count(struct asoc_simple_priv *priv,
> -				  struct link_info *li)
> +static int simple_get_dais_count(struct asoc_simple_priv *priv,
> +				 struct link_info *li)
>  {
>  	struct device *dev = simple_priv_to_dev(priv);
>  	struct device_node *top = dev->of_node;
> @@ -584,12 +584,12 @@ static void simple_get_dais_count(struct asoc_simple_priv *priv,
>  		li->num[0].platforms	= 1;
>  
>  		li->link = 1;
> -		return;
> +		return 0;
>  	}
>  
> -	simple_for_each_link(priv, li,
> -			     simple_count_noml,
> -			     simple_count_dpcm);
> +	return simple_for_each_link(priv, li,
> +				    simple_count_noml,
> +				    simple_count_dpcm);
>  }
>  
>  static int simple_soc_probe(struct snd_soc_card *card)
> @@ -628,7 +628,10 @@ static int asoc_simple_probe(struct platform_device *pdev)
>  	card->probe		= simple_soc_probe;
>  
>  	memset(&li, 0, sizeof(li));
> -	simple_get_dais_count(priv, &li);
> +	ret = simple_get_dais_count(priv, &li);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (!li.link)
>  		return -EINVAL;
>  
> 

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon
Mark Brown April 16, 2021, 3:55 p.m. UTC | #2
On Fri, Apr 16, 2021 at 03:49:34PM +0100, Jon Hunter wrote:
> 
> On 16/04/2021 08:11, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > The DAI counting code doesn't propagate errors when the number of
> > maximum links is exceeded, which causes subsequent initialization code
> > to continue to run and that eventually leads to memory corruption with
> > the code trying to access memory that is out of bounds.
> > 
> > Fix this by propgating errors when the maximum number of links is
> 
> s/propgating/propagating
> 
> > reached, which ensures that the driver fails to load and prevents the
> > memory corruption.
> > 
> > Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> > Signed-off-by: Thierry Reding <treding@nvidia.com>

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.
Mark Brown April 16, 2021, 4:01 p.m. UTC | #3
On Fri, 16 Apr 2021 09:11:46 +0200, Thierry Reding wrote:
> The DAI counting code doesn't propagate errors when the number of
> maximum links is exceeded, which causes subsequent initialization code
> to continue to run and that eventually leads to memory corruption with
> the code trying to access memory that is out of bounds.
> 
> Fix this by propgating errors when the maximum number of links is
> reached, which ensures that the driver fails to load and prevents the
> memory corruption.

Applied to

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

Thanks!

[1/2] ASoC: simple-card-utils: Propagate errors on too many links
      commit: 0f687d826736a5b4eee03170382fe54d413b912a
[2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
      commit: 343e55e71877415a23372388b3e0c59a9bba42f6

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
diff mbox series

Patch

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 0582fe296471..2401212281c2 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -539,8 +539,8 @@  static int graph_for_each_link(struct asoc_simple_priv *priv,
 	return ret;
 }
 
-static void graph_get_dais_count(struct asoc_simple_priv *priv,
-				 struct link_info *li);
+static int graph_get_dais_count(struct asoc_simple_priv *priv,
+				struct link_info *li);
 
 int audio_graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
 {
@@ -552,7 +552,10 @@  int audio_graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
 	card->dev = dev;
 
 	memset(&li, 0, sizeof(li));
-	graph_get_dais_count(priv, &li);
+	ret = graph_get_dais_count(priv, &li);
+	if (ret < 0)
+		return ret;
+
 	if (!li.link)
 		return -EINVAL;
 
@@ -657,8 +660,8 @@  static int graph_count_dpcm(struct asoc_simple_priv *priv,
 	return 0;
 }
 
-static void graph_get_dais_count(struct asoc_simple_priv *priv,
-				 struct link_info *li)
+static int graph_get_dais_count(struct asoc_simple_priv *priv,
+				struct link_info *li)
 {
 	/*
 	 * link_num :	number of links.
@@ -706,9 +709,9 @@  static void graph_get_dais_count(struct asoc_simple_priv *priv,
 	 *	=> 4 DAIs  = 2xCPU + 2xCodec
 	 *	=> 1 ccnf  = 1xdummy-Codec
 	 */
-	graph_for_each_link(priv, li,
-			    graph_count_noml,
-			    graph_count_dpcm);
+	return graph_for_each_link(priv, li,
+				   graph_count_noml,
+				   graph_count_dpcm);
 }
 
 int audio_graph_card_probe(struct snd_soc_card *card)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index bf5ddf1ea65f..1d1c4309b582 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -526,8 +526,8 @@  static int simple_count_dpcm(struct asoc_simple_priv *priv,
 	return 0;
 }
 
-static void simple_get_dais_count(struct asoc_simple_priv *priv,
-				  struct link_info *li)
+static int simple_get_dais_count(struct asoc_simple_priv *priv,
+				 struct link_info *li)
 {
 	struct device *dev = simple_priv_to_dev(priv);
 	struct device_node *top = dev->of_node;
@@ -584,12 +584,12 @@  static void simple_get_dais_count(struct asoc_simple_priv *priv,
 		li->num[0].platforms	= 1;
 
 		li->link = 1;
-		return;
+		return 0;
 	}
 
-	simple_for_each_link(priv, li,
-			     simple_count_noml,
-			     simple_count_dpcm);
+	return simple_for_each_link(priv, li,
+				    simple_count_noml,
+				    simple_count_dpcm);
 }
 
 static int simple_soc_probe(struct snd_soc_card *card)
@@ -628,7 +628,10 @@  static int asoc_simple_probe(struct platform_device *pdev)
 	card->probe		= simple_soc_probe;
 
 	memset(&li, 0, sizeof(li));
-	simple_get_dais_count(priv, &li);
+	ret = simple_get_dais_count(priv, &li);
+	if (ret < 0)
+		return ret;
+
 	if (!li.link)
 		return -EINVAL;