lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 17 Feb 2020 18:19:47 +0100 From: Stephan Gerhold <stephan@...hold.net> To: Mark Brown <broonie@...nel.org> Cc: Sameer Pujar <spujar@...dia.com>, tiwai@...e.com, perex@...ex.cz, alsa-devel@...a-project.org, linux-kernel@...r.kernel.org, Jerome Brunet <jbrunet@...libre.com> Subject: Re: [alsa-devel] [RFC] ASoC: soc-pcm: crash in snd_soc_dapm_new_dai On Mon, Feb 17, 2020 at 06:12:53PM +0100, Stephan Gerhold wrote: > On Mon, Feb 17, 2020 at 03:43:01PM +0000, Mark Brown wrote: > > On Mon, Feb 17, 2020 at 03:41:20PM +0100, Stephan Gerhold wrote: > > > > > I'm a bit confused about this patch, isn't SNDRV_PCM_STREAM_PLAYBACK > > > used for both cpu_dai and codec_dai in the playback case? > > > > It is in the normal case, but with a CODEC<->CODEC link (which was what > > this was targeting) we need to bodge things by swapping playback and > > capture on one end of the link. > > I see. Looking at the code again I'm guessing the cause of the crash > "fixed" by this patch is commit a342031cdd08 ("ASoC: create pcm for > codec2codec links as well") where the codec2codec case was sort of > patched in. This is what we had before this patch: > > /* Adapt stream for codec2codec links */ > struct snd_soc_pcm_stream *cpu_capture = rtd->dai_link->params ? > &cpu_dai->driver->playback : &cpu_dai->driver->capture; > struct snd_soc_pcm_stream *cpu_playback = rtd->dai_link->params ? > &cpu_dai->driver->capture : &cpu_dai->driver->playback; > > This does the swapping you mentioned, so I guess rtd->dai_link->params > is only set for the codec2codec case? > > for_each_rtd_codec_dai(rtd, i, codec_dai) { > 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; > if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE) && > snd_soc_dai_stream_valid(cpu_dai, SNDRV_PCM_STREAM_CAPTURE)) > capture = 1; > } > > capture = capture && cpu_capture->channels_min; > playback = playback && cpu_playback->channels_min; > > And this does a part of the check in snd_soc_dai_stream_valid(), > but without the NULL check of cpu_capture/cpu_playback. > (Maybe that is the cause of the crash.) Uh, no, I am completely wrong here. :) cpu_capture/cpu_playback cannot actually be NULL... I should have looked more carefully at snd_soc_dai_stream_valid()... But I still wonder if the approach below would be easier? > > From my limited understanding, I would say that a much simpler way to > implement this would be: > > /* Adapt stream for codec2codec links */ > int cpu_capture = rtd->dai_link->params ? > SNDRV_PCM_STREAM_PLAYBACK : SNDRV_PCM_STREAM_CAPTURE; > int cpu_playback = rtd->dai_link->params ? > SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK; > > for_each_rtd_codec_dai(rtd, i, codec_dai) { > if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) && > snd_soc_dai_stream_valid(cpu_dai, cpu_playback)) > playback = 1; > if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE) && > snd_soc_dai_stream_valid(cpu_dai, cpu_capture)) > capture = 1; > } > > since snd_soc_dai_stream_valid() does both the NULL-check and the > "channels_min" check. > > But I'm really not familar with the codec2codec case and am unable to > test it :) What do you think? > > Thanks, > Stephan
Powered by blists - more mailing lists