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 | |
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')
-rw-r--r-- | drivers/usb/mon/Makefile | 2 | ||||
-rw-r--r-- | drivers/usb/mon/mon_bin.c | 12 | ||||
-rw-r--r-- | drivers/usb/mon/mon_dma.c | 95 | ||||
-rw-r--r-- | drivers/usb/mon/mon_main.c | 1 | ||||
-rw-r--r-- | drivers/usb/mon/mon_text.c | 14 | ||||
-rw-r--r-- | drivers/usb/mon/usb_mon.h | 14 |
6 files changed, 2 insertions, 136 deletions
diff --git a/drivers/usb/mon/Makefile b/drivers/usb/mon/Makefile index c6516b566731..384b198faa7c 100644 --- a/drivers/usb/mon/Makefile +++ b/drivers/usb/mon/Makefile | |||
@@ -2,6 +2,6 @@ | |||
2 | # Makefile for USB monitor | 2 | # Makefile for USB monitor |
3 | # | 3 | # |
4 | 4 | ||
5 | usbmon-objs := mon_main.o mon_stat.o mon_text.o mon_bin.o mon_dma.o | 5 | usbmon-objs := mon_main.o mon_stat.o mon_text.o mon_bin.o |
6 | 6 | ||
7 | obj-$(CONFIG_USB_MON) += usbmon.o | 7 | obj-$(CONFIG_USB_MON) += usbmon.o |
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); |
diff --git a/drivers/usb/mon/mon_dma.c b/drivers/usb/mon/mon_dma.c deleted file mode 100644 index 140cc80bd2b1..000000000000 --- a/drivers/usb/mon/mon_dma.c +++ /dev/null | |||
@@ -1,95 +0,0 @@ | |||
1 | /* | ||
2 | * The USB Monitor, inspired by Dave Harding's USBMon. | ||
3 | * | ||
4 | * mon_dma.c: Library which snoops on DMA areas. | ||
5 | * | ||
6 | * Copyright (C) 2005 Pete Zaitcev (zaitcev@redhat.com) | ||
7 | */ | ||
8 | #include <linux/kernel.h> | ||
9 | #include <linux/list.h> | ||
10 | #include <linux/highmem.h> | ||
11 | #include <asm/page.h> | ||
12 | |||
13 | #include <linux/usb.h> /* Only needed for declarations in usb_mon.h */ | ||
14 | #include "usb_mon.h" | ||
15 | |||
16 | /* | ||
17 | * PC-compatibles, are, fortunately, sufficiently cache-coherent for this. | ||
18 | */ | ||
19 | #if defined(__i386__) || defined(__x86_64__) /* CONFIG_ARCH_I386 doesn't exit */ | ||
20 | #define MON_HAS_UNMAP 1 | ||
21 | |||
22 | #define phys_to_page(phys) pfn_to_page((phys) >> PAGE_SHIFT) | ||
23 | |||
24 | char mon_dmapeek(unsigned char *dst, dma_addr_t dma_addr, int len) | ||
25 | { | ||
26 | struct page *pg; | ||
27 | unsigned long flags; | ||
28 | unsigned char *map; | ||
29 | unsigned char *ptr; | ||
30 | |||
31 | /* | ||
32 | * On i386, a DMA handle is the "physical" address of a page. | ||
33 | * In other words, the bus address is equal to physical address. | ||
34 | * There is no IOMMU. | ||
35 | */ | ||
36 | pg = phys_to_page(dma_addr); | ||
37 | |||
38 | /* | ||
39 | * We are called from hardware IRQs in case of callbacks. | ||
40 | * But we can be called from softirq or process context in case | ||
41 | * of submissions. In such case, we need to protect KM_IRQ0. | ||
42 | */ | ||
43 | local_irq_save(flags); | ||
44 | map = kmap_atomic(pg, KM_IRQ0); | ||
45 | ptr = map + (dma_addr & (PAGE_SIZE-1)); | ||
46 | memcpy(dst, ptr, len); | ||
47 | kunmap_atomic(map, KM_IRQ0); | ||
48 | local_irq_restore(flags); | ||
49 | return 0; | ||
50 | } | ||
51 | |||
52 | void mon_dmapeek_vec(const struct mon_reader_bin *rp, | ||
53 | unsigned int offset, dma_addr_t dma_addr, unsigned int length) | ||
54 | { | ||
55 | unsigned long flags; | ||
56 | unsigned int step_len; | ||
57 | struct page *pg; | ||
58 | unsigned char *map; | ||
59 | unsigned long page_off, page_len; | ||
60 | |||
61 | local_irq_save(flags); | ||
62 | while (length) { | ||
63 | /* compute number of bytes we are going to copy in this page */ | ||
64 | step_len = length; | ||
65 | page_off = dma_addr & (PAGE_SIZE-1); | ||
66 | page_len = PAGE_SIZE - page_off; | ||
67 | if (page_len < step_len) | ||
68 | step_len = page_len; | ||
69 | |||
70 | /* copy data and advance pointers */ | ||
71 | pg = phys_to_page(dma_addr); | ||
72 | map = kmap_atomic(pg, KM_IRQ0); | ||
73 | offset = mon_copy_to_buff(rp, offset, map + page_off, step_len); | ||
74 | kunmap_atomic(map, KM_IRQ0); | ||
75 | dma_addr += step_len; | ||
76 | length -= step_len; | ||
77 | } | ||
78 | local_irq_restore(flags); | ||
79 | } | ||
80 | |||
81 | #endif /* __i386__ */ | ||
82 | |||
83 | #ifndef MON_HAS_UNMAP | ||
84 | char mon_dmapeek(unsigned char *dst, dma_addr_t dma_addr, int len) | ||
85 | { | ||
86 | return 'D'; | ||
87 | } | ||
88 | |||
89 | void mon_dmapeek_vec(const struct mon_reader_bin *rp, | ||
90 | unsigned int offset, dma_addr_t dma_addr, unsigned int length) | ||
91 | { | ||
92 | ; | ||
93 | } | ||
94 | |||
95 | #endif /* MON_HAS_UNMAP */ | ||
diff --git a/drivers/usb/mon/mon_main.c b/drivers/usb/mon/mon_main.c index 5e0ab4201c00..e0c2db3b767b 100644 --- a/drivers/usb/mon/mon_main.c +++ b/drivers/usb/mon/mon_main.c | |||
@@ -361,7 +361,6 @@ static int __init mon_init(void) | |||
361 | } | 361 | } |
362 | // MOD_INC_USE_COUNT(which_module?); | 362 | // MOD_INC_USE_COUNT(which_module?); |
363 | 363 | ||
364 | |||
365 | mutex_lock(&usb_bus_list_lock); | 364 | mutex_lock(&usb_bus_list_lock); |
366 | list_for_each_entry (ubus, &usb_bus_list, bus_list) { | 365 | list_for_each_entry (ubus, &usb_bus_list, bus_list) { |
367 | mon_bus_init(ubus); | 366 | mon_bus_init(ubus); |
diff --git a/drivers/usb/mon/mon_text.c b/drivers/usb/mon/mon_text.c index a7eb4c99342c..9f1a9227ebe6 100644 --- a/drivers/usb/mon/mon_text.c +++ b/drivers/usb/mon/mon_text.c | |||
@@ -150,20 +150,6 @@ static inline char mon_text_get_data(struct mon_event_text *ep, struct urb *urb, | |||
150 | return '>'; | 150 | return '>'; |
151 | } | 151 | } |
152 | 152 | ||
153 | /* | ||
154 | * The check to see if it's safe to poke at data has an enormous | ||
155 | * number of corner cases, but it seems that the following is | ||
156 | * more or less safe. | ||
157 | * | ||
158 | * We do not even try to look at transfer_buffer, because it can | ||
159 | * contain non-NULL garbage in case the upper level promised to | ||
160 | * set DMA for the HCD. | ||
161 | */ | ||
162 | if (urb->dev->bus->uses_dma && | ||
163 | (urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) { | ||
164 | return mon_dmapeek(ep->data, urb->transfer_dma, len); | ||
165 | } | ||
166 | |||
167 | if (urb->transfer_buffer == NULL) | 153 | if (urb->transfer_buffer == NULL) |
168 | return 'Z'; /* '0' would be not as pretty. */ | 154 | return 'Z'; /* '0' would be not as pretty. */ |
169 | 155 | ||
diff --git a/drivers/usb/mon/usb_mon.h b/drivers/usb/mon/usb_mon.h index f5d84ff8c101..df9a4df342c7 100644 --- a/drivers/usb/mon/usb_mon.h +++ b/drivers/usb/mon/usb_mon.h | |||
@@ -65,20 +65,6 @@ int __init mon_bin_init(void); | |||
65 | void mon_bin_exit(void); | 65 | void mon_bin_exit(void); |
66 | 66 | ||
67 | /* | 67 | /* |
68 | * DMA interface. | ||
69 | * | ||
70 | * XXX The vectored side needs a serious re-thinking. Abstracting vectors, | ||
71 | * like in Paolo's original patch, produces a double pkmap. We need an idea. | ||
72 | */ | ||
73 | extern char mon_dmapeek(unsigned char *dst, dma_addr_t dma_addr, int len); | ||
74 | |||
75 | struct mon_reader_bin; | ||
76 | extern void mon_dmapeek_vec(const struct mon_reader_bin *rp, | ||
77 | unsigned int offset, dma_addr_t dma_addr, unsigned int len); | ||
78 | extern unsigned int mon_copy_to_buff(const struct mon_reader_bin *rp, | ||
79 | unsigned int offset, const unsigned char *from, unsigned int len); | ||
80 | |||
81 | /* | ||
82 | */ | 68 | */ |
83 | extern struct mutex mon_lock; | 69 | extern struct mutex mon_lock; |
84 | 70 | ||