linux-kernel - Re: [PATCH 0/20] dma

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080326124929.GA12602@elte.hu>
Date:	Wed, 26 Mar 2008 13:49:29 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Glauber Costa <gcosta@...hat.com>
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	glommer@...il.com, tglx@...utronix.de,
	kvm-devel@...ts.sourceforge.net, avi@...ranet.com,
	amit.shah@...ranet.com
Subject: Re: [PATCH 0/20] dma_ops for i386


* Ingo Molnar <mingo@...e.hu> wrote:

> > The motivation for that is the ongoing work for pci-passthrough in 
> > KVM. So ingo, avi, what do you think it's the best way to handle 
> > these patches through?
> 
> looks very nice to me! I've applied it to x86.git, lets see what 
> happens.

Houston, we've got a problem! :-/

randconfig testing found that this patchset broke sendfile on certain 
.config's - DMA started returning all 0xfffffffff data, corrupting 
files. (config attached)

After some bisection fun it turns out that the conversions from struct 
page are wrong:

+static inline dma_addr_t dma_map_page(struct device *dev, struct page *page,
+                                     size_t offset, size_t size,
+                                     int direction)
+{
+       return dma_map_single(dev, page_address(page)+offset, size, direction);

because page_address() is not defined for highmem pages in general. (and 
even if it's defined, it will corrupt data)

changing it to page_to_phys() is not good because it goes via a 32-bit 
bottleneck that trims things on PAE:

       dma_addr_t      (*map_single)(struct device *hwdev, void *ptr,

the 'ptr' is 32-bit albeit it's a DMA target.

what i came up is the prototype 32-bit fix below - this works on 32-bit 
but breaks 64-bit because we pass in physical addresses instead of 
virtual direct addresses.

i'll fix the 64-bit side but that means materially touching all the 
dma_mapping_ops instantiations materially on the 64-bit side - not 
really something we wanted to do :-/

	Ingo

---------------->
Subject: x86: dma-ops on highmem fix
From: Ingo Molnar <mingo@...e.hu>

Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
 arch/x86/kernel/pci-base_32.c |    4 ++--
 include/asm-x86/dma-mapping.h |   10 ++++++----
 2 files changed, 8 insertions(+), 6 deletions(-)

Index: linux-x86.q/arch/x86/kernel/pci-base_32.c
===================================================================
--- linux-x86.q.orig/arch/x86/kernel/pci-base_32.c
+++ linux-x86.q/arch/x86/kernel/pci-base_32.c
@@ -4,12 +4,12 @@
 #include <linux/dma-mapping.h>
 #include <asm/dma-mapping.h>
 
-static dma_addr_t pci32_map_single(struct device *dev, void *ptr,
+static dma_addr_t pci32_map_single(struct device *dev, u64 ptr,
 				   size_t size, int direction)
 {
 	WARN_ON(size == 0);
 	flush_write_buffers();
-	return virt_to_phys(ptr);
+	return ptr;
 }
 
 static int pci32_dma_map_sg(struct device *dev, struct scatterlist *sglist,
Index: linux-x86.q/include/asm-x86/dma-mapping.h
===================================================================
--- linux-x86.q.orig/include/asm-x86/dma-mapping.h
+++ linux-x86.q/include/asm-x86/dma-mapping.h
@@ -16,10 +16,10 @@ struct dma_mapping_ops {
 				dma_addr_t *dma_handle, gfp_t gfp);
 	void            (*free_coherent)(struct device *dev, size_t size,
 				void *vaddr, dma_addr_t dma_handle);
-	dma_addr_t      (*map_single)(struct device *hwdev, void *ptr,
+	dma_addr_t      (*map_single)(struct device *hwdev, u64 ptr,
 				size_t size, int direction);
 	/* like map_single, but doesn't check the device mask */
-	dma_addr_t      (*map_simple)(struct device *hwdev, char *ptr,
+	dma_addr_t      (*map_simple)(struct device *hwdev, u64 ptr,
 				size_t size, int direction);
 	void            (*unmap_single)(struct device *dev, dma_addr_t addr,
 				size_t size, int direction);
@@ -73,7 +73,7 @@ dma_map_single(struct device *hwdev, voi
 	       int direction)
 {
 	BUG_ON(!valid_dma_direction(direction));
-	return dma_ops->map_single(hwdev, ptr, size, direction);
+	return dma_ops->map_single(hwdev, virt_to_phys(ptr), size, direction);
 }
 
 static inline void
@@ -174,7 +174,9 @@ static inline dma_addr_t dma_map_page(st
 				      size_t offset, size_t size,
 				      int direction)
 {
-	return dma_map_single(dev, page_address(page)+offset, size, direction);
+	BUG_ON(!valid_dma_direction(direction));
+	return dma_ops->map_single(dev, page_to_phys(page)+offset,
+				   size, direction);
 }
 
 static inline void dma_unmap_page(struct device *dev, dma_addr_t addr,

View attachment "config" of type "text/plain" (45676 bytes)