On Sat, 18 Nov 2017, Fredrik Noring wrote: > Hello USB maintainers, > > I'm porting kernel support for the Sony PlayStation 2 from v2.6.35 to v4.14, > and a part of this effort is to port the PS2 OHCI driver. USB keyboards seem > to work quite well with v4.14, but USB mass storage devices cause kernel > crashes. > > Any help in finding the cause of this crash, as well as comments on how to > improve this provisional driver are very much welcome. The crash trace is: > > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 34 at drivers/usb/core/hcd.c:1390 hcd_alloc_coherent+0x4c/0xc8 > Modules linked in: > CPU: 0 PID: 34 Comm: usb-storage Not tainted 4.14.0+ #718 > Stack : 00000000 80402608 00000003 0000000b 805b3b98 805611c4 00000094 80062b8c > 00000009 0000056e 81e9fc00 805b0000 00000094 10018c00 81f51b10 00000001 > 00000000 00000000 805b0000 00000001 0000000a 00000003 00000001 3a555043 > 80436790 0000000f 81f519c8 81f51990 00000000 00000000 802e544c 80517464 > 00000009 0000056e 81e9fc00 805b0000 00000003 ffffee7f 00000000 805b0000 > ... > Call Trace: > [<80021104>] show_stack+0x74/0x104 > [<80036310>] __warn+0x114/0x11c > [<800363ac>] warn_slowpath_null+0x1c/0x30 > [<802e544c>] hcd_alloc_coherent+0x4c/0xc8 > [<802e6160>] usb_hcd_map_urb_for_dma+0x524/0x580 > [<802e729c>] usb_hcd_submit_urb+0x7d8/0x7e0 > [<802e95d8>] usb_sg_wait+0x14c/0x1a0 > [<802fb0f0>] usb_stor_bulk_transfer_sglist.part.1+0xac/0x124 > [<802fb4b4>] usb_stor_bulk_srb+0x40/0x60 > [<802fb8a8>] usb_stor_Bulk_transport+0x160/0x37c > [<802fbcec>] usb_stor_invoke_transport+0x3c/0x500 > [<802fd220>] usb_stor_control_thread+0x260/0x2a0 > [<8004ef64>] kthread+0x138/0x140 > [<8001b578>] ret_from_kernel_thread+0x14/0x1c > ---[ end trace 2be087478be6627e ]--- > > This particular WARN_ON_ONCE is explained here: > > commit 4307a28eb0128417d9a2b9d858d2bce70ee5b383 > Author: Andrea Righi <arighi@xxxxxxxxxxx> > Date: Mon Jun 28 16:56:45 2010 +0200 > > USB: EHCI: fix NULL pointer dererence in HCDs that use HCD_LOCAL_MEM > > If we use the HCD_LOCAL_MEM flag and dma_declare_coherent_memory() to > enforce the host controller's local memory utilization we also need to > disable native scatter-gather support, otherwise hcd_alloc_coherent() in > map_urb_for_dma() is called with urb->transfer_buffer == NULL, that > triggers a NULL pointer dereference. > > We can also consider to add a WARN_ON() and return an error code to > better catch this problem in the future. > > At the moment no driver seems to hit this bug, so I should > consider this a low-priority fix. > > Signed-off-by: Andrea Righi <arighi@xxxxxxxxxxx> > Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx> > > I've tried to disable scatter-gather, as suggested, by: > > diff --git a/drivers/usb/host/ohci-ps2.c b/drivers/usb/host/ohci-ps2.c > index 066d6a9c7ccb..eaa099aa0740 100755 > --- a/drivers/usb/host/ohci-ps2.c > +++ b/drivers/usb/host/ohci-ps2.c > @@ -25,6 +25,9 @@ static int ohci_ps2_start(struct usb_hcd *hcd) > > ohci_hcd_init(ohci); > ohci_init(ohci); > + > + hcd->self.sg_tablesize = 0; > + > ohci_run(ohci); > hcd->state = HC_STATE_RUNNING; > return 0; > > With the patch above, the kernel successfully detects USB mass storage > devices. However, it also causes another crash: > > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 15 at ./include/linux/dma-mapping.h:530 hcd_buffer_free+0x130/0x200 > Modules linked in: > CPU: 0 PID: 15 Comm: kworker/0:1 Not tainted 4.14.0+ #719 > Workqueue: usb_hub_wq hub_event > Stack : 81f3b5b8 80402608 00000003 0000000b 000000f6 805611c4 81ccc280 804f398c > 802ee5e0 804fd520 00000009 00000212 00000001 10058400 81c09a38 00000001 > 00000000 00000000 00000000 00000003 0000000a 00000002 00000001 00000000 > 00000001 00003368 81c09878 81c09840 00000000 00000000 802ee5e0 804fd520 > 00000009 00000212 00000001 fffffff3 00000002 00000000 00000000 805b0000 > ... > Call Trace: > [<80021104>] show_stack+0x74/0x104 > [<80036310>] __warn+0x114/0x11c > [<800363ac>] warn_slowpath_null+0x1c/0x30 > [<802ee5e0>] hcd_buffer_free+0x130/0x200 > [<802e56ec>] usb_hcd_unmap_urb_for_dma+0x160/0x16c > [<802e576c>] __usb_hcd_giveback_urb+0x54/0xf0 > [<802f5d64>] finish_urb+0x98/0x138 > [<802f716c>] ohci_work+0x14c/0x494 > [<802f9878>] ohci_irq+0x13c/0x2e4 > [<802e4e0c>] usb_hcd_irq+0x2c/0x44 > [<8006366c>] __handle_irq_event_percpu+0x98/0x158 > [<8006374c>] handle_irq_event_percpu+0x20/0x64 > [<800637cc>] handle_irq_event+0x3c/0x70 > [<80067024>] handle_level_irq+0xe4/0x120 > [<80062d70>] generic_handle_irq+0x28/0x38 > [<80409b58>] do_IRQ+0x18/0x24 > ---[ end trace fa1f0201799de649 ]--- This is caused by a deficiency in the DMA core: dma_free_coherent() wants interrupts to be enabled when it is called. I'm not sure how the other host controller drivers cope with this. > Please find the (provisional) patch for the PS2 OHCI driver below. Be aware that your driver should utilize ohci_init_driver(), like several of the other platform-specific OHCI drivers do. Unless there's some very good reason, new drivers should never use the old interface. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
