Re: [PATCH] arm: use mmap_write_(un)lock for copy_to_user

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Sat, Sep 26, 2020 at 09:28:54PM +0200, Christian Lamparter wrote:
> changes ARM's copy_to_user to use mmap_*write*_lock
> variants. This is because the data is written to
> user-space and not read.

The "read" lock is there to ensure that the page tables are not changed
(e.g. due to a page fault in another thread) while we are making changes
to the page. It is a "read" lock because we can tolerate other threads
reading the page tables and mm structures, but not making changes to
those structures.

This has nothing to do with whether we are reading or writing userspace.

Therefore, your patch is incorrect.

What problem are you seeing?

> 
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
>  arch/arm/lib/uaccess_with_memcpy.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/lib/uaccess_with_memcpy.c b/arch/arm/lib/uaccess_with_memcpy.c
> index 106f83a5ea6d..7491c13fdf0e 100644
> --- a/arch/arm/lib/uaccess_with_memcpy.c
> +++ b/arch/arm/lib/uaccess_with_memcpy.c
> @@ -101,7 +101,7 @@ __copy_to_user_memcpy(void __user *to, const void *from, unsigned long n)
>  	atomic = faulthandler_disabled();
>  
>  	if (!atomic)
> -		mmap_read_lock(current->mm);
> +		mmap_write_lock(current->mm);
>  	while (n) {
>  		pte_t *pte;
>  		spinlock_t *ptl;
> @@ -109,11 +109,11 @@ __copy_to_user_memcpy(void __user *to, const void *from, unsigned long n)
>  
>  		while (!pin_page_for_write(to, &pte, &ptl)) {
>  			if (!atomic)
> -				mmap_read_unlock(current->mm);
> +				mmap_write_unlock(current->mm);
>  			if (__put_user(0, (char __user *)to))
>  				goto out;
>  			if (!atomic)
> -				mmap_read_lock(current->mm);
> +				mmap_write_lock(current->mm);
>  		}
>  
>  		tocopy = (~(unsigned long)to & ~PAGE_MASK) + 1;
> @@ -133,7 +133,7 @@ __copy_to_user_memcpy(void __user *to, const void *from, unsigned long n)
>  			spin_unlock(ptl);
>  	}
>  	if (!atomic)
> -		mmap_read_unlock(current->mm);
> +		mmap_write_unlock(current->mm);
>  
>  out:
>  	return n;
> -- 
> 2.28.0
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel