linux-kernel - Re: [PATCH] mm: fix potential invalid pointer dereference in kmemdup()

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: <4c223e9c-2d43-fb8f-7ac9-ad2121914170@redhat.com>
Date:   Tue, 7 Mar 2023 11:04:37 +0100
From:   David Hildenbrand <david@...hat.com>
To:     Xujun Leng <lengxujun2007@....com>, akpm@...ux-foundation.org
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: fix potential invalid pointer dereference in
 kmemdup()

On 07.03.23 10:03, Xujun Leng wrote:
> If kmemdup() was called with src == NULL, then memcpy() source address
> is fatal, and if kmemdup() was called with len == 0, kmalloc_track_caller()
> will return ZERO_SIZE_PTR to variable p, then memcpy() destination address
> is fatal. Both 2 cases will cause an invalid pointer dereference.
> 

"fix" in subject implies that there is actually a case broken. Is there, 
or is this rather a "sanitize" ?

> Signed-off-by: Xujun Leng <lengxujun2007@....com>
> ---
>   mm/util.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/mm/util.c b/mm/util.c
> index dd12b9531ac4..d1a3b3d2988e 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -128,6 +128,9 @@ void *kmemdup(const void *src, size_t len, gfp_t gfp)
>   {
>   	void *p;
>   
> +	if (!src || len == 0)
> +		return NULL;
> +
>   	p = kmalloc_track_caller(len, gfp);
>   	if (p)
>   		memcpy(p, src, len);

Why should we take care of kmemdup(), but not memdup_user() ? Shouldn't 
it suffer from similar problems?

-- 
Thanks,

David / dhildenb