linux-kernel - Re: [PATCH] mm/util: make memdup_user_nul() similar to memdup

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: <202501090937.221C3F6BA@keescook>
Date: Thu, 9 Jan 2025 09:38:19 -0800
From: Kees Cook <kees@...nel.org>
To: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	linux-mm <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm/util: make memdup_user_nul() similar to memdup_user()

On Sat, Dec 21, 2024 at 04:47:29PM +0900, Tetsuo Handa wrote:
> Since the string data to copy from userspace is likely less than PAGE_SIZE
> bytes, replace GFP_KERNEL with GFP_USER like commit 6c2c97a24f09
> ("memdup_user(): switch to GFP_USER") does and add __GFP_NOWARN like commit
> 6c8fcc096be9 ("mm: don't let userspace spam allocations warnings") does.
> Also, use dedicated slab buckets like commit d73778e4b867 ("mm/util: Use
> dedicated slab buckets for memdup_user()") does.
> 
> Reported-by: syzbot+7e12e97b36154c54414b@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7e12e97b36154c54414b
> Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>

Thanks for this! Yes, please. With the commit log updated to answer
akpm's questions:

Acked-by: Kees Cook <kees@...nel.org>

-Kees

> ---
>  mm/util.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/mm/util.c b/mm/util.c
> index c1c3b06ab4f9..60aa40f612b8 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -297,12 +297,7 @@ void *memdup_user_nul(const void __user *src, size_t len)
>  {
>  	char *p;
>  
> -	/*
> -	 * Always use GFP_KERNEL, since copy_from_user() can sleep and
> -	 * cause pagefault, which makes it pointless to use GFP_NOFS
> -	 * or GFP_ATOMIC.
> -	 */
> -	p = kmalloc_track_caller(len + 1, GFP_KERNEL);
> +	p = kmem_buckets_alloc_track_caller(user_buckets, len + 1, GFP_USER | __GFP_NOWARN);
>  	if (!p)
>  		return ERR_PTR(-ENOMEM);
>  
> -- 
> 2.43.5

-- 
Kees Cook