linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* Re: [linux-next:master 9388/11353] sound/soc/soc-compress.c:628 snd_soc_new_compress() error: we previously assumed 'codec_dai' could be null (see line 571)
@ 2022-03-10  1:38 Jiasheng Jiang
  0 siblings, 0 replies; 4+ messages in thread
From: Jiasheng Jiang @ 2022-03-10  1:38 UTC (permalink / raw)
  To: dan.carpenter; +Cc: kbuild, lkp, kbuild-all, linux-mm, broonie, Jiasheng Jiang

On Wed, Mar 09, 2022 at 18:18:23PM +0800, Dan Carpenter wrote:
> Yeah.  That's true.  Smatch tries to look at conditions and figure out
> the implications but this one is too complicated.
> 
> But if codec_dai can really be NULL then it would be easier to
> understand if we reversed the check on codec_dai and returned early.
> 
>	if (!codec_dai) {
>		dev_err(rtd->card->dev, "Missing codec\n");
>		return -EINVAL;
>	}
>
>	/* check client and interface hw capabilities */
>	if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) &&
> 	    snd_soc_dai_stream_valid(cpu_dai,   SNDRV_PCM_STREAM_PLAYBACK))
> 		playback = 1;

Thanks, it looks good to me.
I will submit a patch with
"Reported-by: Dan Carpenter <dan.carpenter@oracle.com>"
to make the code more clear.

Jiang



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [linux-next:master 9388/11353] sound/soc/soc-compress.c:628 snd_soc_new_compress() error: we previously assumed 'codec_dai' could be null (see line 571)
  2022-03-09  9:32 Jiasheng Jiang
@ 2022-03-09 10:18 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2022-03-09 10:18 UTC (permalink / raw)
  To: Jiasheng Jiang; +Cc: kbuild, lkp, kbuild-all, linux-mm, broonie

On Wed, Mar 09, 2022 at 05:32:19PM +0800, Jiasheng Jiang wrote:
> On Wed, Mar 09, 2022 at 01:58:04AM +0800, Dan Carpenter wrote:
> > smatch warnings:
> > sound/soc/soc-compress.c:628 snd_soc_new_compress() error: we previously assumed 'codec_dai' could be null (see line 571)
> 
> > de2c6f98817fa5d Jiasheng Jiang     2021-10-15 @571  	if (codec_dai) {
> > 
> > Can codec_dai be NULL?  This new code assumes so.
> 
> > aeb6fa0f15c71a1 Peng Donglin       2017-08-16 @628  			rtd->dai_link->stream_name, codec_dai->name, num);
> >                                                                                                     ^^^^^^^^^^^^^^^^
> > Unchecked dereference
> 
> Actually, if 'codec_dai' is NULL, the check in line 584 will fail and return -EINVAL.
> 
> a1068045883ed4a Vinod Koul         2016-01-07  584  	if (playback + capture != 1) {
> 141dfc9e3751f5f Charles Keepax     2018-01-26  585  		dev_err(rtd->card->dev,
> 141dfc9e3751f5f Charles Keepax     2018-01-26  586  			"Compress ASoC: Invalid direction for P %d, C %d\n",
> a1068045883ed4a Vinod Koul         2016-01-07  587  			playback, capture);
> a1068045883ed4a Vinod Koul         2016-01-07  588  		return -EINVAL;
> a1068045883ed4a Vinod Koul         2016-01-07  589  	}
> 

Yeah.  That's true.  Smatch tries to look at conditions and figure out
the implications but this one is too complicated.

