linux-kernel - Re: [PATCH] ASoC: wcd934x: Fix a incorrect use of kstrndup

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] [day] [month] [year] [list]

Message-ID: <03a2bb9d-972d-b481-f03b-d50bab422f78@metafoo.de>
Date:   Tue, 14 Dec 2021 18:42:38 +0100
From:   Lars-Peter Clausen <lars@...afoo.de>
To:     Miaoqian Lin <linmq006@...il.com>
Cc:     Banajit Goswami <bgoswami@...eaurora.org>,
        alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
        Takashi Iwai <tiwai@...e.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Subject: Re: [PATCH] ASoC: wcd934x: Fix a incorrect use of kstrndup

On 12/14/21 4:25 PM, Miaoqian Lin wrote:
> In wcd934x_codec_enable_dec(), widget_name is allocated by kstrndup().
> However, according to doc: "Note: Use kmemdup_nul() instead if the size
> is known exactly." So we should use kmemdup_nul() here instead of
> kstrndup(). It's similar to CVE-2019-12454.
>
> Signed-off-by: Miaoqian Lin <linmq006@...il.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 e63c6b723d76..c6677cfbce59 100644
> --- a/sound/soc/codecs/wcd934x.c
> +++ b/sound/soc/codecs/wcd934x.c
> @@ -5005,7 +5005,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);

Thinking a bit more about it, this is wrong. The source string is 
shorter than 15 character. So with this change you are copying past the 
end of the string, which depending on where in memory the string is 
placed can cause undefined behavior.