Re: [PATCH] ASoC: topology: Fix route memory corruption

[Joakim Tjernlund <Joakim.Tjernlund@xxxxxxxxxxxx>]

On Tue, 2024-08-13 at 19:11 +0200, Joakim Tjernlund wrote:
> On Thu, 2024-06-13 at 11:01 +0200, Amadeusz Sławiński wrote:
> > It was reported that recent fix for memory corruption during topology
> > load, causes corruption in other cases. Instead of being overeager with
> > checking topology, assume that it is properly formatted and just
> > duplicate strings.
> > 
> > Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> > Closes: https://lore.kernel.org/linux-sound/171812236450.201359.3019210915105428447.b4-ty@xxxxxxxxxx/T/#m8c4bd5abf453960fde6f826c4b7f84881da63e9d
> > Suggested-by: Péter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx>
> > Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
> > ---
> >  sound/soc/soc-topology.c | 12 +++---------
> >  1 file changed, 3 insertions(+), 9 deletions(-)
> > 
> > diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
> > index 0225bc5fc425a..4b166294602fa 100644
> > --- a/sound/soc/soc-topology.c
> > +++ b/sound/soc/soc-topology.c
> > @@ -1052,21 +1052,15 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
> >  			break;
> >  		}
> >  
> > -		route->source = devm_kmemdup(tplg->dev, elem->source,
> > -					     min(strlen(elem->source), maxlen),
> > -					     GFP_KERNEL);
> > -		route->sink = devm_kmemdup(tplg->dev, elem->sink,
> > -					   min(strlen(elem->sink), maxlen),
> > -					   GFP_KERNEL);
> > +		route->source = devm_kstrdup(tplg->dev, elem->source, GFP_KERNEL);
> > +		route->sink = devm_kstrdup(tplg->dev, elem->sink, GFP_KERNEL);
> >  		if (!route->source || !route->sink) {
> >  			ret = -ENOMEM;
> >  			break;
> >  		}
> >  
> >  		if (strnlen(elem->control, maxlen) != 0) {
> > -			route->control = devm_kmemdup(tplg->dev, elem->control,
> > -						      min(strlen(elem->control), maxlen),
> > -						      GFP_KERNEL);
> > +			route->control = devm_kstrdup(tplg->dev, elem->control, GFP_KERNEL);
> >  			if (!route->control) {
> >  				ret = -ENOMEM;
> >  				break;
> 
> I am getting very similar corruption/SOF errors in 6.6.45 but there is no similar
> fix in 6.6.x that I can see. Hand hacked this patch and now the errors are gone:
> 
> --- ./sound/soc/soc-topology.c.org	2024-08-13 17:58:49.837295484 +0200
> +++ ./sound/soc/soc-topology.c	2024-08-13 18:20:44.564107024 +0200
> @@ -1060,23 +1060,15 @@
>  			break;
>  		}
>  
> -		route->source = devm_kmemdup(tplg->dev, elem->source,
> -					     min(strlen(elem->source),
> -						 SNDRV_CTL_ELEM_ID_NAME_MAXLEN),
> -					     GFP_KERNEL);
> -		route->sink = devm_kmemdup(tplg->dev, elem->sink,
> -					   min(strlen(elem->sink), SNDRV_CTL_ELEM_ID_NAME_MAXLEN),
> -					   GFP_KERNEL);
> +		route->source = devm_kstrdup(tplg->dev, elem->source, GFP_KERNEL);
> +		route->sink = devm_kstrdup(tplg->dev, elem->sink, GFP_KERNEL);
>  		if (!route->source || !route->sink) {
>  			ret = -ENOMEM;
>  			break;
>  		}
>  
>  		if (strnlen(elem->control, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) != 0) {
> -			route->control = devm_kmemdup(tplg->dev, elem->control,
> -						      min(strlen(elem->control),
> -							  SNDRV_CTL_ELEM_ID_NAME_MAXLEN),
> -						      GFP_KERNEL);
> +			route->control = devm_kstrdup(tplg->dev, elem->control, GFP_KERNEL);
>  			if (!route->control) {
>  				ret = -ENOMEM;
>  				break;
> 
>      
> Just luck?
> 
>  Jocke

Never mind, it was just added to 6.6.x stable queue.

 Jocke