On Sun, Jul 10, 2005 at 08:25:39PM -0700, Pete Zaitcev wrote:
Hey Pete,
We have usbmon enabled now and it's good, but usb-storage moved
away
from under me a little bit. It now maps everything for DMA beforehand,
which makes usbmon to miss the SCSI commands and replies.
I am going to address this problem in usbmon, where it belongs, but
it requires some work which is not done.
What do you think about carrying the attached patch for a few months
(I hope to turn around sooner, but you know how that works!)
I'm puzzled (mostly due to my cluelessness in this area)..
@@ -948,8 +948,9 @@ int usb_stor_Bulk_max_lun(struct us_data
int usb_stor_Bulk_transport(struct scsi_cmnd *srb, struct us_data *us)
{
- struct bulk_cb_wrap *bcb = (struct bulk_cb_wrap *) us->iobuf;
- struct bulk_cs_wrap *bcs = (struct bulk_cs_wrap *) us->iobuf;
+ /* Offset into iobuf a little in order to defeat pre-set DMA */
+ struct bulk_cb_wrap *bcb = (struct bulk_cb_wrap *) (us->iobuf + 4);
+ struct bulk_cs_wrap *bcs = (struct bulk_cs_wrap *) (us->iobuf + 4);
I assume this is being done to defeat the check..
/* we assume that if transfer_buffer isn't us->iobuf then it
* hasn't been mapped for DMA. Yes, this is clunky, but it's
* easier than always having the caller tell us whether the
* transfer buffer has already been mapped. */
us->current_urb->transfer_flags =
URB_ASYNC_UNLINK | URB_NO_SETUP_DMA_MAP;
in usb_stor_msg_common() ?
Is there any danger of the rest of the code that uses ->iobuf not being
aware that we're now starting 4 bytes in, and running past the end of the
buffer ?
@@ -960,7 +961,7 @@ int usb_stor_Bulk_transport(struct scsi_
/* Take care of BULK32 devices; set extra byte to 0 */
if ( unlikely(us->flags & US_FL_BULK32)) {
cbwlen = 32;
- us->iobuf[31] = 0;
+ ((unsigned char *)bcb)[31] = 0;
}
If ->iobuf is a ptr to 31 bytes, and bcb starts 4 bytes in, won't this
be writing to the 35th byte of iobuf ?
I'm probably missing something obvious, so hand me a dummys guide to
usb-storage hacking ;)
Dave