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

From: Jon Hunter <jonathanh@nvidia.com>
To: Thierry Reding <thierry.reding@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>
Cc: Takashi Iwai <tiwai@suse.com>, Jaroslav Kysela <perex@perex.cz>,
	"Kuninori Morimoto" <kuninori.morimoto.gx@renesas.com>,
	<alsa-devel@alsa-project.org>, <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH 1/2] ASoC: simple-card-utils: Propagate errors on too many links
Date: Fri, 16 Apr 2021 15:49:34 +0100	[thread overview]
Message-ID: <1303b932-5226-0ee6-8f41-fd896908b045@nvidia.com> (raw)
In-Reply-To: <20210416071147.2149109-1-thierry.reding@gmail.com>

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

-- 
nvpublic