Re: [PATCH] usb: usbfs: correct kernel->user page attribute mismatch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2020-05-01 8:05 am, Greg KH wrote:
On Thu, Apr 30, 2020 at 04:19:22PM -0500, Jeremy Linton wrote:
On arm64, and possibly other architectures, requesting
IO coherent memory may return Normal-NC if the underlying
hardware isn't coherent. If these pages are then
remapped into userspace as Normal, that defeats the
purpose of getting Normal-NC, as well as resulting in
mappings with differing cache attributes.

What is "Normal-NC"?

In particular this happens with libusb, when it attempts
to create zero-copy buffers as is used by rtl-sdr, and

What is "rtl-sdr"

maybe other applications. The result is usually
application death.

So is this a new problem?  Old problem?  Old problem only showing up on
future devices?  On current devices?  I need a hint here as to know if
this is a bugfix or just work to make future devices work properly.


If dma_mmap_attr() is used instead of remap_pfn_range,
the page cache/etc attributes can be matched between the
kernel and userspace.

Signed-off-by: Jeremy Linton <jeremy.linton@xxxxxxx>
---
  drivers/usb/core/devio.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 6833c918abce..1e7458dd6e5d 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -217,6 +217,7 @@ static int usbdev_mmap(struct file *file, struct vm_area_struct *vma)
  {
  	struct usb_memory *usbm = NULL;
  	struct usb_dev_state *ps = file->private_data;
+	struct usb_hcd *hcd = bus_to_hcd(ps->dev->bus);
  	size_t size = vma->vm_end - vma->vm_start;
  	void *mem;
  	unsigned long flags;
@@ -250,9 +251,7 @@ static int usbdev_mmap(struct file *file, struct vm_area_struct *vma)
  	usbm->vma_use_count = 1;
  	INIT_LIST_HEAD(&usbm->memlist);
- if (remap_pfn_range(vma, vma->vm_start,
-			virt_to_phys(usbm->mem) >> PAGE_SHIFT,
-			size, vma->vm_page_prot) < 0) {
+	if (dma_mmap_attrs(hcd->self.sysdev, vma, mem, dma_handle, size, 0)) {

Given that this code has not changed since 2016, how has no one noticed
this issue before?

They have. Here's where the most recent one in my inbox ended, which has breadcrumbs to a couple more:

https://lore.kernel.org/linux-arm-kernel/20190808130525.GA1756@xxxxxxxxx/

Note the author ;)

From memory, all the previous attempts wound up getting stuck on the subtlety that buffers from hcd_alloc() may or may not actually have come from the DMA API. Since then, the localmem_pool rework has probably helped a bit, but I'm not sure we've ever really nailed down whether kmalloc()ed buffers from PIO-mode controllers (i.e. the !hcd_uses_dma() case) can end up down this devio path.

Robin.



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux