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

[Takashi Iwai]

On Tue, 10 Sep 2019 16:46:04 +0200,
Takashi Iwai wrote:
> 
> On Tue, 10 Sep 2019 16:23:59 +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 at ti.com>
> > >> Reported-by: Arthur She <arthur.she at 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]
> > 
> > I have not seen such a warning.
> > 'may overlap up to 0 bytes' ?
> > snd_pcm_info {
> > ...
> >         unsigned char id[64];           /* ID (user selectable) */
> >         unsigned char name[80];         /* name of this device */
> >         unsigned char subname[32];      /* subdevice name */
> > ...
> > };
> > 
> > and strncpy() supposed to be something like this:
> > char * strncpy(char *dest, const char *src, size_t n)
> > {
> > 	size_t i;
> > 
> > 	for (i = 0; i < n && src[i] != '\0'; i++)
> > 		dest[i] = src[i];
> > 	for ( ; i < n; i++)
> > 		dest[i] = '\0';
> > 
> > 	return dest;
> > }
> > 
> > I can see if I can get my compilers to show the warning and try
> > strscpy() if it helps on it.
> 
> strncpy() doesn't guarantee the string termination if you pass the
> exact buffer size.  Better to use strscpy() in such a case.

On the second thought, if pcm->id isn't NULL-terminated, it would
result in OOB access.  So, if strncpy() usage is just to guarantee for
the zero-fill, that's OK, to ignore as a possibly spurious old gcc
warning...


Takashi