Re: [alsa-devel] [PATCH] ASoC: dmaengine: Make the pcm->name equal to pcm->id if the name is not set

[Takashi Iwai <tiwai@suse.de>]

On Wed, 11 Sep 2019 09:22:53 +0200,
Peter Ujfalusi wrote:
> 
> 
> 
> On 10/09/2019 15.07, Takashi Iwai wrote:
> > On Fri, 06 Sep 2019 07:55:24 +0200,
> > Peter Ujfalusi wrote:
> >>
> >> Some tools use the snd_pcm_info_get_name() to try to identify PCMs or for
> >> other purposes.
> >>
> >> Currently it is left empty with the dmaengine-pcm, in this case copy the
> >> pcm->id string as pcm->name.
> >>
> >> For example IGT is using this to find the HDMI PCM for testing audio on it.
> >>
> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> >> Reported-by: Arthur She <arthur.she@linaro.org>
> >> ---
> >> Hi,
> >>
> >> this was actually reported for 4.14 kernel with omap-pcm (replaced by sdma-pcm
> >> in v4.18), since then we only use the generic dmaengine PCM but the same issue
> >> applies today.
> >>
> >> Regards,
> >> Peter
> >>
> >>  sound/soc/soc-generic-dmaengine-pcm.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
> >> index 748f5f641002..d93db2c2b527 100644
> >> --- a/sound/soc/soc-generic-dmaengine-pcm.c
> >> +++ b/sound/soc/soc-generic-dmaengine-pcm.c
> >> @@ -306,6 +306,12 @@ static int dmaengine_pcm_new(struct snd_soc_pcm_runtime *rtd)
> >>  
> >>  		if (!dmaengine_pcm_can_report_residue(dev, pcm->chan[i]))
> >>  			pcm->flags |= SND_DMAENGINE_PCM_FLAG_NO_RESIDUE;
> >> +
> >> +		if (rtd->pcm->streams[i].pcm->name[0] == '\0') {
> >> +			strncpy(rtd->pcm->streams[i].pcm->name,
> >> +				rtd->pcm->streams[i].pcm->id,
> >> +				sizeof(rtd->pcm->streams[i].pcm->name));
> >> +		}
> > 
> > Any reason to use strncpy() instead of strscpy()?
> > After merging Mark's branch, I got a compile warning like:
> >   sound/soc/soc-generic-dmaengine-pcm.c:311:4: warning: 'strncpy'
> >   accessing 80 bytes at offsets 88 and 24 may overlap up to 0 bytes at
> >   offset [9223372036854775807, -9223372036854775808] [-Wrestrict]
> 
> fwiw I run it again with sparse and it only reports these:
>  CHECK   sound/soc/soc-generic-dmaengine-pcm.c
> sound/soc/soc-generic-dmaengine-pcm.c:177:42: warning: restricted snd_pcm_format_t degrades to integer
> sound/soc/soc-generic-dmaengine-pcm.c:177:47: warning: restricted snd_pcm_format_t degrades to integer
> sound/soc/soc-generic-dmaengine-pcm.c:177:71: warning: restricted snd_pcm_format_t degrades to integer
>   CC      sound/soc/soc-generic-dmaengine-pcm.o
> 
> gcc 6.4.0...9.2.0
> sparse 0.6.0

That happens only with some cross-build on my build tests, so it must
be a pretty minor issue.

But in general, almost all strncpy() use case can be replaced
gracefully with a safer function, either strscpy() or strscpy_pad().
IOW, if you see strncpy(), the patch looks doubtful :)


thanks,

Takashi


> 
> > 
> > 
> > Takashi
> > 
> >>  
> >>  	return 0;
> >> -- 
> >> Peter
> >>
> >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> >> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> >>
> >> _______________________________________________
> >> Alsa-devel mailing list
> >> Alsa-devel@alsa-project.org
> >> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >>
> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel