diff options
author | Pete Zaitcev <zaitcev@redhat.com> | 2009-06-11 10:53:20 -0400 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@suse.de> | 2009-09-23 09:46:19 -0400 |
commit | 4e9e92003529e5c7bb11281f7c2c9b3fe8858403 (patch) | |
tree | 07169c9a996a119aebb5865a76ff1177afe90a22 /drivers/usb/mon/mon_bin.c | |
parent | f4e2332cfcf900e0a926c4e0fc35f751bcbcaa1b (diff) |
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>
Diffstat (limited to 'drivers/usb/mon/mon_bin.c')
-rw-r--r-- | drivers/usb/mon/mon_bin.c | 12 |
1 files changed, 1 insertions, 11 deletions
diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c index 0f7a30b7d2d1..dfdc43e2e00d 100644 --- a/drivers/usb/mon/mon_bin.c +++ b/drivers/usb/mon/mon_bin.c | |||
@@ -220,9 +220,8 @@ static void mon_free_buff(struct mon_pgmap *map, int npages); | |||
220 | 220 | ||
221 | /* | 221 | /* |
222 | * This is a "chunked memcpy". It does not manipulate any counters. | 222 | * This is a "chunked memcpy". It does not manipulate any counters. |
223 | * But it returns the new offset for repeated application. | ||
224 | */ | 223 | */ |
225 | unsigned int mon_copy_to_buff(const struct mon_reader_bin *this, | 224 | static void mon_copy_to_buff(const struct mon_reader_bin *this, |
226 | unsigned int off, const unsigned char *from, unsigned int length) | 225 | unsigned int off, const unsigned char *from, unsigned int length) |
227 | { | 226 | { |
228 | unsigned int step_len; | 227 | unsigned int step_len; |
@@ -247,7 +246,6 @@ unsigned int mon_copy_to_buff(const struct mon_reader_bin *this, | |||
247 | from += step_len; | 246 | from += step_len; |
248 | length -= step_len; | 247 | length -= step_len; |
249 | } | 248 | } |
250 | return off; | ||
251 | } | 249 | } |
252 | 250 | ||
253 | /* | 251 | /* |
@@ -400,15 +398,8 @@ static char mon_bin_get_data(const struct mon_reader_bin *rp, | |||
400 | unsigned int offset, struct urb *urb, unsigned int length) | 398 | unsigned int offset, struct urb *urb, unsigned int length) |
401 | { | 399 | { |
402 | 400 | ||
403 | if (urb->dev->bus->uses_dma && | ||
404 | (urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) { | ||
405 | mon_dmapeek_vec(rp, offset, urb->transfer_dma, length); | ||
406 | return 0; | ||
407 | } | ||
408 | |||
409 | if (urb->transfer_buffer == NULL) | 401 | if (urb->transfer_buffer == NULL) |
410 | return 'Z'; | 402 | return 'Z'; |
411 | |||
412 | mon_copy_to_buff(rp, offset, urb->transfer_buffer, length); | 403 | mon_copy_to_buff(rp, offset, urb->transfer_buffer, length); |
413 | return 0; | 404 | return 0; |
414 | } | 405 | } |
@@ -635,7 +626,6 @@ static int mon_bin_open(struct inode *inode, struct file *file) | |||
635 | spin_lock_init(&rp->b_lock); | 626 | spin_lock_init(&rp->b_lock); |
636 | init_waitqueue_head(&rp->b_wait); | 627 | init_waitqueue_head(&rp->b_wait); |
637 | mutex_init(&rp->fetch_lock); | 628 | mutex_init(&rp->fetch_lock); |
638 | |||
639 | rp->b_size = BUFF_DFL; | 629 | rp->b_size = BUFF_DFL; |
640 | 630 | ||
641 | size = sizeof(struct mon_pgmap) * (rp->b_size/CHUNK_SIZE); | 631 | size = sizeof(struct mon_pgmap) * (rp->b_size/CHUNK_SIZE); |