But if codec_dai can really be NULL then it would be easier to
understand if we reversed the check on codec_dai and returned early.

	if (!codec_dai) {
		dev_err(rtd->card->dev, "Missing codec\n");
		return -EINVAL;
	}

	/* check client and interface hw capabilities */
	if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) &&
	    snd_soc_dai_stream_valid(cpu_dai,   SNDRV_PCM_STREAM_PLAYBACK))
		playback = 1;

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [linux-next:master 9388/11353] sound/soc/soc-compress.c:628 snd_soc_new_compress() error: we previously assumed 'codec_dai' could be null (see line 571)
@ 2022-03-09  9:32 Jiasheng Jiang
  2022-03-09 10:18 ` Dan Carpenter
  0 siblings, 1 reply; 4+ messages in thread
From: Jiasheng Jiang @ 2022-03-09  9:32 UTC (permalink / raw)
  To: dan.carpenter, kbuild; +Cc: lkp, kbuild-all, linux-mm, broonie, Jiasheng Jiang

On Wed, Mar 09, 2022 at 01:58:04AM +0800, Dan Carpenter wrote:
> smatch warnings:
> sound/soc/soc-compress.c:628 snd_soc_new_compress() error: we previously assumed 'codec_dai' could be null (see line 571)

> de2c6f98817fa5d Jiasheng Jiang     2021-10-15 @571  	if (codec_dai) {
> 
> Can codec_dai be NULL?  This new code assumes so.

> aeb6fa0f15c71a1 Peng Donglin       2017-08-16 @628  			rtd->dai_link->stream_name, codec_dai->name, num);
>                                                                                                     ^^^^^^^^^^^^^^^^
> Unchecked dereference

Actually, if 'codec_dai' is NULL, the check in line 584 will fail and return -EINVAL.

a1068045883ed4a Vinod Koul         2016-01-07  584  	if (playback + capture != 1) {
141dfc9e3751f5f Charles Keepax     2018-01-26  585  		dev_err(rtd->card->dev,
141dfc9e3751f5f Charles Keepax     2018-01-26  586  			"Compress ASoC: Invalid direction for P %d, C %d\n",
a1068045883ed4a Vinod Koul         2016-01-07  587  			playback, capture);
a1068045883ed4a Vinod Koul         2016-01-07  588  		return -EINVAL;
a1068045883ed4a Vinod Koul         2016-01-07  589  	}

Thanks,
Jiang



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [linux-next:master 9388/11353] sound/soc/soc-compress.c:628 snd_soc_new_compress() error: we previously assumed 'codec_dai' could be null (see line 571)
@ 2022-03-08 17:58 Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2022-03-08 17:58 UTC (permalink / raw)
  To: kbuild, Jiasheng Jiang
  Cc: lkp, kbuild-all, Linux Memory Management List, Mark Brown

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   cb153b68ff91cbc434f3de70ac549e110543e1bb
commit: de2c6f98817fa5decb9b7d3b3a8a3ab864c10588 [9388/11353] ASoC: soc-compress: prevent the potentially use of null pointer
config: m68k-randconfig-m031-20220302 (https://download.01.org/0day-ci/archive/20220309/202203090151.tminQHFc-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
sound/soc/soc-compress.c:628 snd_soc_new_compress() error: we previously assumed 'codec_dai' could be null (see line 571)

vim +/codec_dai +628 sound/soc/soc-compress.c

6f0c42269f000b1 Jie Yang           2015-10-13  544  int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
1245b7005de02d5 Namarta Kohli      2012-08-16  545  {
9e7e3738ab0e908 Kuninori Morimoto  2017-10-11  546  	struct snd_soc_component *component;
c2233a266178f89 Kuninori Morimoto  2020-03-30  547  	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
c2233a266178f89 Kuninori Morimoto  2020-03-30  548  	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
1245b7005de02d5 Namarta Kohli      2012-08-16  549  	struct snd_compr *compr;
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  550  	struct snd_pcm *be_pcm;
1245b7005de02d5 Namarta Kohli      2012-08-16  551  	char new_name[64];
1245b7005de02d5 Namarta Kohli      2012-08-16  552  	int ret = 0, direction = 0;
a1068045883ed4a Vinod Koul         2016-01-07  553  	int playback = 0, capture = 0;
613fb50059cf19a Kuninori Morimoto  2020-01-10  554  	int i;
1245b7005de02d5 Namarta Kohli      2012-08-16  555  
7428d8c8bd79368 Kuninori Morimoto  2020-10-30  556  	/*
7428d8c8bd79368 Kuninori Morimoto  2020-10-30  557  	 * make sure these are same value,
7428d8c8bd79368 Kuninori Morimoto  2020-10-30  558  	 * and then use these as equally
7428d8c8bd79368 Kuninori Morimoto  2020-10-30  559  	 */
7428d8c8bd79368 Kuninori Morimoto  2020-10-30  560  	BUILD_BUG_ON((int)SNDRV_PCM_STREAM_PLAYBACK != (int)SND_COMPRESS_PLAYBACK);
7428d8c8bd79368 Kuninori Morimoto  2020-10-30  561  	BUILD_BUG_ON((int)SNDRV_PCM_STREAM_CAPTURE  != (int)SND_COMPRESS_CAPTURE);
7428d8c8bd79368 Kuninori Morimoto  2020-10-30  562  
6e1276a5e613d25 Bard Liao          2020-02-25  563  	if (rtd->num_cpus > 1 ||
6e1276a5e613d25 Bard Liao          2020-02-25  564  	    rtd->num_codecs > 1) {
141dfc9e3751f5f Charles Keepax     2018-01-26  565  		dev_err(rtd->card->dev,
6e1276a5e613d25 Bard Liao          2020-02-25  566  			"Compress ASoC: Multi CPU/Codec not supported\n");
8151d5e60232d31 Benoit Cousson     2014-07-08  567  		return -EINVAL;
8151d5e60232d31 Benoit Cousson     2014-07-08  568  	}
8151d5e60232d31 Benoit Cousson     2014-07-08  569  
1245b7005de02d5 Namarta Kohli      2012-08-16  570  	/* check client and interface hw capabilities */
de2c6f98817fa5d Jiasheng Jiang     2021-10-15 @571  	if (codec_dai) {

Can codec_dai be NULL?  This new code assumes so.

467fece8fbc6774 Kuninori Morimoto  2019-07-22  572  		if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) &&
467fece8fbc6774 Kuninori Morimoto  2019-07-22  573  		    snd_soc_dai_stream_valid(cpu_dai,   SNDRV_PCM_STREAM_PLAYBACK))
a1068045883ed4a Vinod Koul         2016-01-07  574  			playback = 1;
467fece8fbc6774 Kuninori Morimoto  2019-07-22  575  		if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE) &&
467fece8fbc6774 Kuninori Morimoto  2019-07-22  576  		    snd_soc_dai_stream_valid(cpu_dai,   SNDRV_PCM_STREAM_CAPTURE))
a1068045883ed4a Vinod Koul         2016-01-07  577  			capture = 1;
de2c6f98817fa5d Jiasheng Jiang     2021-10-15  578  	}
a1068045883ed4a Vinod Koul         2016-01-07  579  
a1068045883ed4a Vinod Koul         2016-01-07  580  	/*
a1068045883ed4a Vinod Koul         2016-01-07  581  	 * Compress devices are unidirectional so only one of the directions
a1068045883ed4a Vinod Koul         2016-01-07  582  	 * should be set, check for that (xor)
a1068045883ed4a Vinod Koul         2016-01-07  583  	 */
a1068045883ed4a Vinod Koul         2016-01-07  584  	if (playback + capture != 1) {
141dfc9e3751f5f Charles Keepax     2018-01-26  585  		dev_err(rtd->card->dev,
141dfc9e3751f5f Charles Keepax     2018-01-26  586  			"Compress ASoC: Invalid direction for P %d, C %d\n",
a1068045883ed4a Vinod Koul         2016-01-07  587  			playback, capture);
a1068045883ed4a Vinod Koul         2016-01-07  588  		return -EINVAL;
a1068045883ed4a Vinod Koul         2016-01-07  589  	}
a1068045883ed4a Vinod Koul         2016-01-07  590  
a1068045883ed4a Vinod Koul         2016-01-07  591  	if (playback)
1245b7005de02d5 Namarta Kohli      2012-08-16  592  		direction = SND_COMPRESS_PLAYBACK;
daa2db59ce7e360 Charles Keepax     2013-04-18  593  	else
a1068045883ed4a Vinod Koul         2016-01-07  594  		direction = SND_COMPRESS_CAPTURE;
daa2db59ce7e360 Charles Keepax     2013-04-18  595  
09f448a415ece49 Amadeusz Sławiński 2019-06-17  596  	compr = devm_kzalloc(rtd->card->dev, sizeof(*compr), GFP_KERNEL);
7a0cf42edd9cc33 Markus Elfring     2017-08-10  597  	if (!compr)
1245b7005de02d5 Namarta Kohli      2012-08-16  598  		return -ENOMEM;
1245b7005de02d5 Namarta Kohli      2012-08-16  599  
1f88eb0f0660f8b Charles Keepax     2013-02-05  600  	compr->ops = devm_kzalloc(rtd->card->dev, sizeof(soc_compr_ops),
1f88eb0f0660f8b Charles Keepax     2013-02-05  601  				  GFP_KERNEL);
09f448a415ece49 Amadeusz Sławiński 2019-06-17  602  	if (!compr->ops)
09f448a415ece49 Amadeusz Sławiński 2019-06-17  603  		return -ENOMEM;
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  604  
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  605  	if (rtd->dai_link->dynamic) {
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  606  		snprintf(new_name, sizeof(new_name), "(%s)",
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  607  			rtd->dai_link->stream_name);
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  608  
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  609  		ret = snd_pcm_new_internal(rtd->card->snd_card, new_name, num,
d3268a40d4b19ff Qais Yousef        2015-01-14  610  				rtd->dai_link->dpcm_playback,
d3268a40d4b19ff Qais Yousef        2015-01-14  611  				rtd->dai_link->dpcm_capture, &be_pcm);
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  612  		if (ret < 0) {
141dfc9e3751f5f Charles Keepax     2018-01-26  613  			dev_err(rtd->card->dev,
141dfc9e3751f5f Charles Keepax     2018-01-26  614  				"Compress ASoC: can't create compressed for %s: %d\n",
141dfc9e3751f5f Charles Keepax     2018-01-26  615  				rtd->dai_link->name, ret);
09f448a415ece49 Amadeusz Sławiński 2019-06-17  616  			return ret;
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  617  		}
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  618  
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  619  		rtd->pcm = be_pcm;
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  620  		rtd->fe_compr = 1;
d3268a40d4b19ff Qais Yousef        2015-01-14  621  		if (rtd->dai_link->dpcm_playback)
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  622  			be_pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream->private_data = rtd;
d3268a40d4b19ff Qais Yousef        2015-01-14  623  		else if (rtd->dai_link->dpcm_capture)
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  624  			be_pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream->private_data = rtd;
2a99ef0fdb35a0f Liam Girdwood      2014-01-17  625  		memcpy(compr->ops, &soc_compr_dyn_ops, sizeof(soc_compr_dyn_ops));
aeb6fa0f15c71a1 Peng Donglin       2017-08-16  626  	} else {
aeb6fa0f15c71a1 Peng Donglin       2017-08-16  627  		snprintf(new_name, sizeof(new_name), "%s %s-%d",
aeb6fa0f15c71a1 Peng Donglin       2017-08-16 @628  			rtd->dai_link->stream_name, codec_dai->name, num);
                                                                                                    ^^^^^^^^^^^^^^^^
Unchecked dereference



---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-03-10  1:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-10  1:38 [linux-next:master 9388/11353] sound/soc/soc-compress.c:628 snd_soc_new_compress() error: we previously assumed 'codec_dai' could be null (see line 571) Jiasheng Jiang
  -- strict thread matches above, loose matches on Subject: below --
2022-03-09  9:32 Jiasheng Jiang
2022-03-09 10:18 ` Dan Carpenter
2022-03-08 17:58 Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).