linux-kernel - Re: [PATCH] uio: Fix bus error that access memory mapped by physical

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: <YNLfVX5YVWouP1qD@kroah.com>
Date:   Wed, 23 Jun 2021 09:14:29 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     wubian <wubian@...ontech.com>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH] uio: Fix bus error that access memory mapped by physical

On Wed, Jun 23, 2021 at 02:52:14PM +0800, wubian wrote:
> On the arm64, register the uio driver and map a physical space
> on the pci device to user space, then use memset write data to
> the address space, a bus error will occur. This error is due to
> the dc instruction(cache operation) used in the assembly of memset,
> uio mapping physical memory will call pgprot_noncached() to set
> non-cached and non-buffered, while pgprot_writecombine() has fewer
> restrictions. It does not prohibit write buffer, so replacing
> pgprot_noncached() with pgprot_writecombine() can solve this problem.
> 
> Signed-off-by: wubian <wubian@...ontech.com>
> ---
>  drivers/uio/uio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> index ea96e319c8a0..09b04b20fa30 100644
> --- a/drivers/uio/uio.c
> +++ b/drivers/uio/uio.c
> @@ -739,7 +739,11 @@ static int uio_mmap_physical(struct vm_area_struct *vma)
>  
>  	vma->vm_ops = &uio_physical_vm_ops;
>  	if (idev->info->mem[mi].memtype == UIO_MEM_PHYS)
> +#if defined(CONFIG_ARM64)
> +		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> +#else
>  		vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> +#endif

This feels really wrong, shouldn't stuff like this be handled in the
platform itself and not in the driver?

And why is ARM64 special here?  Why not other arches?  What is odd about
this platform?  We almost never want to use #if in .c files, why is it
ok to do that here?

And is this a bugfix?  If so, what commit does it fix?  Should it go to
stable kernels, and if so, how far back?

I need more information here :)

thanks,

greg k-h