linux-kernel - Re: [PATCH 1/4] mm/mlock: return EINVAL for illegal user memory range in mlock

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: <cbda63de-6eb4-30e9-c19e-be58d5c325ec@huawei.com>
Date:   Sat, 10 Dec 2022 11:09:48 +0800
From:   mawupeng <mawupeng1@...wei.com>
To:     <akpm@...ux-foundation.org>
CC:     <mawupeng1@...wei.com>, <linux-mm@...ck.org>,
        <linux-kernel@...r.kernel.org>, <kuleshovmail@...il.com>,
        <aneesh.kumar@...ux.ibm.com>, <clameter@....com>
Subject: Re: [PATCH 1/4] mm/mlock: return EINVAL for illegal user memory range
 in mlock



On 2022/12/5 11:41, Wupeng Ma wrote:
> From: Ma Wupeng <mawupeng1@...wei.com>
> 
> While testing mlock, we have a problem if the len of mlock is ULONG_MAX.
> The return value of mlock is zero. But nothing will be locked since the
> len in do_mlock overflows to zero due to the following code in mlock:
> 
>   len = PAGE_ALIGN(len + (offset_in_page(start)));
> 
> The same problem happens in munlock.
> 
> Since TASK_SIZE is the maximum user space address. The start or len of
> mlock shouldn't be bigger than this. Function access_ok can be used to
> check this issue, so return -EINVAL if bigger.
> 
> Signed-off-by: Ma Wupeng <mawupeng1@...wei.com>
> ---
>  mm/mlock.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/mm/mlock.c b/mm/mlock.c
> index 7032f6dd0ce1..b9422a62a4cf 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -575,6 +575,9 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla
>  	if (!can_do_mlock())
>  		return -EPERM;
>  
> +	if (unlikely(!access_ok((void __user *)start, len)))
> +		return -EINVAL;

When we are runing ltp testcase, a error occurs on mlock[1]. It seems that
ENOMEN is expencted for this testcase.

In the manual of mlock[2]

       ENOMEM (mlock(), mlock2(), and munlock()) Some of the specified
              address range does not correspond to mapped pages in the
              address space of the process.

ENOMEM seem more appropriate for this situation?

[1] https://github.com/linux-test-project/ltp/blob/20220930/testcases/open_posix_testsuite/conformance/interfaces/mlock/8-1.c
[2] https://man7.org/linux/man-pages/man2/mlock.2.html

> +
>  	len = PAGE_ALIGN(len + (offset_in_page(start)));
>  	start &= PAGE_MASK;
>  
> @@ -635,6 +638,9 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)
>  
>  	start = untagged_addr(start);
>  
> +	if (unlikely(!access_ok((void __user *)start, len)))
> +		return -EINVAL;
> +
>  	len = PAGE_ALIGN(len + (offset_in_page(start)));
>  	start &= PAGE_MASK;
>