On Thu, 10 Nov 2016, Petr Vandrovec wrote: > Hi, > I've built 4.9.0-rc4 with CONFIG_VMAP_STACK=y, and while I can use bulk > storage drives, I cannot use CB-based USB floppy drive: kernel complains > (once) that DMA address is invalid, and after that log is full of device > resets: > > Nov 7 12:18:42 petr-dev3 kernel: [ 25.929808] scsi 12:0:0:1: Direct-Access > Generic USB SM/MS/SD 1.00 PQ: 0 ANSI: 0 > Nov 7 12:18:42 petr-dev3 kernel: [ 25.969520] scsi 13:0:0:0: Direct-Access > CITIZEN X1DE-USB 1002 PQ: 0 ANSI: 0 CCS > Nov 7 12:18:42 petr-dev3 kernel: [ 25.978504] sd 13:0:0:0: Attached scsi > generic sg9 type 0 > Nov 7 12:18:42 petr-dev3 kernel: [ 25.981037] ------------[ cut here > ]------------ > Nov 7 12:18:42 petr-dev3 kernel: [ 25.981042] WARNING: CPU: 0 PID: 3829 at > /bhavesh/usr/src/git/libata-pv3/drivers/usb/core/hcd.c:1584 > usb_hcd_map_urb_for_dma+0x425/0x540 > Nov 7 12:18:42 petr-dev3 kernel: [ 25.981042] transfer buffer not dma > capable ...
> Problem is that CB/CBI code passes CDB directly to the HCD for transmission - > and unfortunately SCSI error handling code creates CDB on stack > (scsi_eh_prep_cmnd > uses scsi_eh_save structure for CDB, and as far as I can tell, everybody > creates > scsi_eh_save structure on stack, rather than in kmalloc memory). > > As no other drivers does DMA directly from CDB field, I think if I try to > modify > USB storage code to not create scsi_eh_save on stack it will get broken again > anyway when someone else stores CDB on stack, so I went ahead with memcpy > solution instead: now CB/CBI code copies CDB into its private iobuf, and sends > command from there. That's the right thing to do. > With this fix in place USB floppy works again... > > Thanks, > Petr Vandrovec > > > > > > commit 07da00a2bb122bc7ffb287aa80f58714a17b1d9c > Author: Petr Vandrovec <[email protected]> > Date: Wed Nov 9 14:46:35 2016 -0800 > > Fix UFI USB storage devices with vmalloced stacks > > Some code (all error handling) submits CDBs that are allocated > on stack. This breaks with UFI code that tries to create URB > directly from SCSI command buffer - which happens to be in > vmalloced memory with vmalloced kernel stacks. > > Let's make copy of cmd in usb_stor_CB_transport - it is pretty > cheap, and I cannot find any easy way to modify SCSI error > handling to not use on-stack structure for error handling > command. > > Signed-off-by: Petr Vandrovec <[email protected]> > > diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c > index ffd0867..e46833b 100644 > --- a/drivers/usb/storage/transport.c > +++ b/drivers/usb/storage/transport.c > @@ -954,10 +954,16 @@ int usb_stor_CB_transport(struct scsi_cmnd *srb, struct > us_data *us) > > /* COMMAND STAGE */ > /* let's send the command via the control pipe */ > + /* > + * Command is sometime (f.e. after scsi_eh_prep_cmnd) on the stack. > + * Stack may be vmallocated. So no DMA for us. Make a copy. > + */ > + BUG_ON(srb->cmd_len > US_IOBUF_SIZE); Don't bother with the BUG_ON. The usb_stor_Bulk_transport() routine doesn't check for this, and it needs more buffer space than you do. > + memcpy(us->iobuf, srb->cmnd, srb->cmd_len); > result = usb_stor_ctrl_transfer(us, us->send_ctrl_pipe, > US_CBI_ADSC, > USB_TYPE_CLASS | USB_RECIP_INTERFACE, 0, > - us->ifnum, srb->cmnd, srb->cmd_len); > + us->ifnum, us->iobuf, srb->cmd_len); > > /* check the return code for the command */ > usb_stor_dbg(us, "Call to usb_stor_ctrl_transfer() returned %d\n", Good work. Please submit an updated patch, following the format described in Documentation/SubmittingPatches (no preliminary text, don't indent the patch description, include a --- line after the Signed-off-by, and so on), and be sure to cc: Greg KH <[email protected]>. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
