linux-kernel - Re: [PATCH for-4.11] ASoC: don't dereference NULL pcm

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87zigvz4q3.wl%kuninori.morimoto.gx@renesas.com>
Date:   Thu, 9 Mar 2017 00:17:41 +0000
From:   Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
To:     Brian Norris <briannorris@...omium.org>
CC:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        <linux-kernel@...r.kernel.org>, Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, <alsa-devel@...a-project.org>
Subject: Re: [PATCH for-4.11] ASoC: don't dereference NULL pcm_{new,free}


Hi Brian

Thank you for your patch

> Not all platform drivers have pcm_{new,free} callbacks. Seen with a
> "snd-soc-dummy" codec from sound/soc/rockchip/rk3399_gru_sound.c.
(snip)
> Fixes: 99b04f4c4051 ("ASoC: add Component level pcm_new/pcm_free")
> Signed-off-by: Brian Norris <briannorris@...omium.org>
> ---
> I'm really not that familiar with this subsystem... but this does fix the
> crash seen here.
> 
>  sound/soc/soc-core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 6dca408faae3..2722bb0c5573 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -3326,7 +3326,10 @@ static int snd_soc_platform_drv_pcm_new(struct snd_soc_pcm_runtime *rtd)
>  {
>  	struct snd_soc_platform *platform = rtd->platform;
>  
> -	return platform->driver->pcm_new(rtd);
> +	if (platform->driver->pcm_new)
> +		return platform->driver->pcm_new(rtd);
> +	else
> +		return 0;
>  }
>  
>  static void snd_soc_platform_drv_pcm_free(struct snd_pcm *pcm)
> @@ -3334,7 +3337,8 @@ static void snd_soc_platform_drv_pcm_free(struct snd_pcm *pcm)
>  	struct snd_soc_pcm_runtime *rtd = pcm->private_data;
>  	struct snd_soc_platform *platform = rtd->platform;
>  
> -	platform->driver->pcm_free(pcm);
> +	if (platform->driver->pcm_free)
> +		platform->driver->pcm_free(pcm);
>  }

It is a littlle bit strange for me.
commit 99b04f4c4051 has below code. This means, if platform doesn't have pcm_new/free callback,
component doesn't have snd_soc_platform_drv_pcm_new/free.
But your case, platform doesn't have pcm_new/free, but component had it ?

...
@@ -3181,6 +3198,10 @@ int snd_soc_add_platform(struct device *dev, struct snd_soc_platform *platform,
 		platform->component.probe = snd_soc_platform_drv_probe;
 	if (platform_drv->remove)
 		platform->component.remove = snd_soc_platform_drv_remove;
+	if (platform_drv->pcm_new)
+		platform->component.pcm_new = snd_soc_platform_drv_pcm_new;
+	if (platform_drv->pcm_free)
+		platform->component.pcm_free = snd_soc_platform_drv_pcm_free;
 
 #ifdef CONFIG_DEBUG_FS
 	platform->component.debugfs_prefix = "platform";