Received-SPF: pass (google.com: domain of linux-kernel+bounces-44331-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <8c92d33d-923b-4805-abdf-752e8943072e@linux.intel.com>
Date: Tue, 30 Jan 2024 10:56:41 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] sound: soc: wcd934x: fix an incorrect use of kstrndup()
Content-Language: en-US
To: Fullway Wang <fullwaywang@outlook.com>, broonie@kernel.org,
 srinivas.kandagatla@linaro.org, bgoswami@quicinc.com, tiwai@suse.com
Cc: linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
 fullwaywang@tencent.com
References: <PH7PR20MB59255EF9DFFB022CB1BBB574BF712@PH7PR20MB5925.namprd20.prod.outlook.com>
From: =?UTF-8?Q?Amadeusz_S=C5=82awi=C5=84ski?=
 <amadeuszx.slawinski@linux.intel.com>
In-Reply-To: <PH7PR20MB59255EF9DFFB022CB1BBB574BF712@PH7PR20MB5925.namprd20.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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@outlook.com>
> ---
>   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@suse.cz/ 
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@xylophone.i.decadent.org.uk/