[v4,08/29] dma-api: Teach the "DMA-from-stack" check about vmapped stacks
diff mbox

Message ID fbaa1409c74ddc7b29b52c87303d2af1db91e2ba.1466974736.git.luto@kernel.org
State New
Headers show

Commit Message

Andy Lutomirski June 26, 2016, 9:55 p.m. UTC
If we're using CONFIG_VMAP_STACK and we manage to point an sg entry
at the stack, then either the sg page will be in highmem or sg_virt
will return the direct-map alias.  In neither case will the existing
check_for_stack() implementation realize that it's a stack page.

Fix it by explicitly checking for stack pages.

This has no effect by itself.  It's broken out for ease of review.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 lib/dma-debug.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

Comments

Borislav Petkov June 30, 2016, 7:37 p.m. UTC | #1
On Sun, Jun 26, 2016 at 02:55:30PM -0700, Andy Lutomirski wrote:
> If we're using CONFIG_VMAP_STACK and we manage to point an sg entry
> at the stack, then either the sg page will be in highmem or sg_virt
> will return the direct-map alias.  In neither case will the existing
> check_for_stack() implementation realize that it's a stack page.
> 
> Fix it by explicitly checking for stack pages.
> 
> This has no effect by itself.  It's broken out for ease of review.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  lib/dma-debug.c | 39 +++++++++++++++++++++++++++++++++------
>  1 file changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/dma-debug.c b/lib/dma-debug.c
> index 51a76af25c66..5b2e63cba90e 100644
> --- a/lib/dma-debug.c
> +++ b/lib/dma-debug.c
> @@ -22,6 +22,7 @@
>  #include <linux/stacktrace.h>
>  #include <linux/dma-debug.h>
>  #include <linux/spinlock.h>
> +#include <linux/vmalloc.h>
>  #include <linux/debugfs.h>
>  #include <linux/uaccess.h>
>  #include <linux/export.h>
> @@ -1162,11 +1163,35 @@ static void check_unmap(struct dma_debug_entry *ref)
>  	put_hash_bucket(bucket, &flags);
>  }
>  
> -static void check_for_stack(struct device *dev, void *addr)
> +static void check_for_stack(struct device *dev,
> +			    struct page *page, size_t offset)
>  {
> -	if (object_is_on_stack(addr))
> -		err_printk(dev, NULL, "DMA-API: device driver maps memory from "
> -				"stack [addr=%p]\n", addr);
> +	void *addr;
> +	struct vm_struct *stack_vm_area = task_stack_vm_area(current);

lib/dma-debug.c: In function ‘check_for_stack’:
lib/dma-debug.c:1170:36: error: implicit declaration of function ‘task_stack_vm_area’ [-Werror=implicit-function-declaration]
  struct vm_struct *stack_vm_area = task_stack_vm_area(current);
                                    ^
lib/dma-debug.c:1170:36: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
cc1: some warnings being treated as errors
make[1]: *** [lib/dma-debug.o] Error 1
make: *** [lib] Error 2
make: *** Waiting for unfinished jobs....

Probably reorder pieces from patch 9 to earlier ones...
Andy Lutomirski July 6, 2016, 1:20 p.m. UTC | #2
On Thu, Jun 30, 2016 at 12:37 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Sun, Jun 26, 2016 at 02:55:30PM -0700, Andy Lutomirski wrote:
>> If we're using CONFIG_VMAP_STACK and we manage to point an sg entry
>> at the stack, then either the sg page will be in highmem or sg_virt
>> will return the direct-map alias.  In neither case will the existing
>> check_for_stack() implementation realize that it's a stack page.
>>
>> Fix it by explicitly checking for stack pages.
>>
>> This has no effect by itself.  It's broken out for ease of review.
>>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>  lib/dma-debug.c | 39 +++++++++++++++++++++++++++++++++------
>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/dma-debug.c b/lib/dma-debug.c
>> index 51a76af25c66..5b2e63cba90e 100644
>> --- a/lib/dma-debug.c
>> +++ b/lib/dma-debug.c
>> @@ -22,6 +22,7 @@
>>  #include <linux/stacktrace.h>
>>  #include <linux/dma-debug.h>
>>  #include <linux/spinlock.h>
>> +#include <linux/vmalloc.h>
>>  #include <linux/debugfs.h>
>>  #include <linux/uaccess.h>
>>  #include <linux/export.h>
>> @@ -1162,11 +1163,35 @@ static void check_unmap(struct dma_debug_entry *ref)
>>       put_hash_bucket(bucket, &flags);
>>  }
>>
>> -static void check_for_stack(struct device *dev, void *addr)
>> +static void check_for_stack(struct device *dev,
>> +                         struct page *page, size_t offset)
>>  {
>> -     if (object_is_on_stack(addr))
>> -             err_printk(dev, NULL, "DMA-API: device driver maps memory from "
>> -                             "stack [addr=%p]\n", addr);
>> +     void *addr;
>> +     struct vm_struct *stack_vm_area = task_stack_vm_area(current);
>
> lib/dma-debug.c: In function ‘check_for_stack’:
> lib/dma-debug.c:1170:36: error: implicit declaration of function ‘task_stack_vm_area’ [-Werror=implicit-function-declaration]
>   struct vm_struct *stack_vm_area = task_stack_vm_area(current);
>                                     ^
> lib/dma-debug.c:1170:36: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
> cc1: some warnings being treated as errors
> make[1]: *** [lib/dma-debug.o] Error 1
> make: *** [lib] Error 2
> make: *** Waiting for unfinished jobs....
>
> Probably reorder pieces from patch 9 to earlier ones...

I'll address this by reordering it later in the series.  The temporary
loss of functionality will be unobservable.

--Andy

Patch
diff mbox

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 51a76af25c66..5b2e63cba90e 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -22,6 +22,7 @@ 
 #include <linux/stacktrace.h>
 #include <linux/dma-debug.h>
 #include <linux/spinlock.h>
+#include <linux/vmalloc.h>
 #include <linux/debugfs.h>
 #include <linux/uaccess.h>
 #include <linux/export.h>
@@ -1162,11 +1163,35 @@  static void check_unmap(struct dma_debug_entry *ref)
 	put_hash_bucket(bucket, &flags);
 }
 
