Re: [PATCH] sound: soc: wcd934x: fix an incorrect use of kstrndup()

[Amadeusz Sławiński]

On 1/18/2024 8:52 AM, Fullway Wang wrote:
> In wcd934x_codec_enable_dec(), kstrndup() is used to alloc memory.
> However, kmemdup_nul() should be used instead with the size known.
>
> This is similar to CVE-2019-12454 which was fixed in commit
> a549881.
>
> Signed-off-by: Fullway Wang <fullwaywang@xxxxxxxxxxx>
> ---
>   sound/soc/codecs/wcd934x.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/codecs/wcd934x.c b/sound/soc/codecs/wcd934x.c
> index 1b6e376f3833..4565a0e1877f 100644
> --- a/sound/soc/codecs/wcd934x.c
> +++ b/sound/soc/codecs/wcd934x.c
> @@ -4990,7 +4990,7 @@ static int wcd934x_codec_enable_dec(struct snd_soc_dapm_widget *w,
>   	char *dec;
>   	u8 hpf_coff_freq;
>   
> -	widget_name = kstrndup(w->name, 15, GFP_KERNEL);
> +	widget_name = kmemdup_nul(w->name, 15, GFP_KERNEL);
>   	if (!widget_name)
>   		return -ENOMEM;
>   

This change looks weird to me, and looking at code I'm even more 
confused. As far as I can tell code tries to find a number in w->name. 
It shouldn't need a duplicate string for this, it can search in existing 
one, same applies to referenced commit.

And when it comes to a549881 as already mentioned in 
https://lore.kernel.org/alsa-devel/7573d8ce-7160-39b1-8901-be9155c451a1@xxxxxxx/ 
the size is at most 15 not equal to, so change was misguided. If you 
look at actual code widget name is around 8 characters.

Also it seems like consensus was that CVE was bogus and it was a wrong 
change:
https://lore.kernel.org/alsa-devel/20190618230527.GE6248@lindsey/
there was a revert send, but it seems like it was not applied:
https://lore.kernel.org/alsa-devel/20190612133040.5kgtio7gidxx64gh@xxxxxxxxxxxxxxxxxxxxxxxxxxx/