[PATCH 1/3] usb: chipidea: Fix DMA overwrite for Tegra
Michał Mirosław
mirq-linux at rere.qmqm.pl
Mon Sep 25 16:53:39 PDT 2023
On Mon, Sep 25, 2023 at 07:45:22PM +0800, Peter Chen wrote:
> On 23-09-23 21:41:55, Michał Mirosław wrote:
> > Tegra USB controllers seem to issue DMA in doubleword-sized chunks and thus
> > may write past the buffer provided. This is detected by SLUB:
> >
> > =============================================================================
> > BUG kmalloc-64 (Tainted: G B ): kmalloc Redzone overwritten
> > -----------------------------------------------------------------------------
> >
> > 0x8555cd02-0x8555cd03 @offset=3330. First byte 0x0 instead of 0xcc
> > Allocated in usb_get_status+0x2b/0xac age=1 cpu=3 pid=41
> > __kmem_cache_alloc_node+0x12f/0x1e4
> > __kmalloc+0x33/0x8c
> > usb_get_status+0x2b/0xac
> > hub_probe+0x5e9/0xcec
> > usb_probe_interface+0xbf/0x21c
> > really_probe+0xa5/0x2c4
> > __driver_probe_device+0x75/0x174
> > driver_probe_device+0x31/0x94
> > __device_attach_driver+0x65/0xc0
> > bus_for_each_drv+0x4b/0x74
> > __device_attach+0x69/0x120
> > bus_probe_device+0x65/0x6c
> > device_add+0x48b/0x5f8
> > usb_set_configuration+0x37b/0x6b4
> > usb_generic_driver_probe+0x37/0x68
> > usb_probe_device+0x35/0xb4
> > Slab 0xbf622b80 objects=21 used=18 fp=0x8555cdc0 flags=0x800(slab|zone=0)
> > Object 0x8555cd00 @offset=3328 fp=0x00000000
> >
> > Redzone 8555ccc0: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................
> > Redzone 8555ccd0: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................
> > Redzone 8555cce0: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................
> > Redzone 8555ccf0: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................
> > Object 8555cd00: 01 00 00 00 cc cc cc cc cc cc cc cc cc cc cc cc ................
> > Object 8555cd10: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................
> > Object 8555cd20: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................
> > Object 8555cd30: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................
> > Redzone 8555cd40: cc cc cc cc ....
> > Padding 8555cd74: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZ
> > CPU: 3 PID: 41 Comm: kworker/3:1 Tainted: G B 6.6.0-rc1mq-00118-g59786f827ea1 #1115
> > Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> > Workqueue: usb_hub_wq hub_event
> > [<8010ca28>] (unwind_backtrace) from [<801090a5>] (show_stack+0x11/0x14)
> > [<801090a5>] (show_stack) from [<805da2fb>] (dump_stack_lvl+0x4d/0x7c)
> > [<805da2fb>] (dump_stack_lvl) from [<8026464f>] (check_bytes_and_report+0xb3/0xe4)
> > [<8026464f>] (check_bytes_and_report) from [<802648e1>] (check_object+0x261/0x290)
> > [<802648e1>] (check_object) from [<802671b1>] (free_to_partial_list+0x105/0x3f8)
> > [<802671b1>] (free_to_partial_list) from [<80268613>] (__kmem_cache_free+0x103/0x128)
> > [<80268613>] (__kmem_cache_free) from [<80425a67>] (usb_get_status+0x73/0xac)
> > [<80425a67>] (usb_get_status) from [<80421b31>] (hub_probe+0x5e9/0xcec)
> > [<80421b31>] (hub_probe) from [<80428bbb>] (usb_probe_interface+0xbf/0x21c)
> > [<80428bbb>] (usb_probe_interface) from [<803ee13d>] (really_probe+0xa5/0x2c4)
> > [<803ee13d>] (really_probe) from [<803ee3d1>] (__driver_probe_device+0x75/0x174)
> > [<803ee3d1>] (__driver_probe_device) from [<803ee501>] (driver_probe_device+0x31/0x94)
> > usb 1-1: device descriptor read/8, error -71
> >
> > Fixes: fc53d5279094 ("usb: chipidea: tegra: Support host mode")
> > Signed-off-by: Michał Mirosław <mirq-linux at rere.qmqm.pl>
> > ---
> > drivers/usb/chipidea/host.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > index 08af26b762a2..abddd39d1ff1 100644
> > --- a/drivers/usb/chipidea/host.c
> > +++ b/drivers/usb/chipidea/host.c
> > @@ -411,12 +411,13 @@ static int ci_hdrc_alloc_dma_aligned_buffer(struct urb *urb, gfp_t mem_flags)
> > const unsigned int ci_hdrc_usb_dma_align = 32;
> > size_t kmalloc_size;
> >
> > - if (urb->num_sgs || urb->sg || urb->transfer_buffer_length == 0 ||
> > - !((uintptr_t)urb->transfer_buffer & (ci_hdrc_usb_dma_align - 1)))
> > + if (urb->num_sgs || urb->sg || urb->transfer_buffer_length == 0)
> > + return 0;
> > + if (!((uintptr_t)urb->transfer_buffer & (ci_hdrc_usb_dma_align - 1)) && !(urb->transfer_buffer_length & 3))
> > return 0;
> >
> > /* Allocate a buffer with enough padding for alignment */
> > - kmalloc_size = urb->transfer_buffer_length +
> > + kmalloc_size = ALIGN(urb->transfer_buffer_length, 4) +
> > sizeof(struct ci_hdrc_dma_aligned_buffer) +
> > ci_hdrc_usb_dma_align - 1;
> >
>
> Would you please explain why you make these changes?
Can you point out what's unclear in the commit message? This is to fix
a buffer overflow by DMA from the USB controller as it seems to write
data 32-bit word at a time. What the patch does is extend the workaround
code to account for the extra room needed at the tail of the buffer.
Best Regards
Michał Mirosław
More information about the linux-arm-kernel
mailing list