-static void check_for_stack(struct device *dev, void *addr)
+static void check_for_stack(struct device *dev,
+			    struct page *page, size_t offset)
 {
-	if (object_is_on_stack(addr))
-		err_printk(dev, NULL, "DMA-API: device driver maps memory from "
-				"stack [addr=%p]\n", addr);
+	void *addr;
+	struct vm_struct *stack_vm_area = task_stack_vm_area(current);
+
+	if (!stack_vm_area) {
+		/* Stack is direct-mapped. */
+		if (PageHighMem(page))
+			return;
+		addr = page_address(page) + offset;
+		if (object_is_on_stack(addr))
+			err_printk(dev, NULL, "DMA-API: device driver maps memory from stack [addr=%p]\n",
+				   addr);
+	} else {
+		/* Stack is vmalloced. */
+		int i;
+
+		for (i = 0; i < stack_vm_area->nr_pages; i++) {
+			if (page != stack_vm_area->pages[i])
+				continue;
+
+			addr = (u8 *)current->stack + i * PAGE_SIZE +
+				offset;
+			err_printk(dev, NULL, "DMA-API: device driver maps memory from stack [probable addr=%p]\n",
+				   addr);
+			break;
+		}
+	}
 }
 
 static inline bool overlap(void *addr, unsigned long len, void *start, void *end)
@@ -1289,10 +1314,11 @@  void debug_dma_map_page(struct device *dev, struct page *page, size_t offset,
 	if (map_single)
 		entry->type = dma_debug_single;
 
+	check_for_stack(dev, page, offset);
+
 	if (!PageHighMem(page)) {
 		void *addr = page_address(page) + offset;
 
-		check_for_stack(dev, addr);
 		check_for_illegal_area(dev, addr, size);
 	}
 
@@ -1384,8 +1410,9 @@  void debug_dma_map_sg(struct device *dev, struct scatterlist *sg,
 		entry->sg_call_ents   = nents;
 		entry->sg_mapped_ents = mapped_ents;
 
+		check_for_stack(dev, sg_page(s), s->offset);
+
 		if (!PageHighMem(sg_page(s))) {
-			check_for_stack(dev, sg_virt(s));
 			check_for_illegal_area(dev, sg_virt(s), sg_dma_len(s));
 		}