aboutsummaryrefslogtreecommitdiffstats
path: root/lib/dma-debug.c
diff options
context:
space:
mode:
authorIngo Molnar <mingo@elte.hu>2009-07-10 15:38:02 -0400
committerIngo Molnar <mingo@elte.hu>2009-07-10 16:18:45 -0400
commitf39d1b9792881ce4eb982ec8cc65258bf95674b5 (patch)
tree07f945a2a6f0b1f2fb2af759e386781d0e0bed3f /lib/dma-debug.c
parent2b8777ca0c944bf6498c45ed9c5c246bd63a719e (diff)
dma-debug: Fix the overlap() function to be correct and readable
Linus noticed how unclean and buggy the overlap() function is: - It uses convoluted (and bug-causing) positive checks for range overlap - instead of using a more natural negative check. - Even the positive checks are buggy: a positive intersection check has four natural cases while we checked only for three, missing the (addr < start && addr2 == end) case for example. - The variables are mis-named, making it non-obvious how the check was done. - It needlessly uses u64 instead of unsigned long. Since these are kernel memory pointers and we explicitly exclude highmem ranges anyway we cannot ever overflow 32 bits, even if we could. (and on 64-bit it doesnt matter anyway) All in one, this function needs a total revamp. I used Linus's suggestions minus the paranoid checks (we cannot overflow really because if we get totally bad DMA ranges passed far more things break in the systems than just DMA debugging). I also fixed a few other small details i noticed. Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Cc: Joerg Roedel <joerg.roedel@amd.com> Signed-off-by: Ingo Molnar <mingo@elte.hu>
Diffstat (limited to 'lib/dma-debug.c')
-rw-r--r--lib/dma-debug.c24
1 files changed, 12 insertions, 12 deletions
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index c9187fed0b93..65b0d99b6d0a 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -856,22 +856,21 @@ static void check_for_stack(struct device *dev, void *addr)
856 "stack [addr=%p]\n", addr); 856 "stack [addr=%p]\n", addr);
857} 857}
858 858
859static inline bool overlap(void *addr, u64 size, void *start, void *end) 859static inline bool overlap(void *addr, unsigned long len, void *start, void *end)
860{ 860{
861 void *addr2 = (char *)addr + size; 861 unsigned long a1 = (unsigned long)addr;
862 unsigned long b1 = a1 + len;
863 unsigned long a2 = (unsigned long)start;
864 unsigned long b2 = (unsigned long)end;
862 865
863 return ((addr >= start && addr < end) || 866 return !(b1 <= a2 || a1 >= b2);
864 (addr2 >= start && addr2 < end) ||
865 ((addr < start) && (addr2 > end)));
866} 867}
867 868
868static void check_for_illegal_area(struct device *dev, void *addr, u64 size) 869static void check_for_illegal_area(struct device *dev, void *addr, unsigned long len)
869{ 870{
870 if (overlap(addr, size, _text, _etext) || 871 if (overlap(addr, len, _text, _etext) ||
871 overlap(addr, size, __start_rodata, __end_rodata)) 872 overlap(addr, len, __start_rodata, __end_rodata))
872 err_printk(dev, NULL, "DMA-API: device driver maps " 873 err_printk(dev, NULL, "DMA-API: device driver maps memory from kernel text or rodata [addr=%p] [len=%lu]\n", addr, len);
873 "memory from kernel text or rodata "
874 "[addr=%p] [size=%llu]\n", addr, size);
875} 874}
876 875
877static void check_sync(struct device *dev, 876static void check_sync(struct device *dev,
@@ -969,7 +968,8 @@ void debug_dma_map_page(struct device *dev, struct page *page, size_t offset,
969 entry->type = dma_debug_single; 968 entry->type = dma_debug_single;
970 969
971 if (!PageHighMem(page)) { 970 if (!PageHighMem(page)) {
972 void *addr = ((char *)page_address(page)) + offset; 971 void *addr = page_address(page) + offset;
972
973 check_for_stack(dev, addr); 973 check_for_stack(dev, addr);
974 check_for_illegal_area(dev, addr, size); 974 check_for_illegal_area(dev, addr, size);
975 } 975 }