[Sound-open-firmware] [PATCH 1/2] ASoC: SOF: Prevent NULL dereference in sof_pcm_dai_link_fixup()
The "dia" pointer can be NULL, so handle that condition first before storing "dia->private". Fixes: 839e484f9e17 ("ASoC: SOF: make struct snd_sof_dai IPC agnostic") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- sound/soc/sof/pcm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 1661b0bc6f12..71f5bce0c4c7 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -702,7 +702,7 @@ int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_pa struct snd_sof_dai *dai = snd_sof_find_dai(component, (char *)rtd->dai_link->name); struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component); - struct sof_dai_private_data *private = dai->private; + struct sof_dai_private_data *private; struct snd_soc_dpcm *dpcm; /* no topology exists for this BE, try a common configuration */ @@ -727,6 +727,7 @@ int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_pa /* read format from topology */ snd_mask_none(fmt); + private = dai->private; switch (private->comp_dai->config.frame_fmt) { case SOF_IPC_FRAME_S16_LE: snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S16_LE); -- 2.20.1
This code dereferences "dai" before checking whether it can be NULL. Fixes: 839e484f9e17 ("ASoC: SOF: make struct snd_sof_dai IPC agnostic") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- sound/soc/sof/sof-audio.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index 15c36a51f89f..88ddd1e2476d 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -626,10 +626,13 @@ int sof_set_up_pipelines(struct snd_sof_dev *sdev, bool verify) /* update DAI config. The IPC will be sent in sof_widget_setup() */ if (WIDGET_IS_DAI(swidget->id)) { struct snd_sof_dai *dai = swidget->private; - struct sof_dai_private_data *private = dai->private; + struct sof_dai_private_data *private; struct sof_ipc_dai_config *config; - if (!dai || !private || !private->dai_config) + if (!dai) + continue; + private = dai->private; + if (!private || !private->dai_config) continue; config = private->dai_config; @@ -918,10 +921,13 @@ static int sof_dai_get_clk(struct snd_soc_pcm_runtime *rtd, int clk_type) snd_soc_rtdcom_lookup(rtd, SOF_AUDIO_PCM_DRV_NAME); struct snd_sof_dai *dai = snd_sof_find_dai(component, (char *)rtd->dai_link->name); - struct sof_dai_private_data *private = dai->private; + struct sof_dai_private_data *private; /* use the tplg configured mclk if existed */ - if (!dai || !private || !private->dai_config) + if (!dai) + return 0; + private = dai->private; + if (!private || !private->dai_config) return 0; switch (private->dai_config->type) { -- 2.20.1
On 3/18/22 02:13, Dan Carpenter wrote:
This code dereferences "dai" before checking whether it can be NULL.
Fixes: 839e484f9e17 ("ASoC: SOF: make struct snd_sof_dai IPC agnostic") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Same thing, this code will be removed in follow-up patches and we missed the intermediate error. Thanks for the patch. Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
--- sound/soc/sof/sof-audio.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index 15c36a51f89f..88ddd1e2476d 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -626,10 +626,13 @@ int sof_set_up_pipelines(struct snd_sof_dev *sdev, bool verify) /* update DAI config. The IPC will be sent in sof_widget_setup() */ if (WIDGET_IS_DAI(swidget->id)) { struct snd_sof_dai *dai = swidget->private; - struct sof_dai_private_data *private = dai->private; + struct sof_dai_private_data *private; struct sof_ipc_dai_config *config;
- if (!dai || !private || !private->dai_config) + if (!dai) + continue; + private = dai->private; + if (!private || !private->dai_config) continue;
config = private->dai_config; @@ -918,10 +921,13 @@ static int sof_dai_get_clk(struct snd_soc_pcm_runtime *rtd, int clk_type) snd_soc_rtdcom_lookup(rtd, SOF_AUDIO_PCM_DRV_NAME); struct snd_sof_dai *dai = snd_sof_find_dai(component, (char *)rtd->dai_link->name); - struct sof_dai_private_data *private = dai->private; + struct sof_dai_private_data *private;
/* use the tplg configured mclk if existed */ - if (!dai || !private || !private->dai_config) + if (!dai) + return 0; + private = dai->private; + if (!private || !private->dai_config) return 0;
switch (private->dai_config->type) {
On 3/18/22 02:12, Dan Carpenter wrote:
The "dia" pointer can be NULL, so handle that condition first before storing "dia->private".
Fixes: 839e484f9e17 ("ASoC: SOF: make struct snd_sof_dai IPC agnostic") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Thanks for the patch. This part will be removed in follow-up patches, likely the reason why this problem was missed. Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
--- sound/soc/sof/pcm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 1661b0bc6f12..71f5bce0c4c7 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -702,7 +702,7 @@ int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_pa struct snd_sof_dai *dai = snd_sof_find_dai(component, (char *)rtd->dai_link->name); struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component); - struct sof_dai_private_data *private = dai->private; + struct sof_dai_private_data *private; struct snd_soc_dpcm *dpcm;
/* no topology exists for this BE, try a common configuration */ @@ -727,6 +727,7 @@ int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_pa /* read format from topology */ snd_mask_none(fmt);
+ private = dai->private; switch (private->comp_dai->config.frame_fmt) { case SOF_IPC_FRAME_S16_LE: snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S16_LE);
On Fri, 2022-03-18 at 09:42 -0500, Pierre-Louis Bossart wrote:
>
> On 3/18/22 02:12, Dan Carpenter wrote:
> > The "dia" pointer can be NULL, so handle that condition first
> > before
> > storing "dia->private".
> >
> > Fixes: 839e484f9e17 ("ASoC: SOF: make struct snd_sof_dai IPC
> > agnostic")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> Thanks for the patch.
>
> This part will be removed in follow-up patches, likely the reason
> why
> this problem was missed.
Hi Dan/Pierre,
Both these problems are address in the series I posted yesterday.
Particularly patches 16 and 18.
Thanks,
Ranjani
>
> Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>
> > ---
> > sound/soc/sof/pcm.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
> > index 1661b0bc6f12..71f5bce0c4c7 100644
> > --- a/sound/soc/sof/pcm.c
> > +++ b/sound/soc/sof/pcm.c
> > @@ -702,7 +702,7 @@ int sof_pcm_dai_link_fixup(struct
> > snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_pa
> > struct snd_sof_dai *dai =
> > snd_sof_find_dai(component, (char *)rtd->dai_link-
> > >name);
> > struct snd_sof_dev *sdev =
> > snd_soc_component_get_drvdata(component);
> > - struct sof_dai_private_data *private = dai->private;
> > + struct sof_dai_private_data *private;
> > struct snd_soc_dpcm *dpcm;
> >
> > /* no topology exists for this BE, try a common configuration
> > */
> > @@ -727,6 +727,7 @@ int sof_pcm_dai_link_fixup(struct
> > snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_pa
> > /* read format from topology */
> > snd_mask_none(fmt);
> >
> > + private = dai->private;
> > switch (private->comp_dai->config.frame_fmt) {
> > case SOF_IPC_FRAME_S16_LE:
> > snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S16_LE);
On Sun, Mar 20, 2022, 12:10 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
The "dia" pointer can be NULL, so handle that condition first before storing "dia->private".
Typo in commit message, it is DAI in code, not dia.
Fixes: 839e484f9e17 ("ASoC: SOF: make struct snd_sof_dai IPC agnostic") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- sound/soc/sof/pcm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 1661b0bc6f12..71f5bce0c4c7 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -702,7 +702,7 @@ int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_pa struct snd_sof_dai *dai = snd_sof_find_dai(component, (char *)rtd->dai_link->name); struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component); - struct sof_dai_private_data *private = dai->private; + struct sof_dai_private_data *private; struct snd_soc_dpcm *dpcm;
/* no topology exists for this BE, try a common configuration */ @@ -727,6 +727,7 @@ int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_pa /* read format from topology */ snd_mask_none(fmt);
+ private = dai->private; switch (private->comp_dai->config.frame_fmt) { case SOF_IPC_FRAME_S16_LE: snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S16_LE); -- 2.20.1
_______________________________________________ Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
participants (4)
-
Curtis Malainey -
Dan Carpenter -
Pierre-Louis Bossart -
Ranjani Sridharan