Messages in this thread | | | Subject | Re: [PATCH] usb: usbfs: correct kernel->user page attribute mismatch | From | Robin Murphy <> | Date | Fri, 1 May 2020 16:36:17 +0100 |
| |
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@arm.com> >> --- >> 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@kroah.com/
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.
| |