The current implementation of usbdev_mmap uses usb_alloc_coherent to
allocate memory pages that will later be mapped into the user space.
Meanwhile, usb_alloc_coherent employs three different methods to
allocate memory, as outlined below:
* If hcd->localmem_pool is non-null, it uses gen_pool_dma_alloc to
allocate memory;
* If DMA is not available, it uses kmalloc to allocate memory;
* Otherwise, it uses dma_alloc_coherent.
However, it should be noted that gen_pool_dma_alloc does not guarantee
that the resulting memory will be page-aligned. Furthermore, trying to
map slab pages (i.e., memory allocated by kmalloc) into the user space
is not resonable and can lead to problems, such as a type confusion bug
when PAGE_TABLE_CHECK=y [1].
To address these issues, this patch introduces hcd_alloc_coherent_pages,
which addresses the above two problems. Specifically,
hcd_alloc_coherent_pages uses gen_pool_dma_alloc_align instead of
gen_pool_dma_alloc to ensure that the memory is page-aligned. To replace
kmalloc, hcd_alloc_coherent_pages directly allocates pages by calling
__get_free_pages.
Reported-by: [email protected]
Closes: https://lore.kernel.org/lkml/[email protected]/ [1]
Fixes: f7d34b445abc ("USB: Add support for usbfs zerocopy.")
Fixes: ff2437befd8f ("usb: host: Fix excessive alignment restriction for local memory allocations")
Cc: [email protected]
Signed-off-by: Ruihan Li <[email protected]>
---
drivers/usb/core/buffer.c | 41 +++++++++++++++++++++++++++++++++++++++
drivers/usb/core/devio.c | 9 +++++----
include/linux/usb/hcd.h | 5 +++++
3 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index fbb087b72..268ccbec8 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -172,3 +172,44 @@ void hcd_buffer_free(
}
dma_free_coherent(hcd->self.sysdev, size, addr, dma);
}
+
+void *hcd_buffer_alloc_pages(struct usb_hcd *hcd,
+ size_t size, gfp_t mem_flags, dma_addr_t *dma)
+{
+ if (size == 0)
+ return NULL;
+
+ if (hcd->localmem_pool)
+ return gen_pool_dma_alloc_align(hcd->localmem_pool,
+ size, dma, PAGE_SIZE);
+
+ /* some USB hosts just use PIO */
+ if (!hcd_uses_dma(hcd)) {
+ *dma = DMA_MAPPING_ERROR;
+ return (void *)__get_free_pages(mem_flags,
+ get_order(size));
+ }
+
+ return dma_alloc_coherent(hcd->self.sysdev,
+ size, dma, mem_flags);
+}
+
+void hcd_buffer_free_pages(struct usb_hcd *hcd,
+ size_t size, void *addr, dma_addr_t dma)
+{
+ if (!addr)
+ return;
+
+ if (hcd->localmem_pool) {
+ gen_pool_free(hcd->localmem_pool,
+ (unsigned long)addr, size);
+ return;
+ }
+
+ if (!hcd_uses_dma(hcd)) {
+ free_pages((unsigned long)addr, get_order(size));
+ return;
+ }
+
+ dma_free_coherent(hcd->self.sysdev, size, addr, dma);
+}
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index e501a03d6..3936ca7f7 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -186,6 +186,7 @@ static int connected(struct usb_dev_state *ps)
static void dec_usb_memory_use_count(struct usb_memory *usbm, int *count)
{
struct usb_dev_state *ps = usbm->ps;
+ struct usb_hcd *hcd = bus_to_hcd(ps->dev->bus);
unsigned long flags;
spin_lock_irqsave(&ps->lock, flags);
@@ -194,8 +195,8 @@ static void dec_usb_memory_use_count(struct usb_memory *usbm, int *count)
list_del(&usbm->memlist);
spin_unlock_irqrestore(&ps->lock, flags);
- usb_free_coherent(ps->dev, usbm->size, usbm->mem,
- usbm->dma_handle);
+ hcd_buffer_free_pages(hcd, usbm->size,
+ usbm->mem, usbm->dma_handle);
usbfs_decrease_memory_usage(
usbm->size + sizeof(struct usb_memory));
kfree(usbm);
@@ -247,8 +248,8 @@ static int usbdev_mmap(struct file *file, struct vm_area_struct *vma)
goto error_decrease_mem;
}
- mem = usb_alloc_coherent(ps->dev, size, GFP_USER | __GFP_NOWARN,
- &dma_handle);
+ mem = hcd_buffer_alloc_pages(hcd,
+ size, GFP_USER | __GFP_NOWARN, &dma_handle);
if (!mem) {
ret = -ENOMEM;
goto error_free_usbm;
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 094c77eaf..0c7eff91a 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -501,6 +501,11 @@ void *hcd_buffer_alloc(struct usb_bus *bus, size_t size,
void hcd_buffer_free(struct usb_bus *bus, size_t size,
void *addr, dma_addr_t dma);
+void *hcd_buffer_alloc_pages(struct usb_hcd *hcd,
+ size_t size, gfp_t mem_flags, dma_addr_t *dma);
+void hcd_buffer_free_pages(struct usb_hcd *hcd,
+ size_t size, void *addr, dma_addr_t dma);
+
/* generic bus glue, needed for host controllers that don't use PCI */
extern irqreturn_t usb_hcd_irq(int irq, void *__hcd);
--
2.40.1
On Mon, May 15, 2023 at 09:09:55PM +0800, Ruihan Li wrote:
> The current implementation of usbdev_mmap uses usb_alloc_coherent to
> allocate memory pages that will later be mapped into the user space.
> Meanwhile, usb_alloc_coherent employs three different methods to
> allocate memory, as outlined below:
> * If hcd->localmem_pool is non-null, it uses gen_pool_dma_alloc to
> allocate memory;
> * If DMA is not available, it uses kmalloc to allocate memory;
> * Otherwise, it uses dma_alloc_coherent.
>
> However, it should be noted that gen_pool_dma_alloc does not guarantee
> that the resulting memory will be page-aligned. Furthermore, trying to
> map slab pages (i.e., memory allocated by kmalloc) into the user space
> is not resonable and can lead to problems, such as a type confusion bug
> when PAGE_TABLE_CHECK=y [1].
>
> To address these issues, this patch introduces hcd_alloc_coherent_pages,
> which addresses the above two problems. Specifically,
> hcd_alloc_coherent_pages uses gen_pool_dma_alloc_align instead of
> gen_pool_dma_alloc to ensure that the memory is page-aligned. To replace
> kmalloc, hcd_alloc_coherent_pages directly allocates pages by calling
> __get_free_pages.
>
> Reported-by: [email protected]
> Closes: https://lore.kernel.org/lkml/[email protected]/ [1]
> Fixes: f7d34b445abc ("USB: Add support for usbfs zerocopy.")
> Fixes: ff2437befd8f ("usb: host: Fix excessive alignment restriction for local memory allocations")
> Cc: [email protected]
> Signed-off-by: Ruihan Li <[email protected]>
> ---
For parts 1/4 and 2/4:
Acked-by: Alan Stern <[email protected]>
Alan Stern
On Mon, May 15, 2023 at 09:09:55PM +0800, Ruihan Li wrote:
> To address these issues, this patch introduces hcd_alloc_coherent_pages,
> which addresses the above two problems. Specifically,
> hcd_alloc_coherent_pages uses gen_pool_dma_alloc_align instead of
> gen_pool_dma_alloc to ensure that the memory is page-aligned. To replace
> kmalloc, hcd_alloc_coherent_pages directly allocates pages by calling
> __get_free_pages.
This looks reasonable in that it fixes the bug. But I really don't
like how it makes the mess of USB allocation APIs even messier :P
Not really your faul, but someone really needs to look into the usb
memory allocators and DMA mapping, which is tied to that and just as
bad.