USB: usbmon: end ugly tricks with DMA peeking
This patch fixes crashes when usbmon attempts to access GART aperture.
The old code attempted to take a bus address and convert it into a
virtual address, which clearly was impossible on systems with actual
IOMMUs. Let us not persist in this foolishness, and use transfer_buffer
in all cases instead.
I think downsides are negligible. The ones I see are:
- A driver may pass an address of one buffer down as transfer_buffer,
and entirely different entity mapped for DMA, resulting in misleading
output of usbmon. Note, however, that PIO based controllers would
do transfer the same data that usbmon sees here.
- Out of tree drivers may crash usbmon if they store garbage in
transfer_buffer. I inspected the in-tree drivers, and clarified
the documentation in comments.
- Drivers that use get_user_pages will not be possible to monitor.
I only found one driver with this problem (drivers/staging/rspiusb).
- Same happens with with usb_storage transferring from highmem, but
it works fine on 64-bit systems, so I think it's not a concern.
At least we don't crash anymore.
Why didn't we do this in 2.6.10? That's because back in those days
it was popular not to fill in transfer_buffer, so almost all
traffic would be invisible (e.g. all of HID was like that).
But now, the tree is almost 100% PIO friendly, so we can do the
right thing at last.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c
index 0f7a30b..dfdc43e 100644
--- a/drivers/usb/mon/mon_bin.c
+++ b/drivers/usb/mon/mon_bin.c
@@ -220,9 +220,8 @@
/*
* This is a "chunked memcpy". It does not manipulate any counters.
- * But it returns the new offset for repeated application.
*/
-unsigned int mon_copy_to_buff(const struct mon_reader_bin *this,
+static void mon_copy_to_buff(const struct mon_reader_bin *this,
unsigned int off, const unsigned char *from, unsigned int length)
{
unsigned int step_len;
@@ -247,7 +246,6 @@
from += step_len;
length -= step_len;
}
- return off;
}
/*
@@ -400,15 +398,8 @@
unsigned int offset, struct urb *urb, unsigned int length)
{
- if (urb->dev->bus->uses_dma &&
- (urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
- mon_dmapeek_vec(rp, offset, urb->transfer_dma, length);
- return 0;
- }
-
if (urb->transfer_buffer == NULL)
return 'Z';
-
mon_copy_to_buff(rp, offset, urb->transfer_buffer, length);
return 0;
}
@@ -635,7 +626,6 @@
spin_lock_init(&rp->b_lock);
init_waitqueue_head(&rp->b_wait);
mutex_init(&rp->fetch_lock);
-
rp->b_size = BUFF_DFL;
size = sizeof(struct mon_pgmap) * (rp->b_size/CHUNK_SIZE);