linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] staging: android: ion: Zero CMA allocated memory
@ 2018-01-26 17:48 Liam Mark
  2018-01-27  2:04 ` [Linaro-mm-sig] " Chen Feng
  2018-02-06 22:49 ` Laura Abbott
  0 siblings, 2 replies; 4+ messages in thread
From: Liam Mark @ 2018-01-26 17:48 UTC (permalink / raw)
  To: Laura Abbott, Sumit Semwal
  Cc: Dan Carpenter, Greg KH, linux-kernel, devel, linaro-mm-sig

Since commit 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
the CMA API is now used directly and therefore the allocated memory is no
longer automatically zeroed.

Explicitly zero CMA allocated memory to ensure that no data is exposed to
userspace.

Fixes: 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
Signed-off-by: Liam Mark <lmark@codeaurora.org>
Acked-by: Laura Abbott <labbott@redhat.com>
---
Changes in v2:
  - Clean up the commit message.
  - Add 'Fixes:'

Changes in v3:
 - Add support for highmem pages

 drivers/staging/android/ion/ion_cma_heap.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c
index 86196ffd2faf..fa3e4b7e0c9f 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -21,6 +21,7 @@
 #include <linux/err.h>
 #include <linux/cma.h>
 #include <linux/scatterlist.h>
+#include <linux/highmem.h>
 
 #include "ion.h"
 
@@ -51,6 +52,22 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
 	if (!pages)
 		return -ENOMEM;
 
+	if (PageHighMem(pages)) {
+		unsigned long nr_clear_pages = nr_pages;
+		struct page *page = pages;
+
+		while (nr_clear_pages > 0) {
+			void *vaddr = kmap_atomic(page);
+
+			memset(vaddr, 0, PAGE_SIZE);
+			kunmap_atomic(vaddr);
+			page++;
+			nr_clear_pages--;
+		}
+	} else {
+		memset(page_address(pages), 0, size);
+	}
+
 	table = kmalloc(sizeof(*table), GFP_KERNEL);
 	if (!table)
 		goto err;
-- 
1.8.5.2


Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [Linaro-mm-sig] [PATCH v3] staging: android: ion: Zero CMA allocated memory
  2018-01-26 17:48 [PATCH v3] staging: android: ion: Zero CMA allocated memory Liam Mark
@ 2018-01-27  2:04 ` Chen Feng
  2018-01-27  6:52   ` Laura Abbott
  2018-02-06 22:49 ` Laura Abbott
  1 sibling, 1 reply; 4+ messages in thread
From: Chen Feng @ 2018-01-27  2:04 UTC (permalink / raw)
  To: Liam Mark, Laura Abbott, Sumit Semwal
  Cc: devel, Greg KH, linaro-mm-sig, linux-kernel, Dan Carpenter,
	Xiaqing (A),
	Zhuangluan Su



On 2018/1/27 1:48, Liam Mark wrote:
> Since commit 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
> the CMA API is now used directly and therefore the allocated memory is no
> longer automatically zeroed.
> 
> Explicitly zero CMA allocated memory to ensure that no data is exposed to
> userspace.
> 
> Fixes: 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
> Signed-off-by: Liam Mark <lmark@codeaurora.org>
> ---
> Changes in v2:
>   - Clean up the commit message.
>   - Add 'Fixes:'
> 
> Changes in v3:
>  - Add support for highmem pages
> 
>  drivers/staging/android/ion/ion_cma_heap.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c
> index 86196ffd2faf..fa3e4b7e0c9f 100644
> --- a/drivers/staging/android/ion/ion_cma_heap.c
> +++ b/drivers/staging/android/ion/ion_cma_heap.c
> @@ -21,6 +21,7 @@
>  #include <linux/err.h>
>  #include <linux/cma.h>
>  #include <linux/scatterlist.h>
> +#include <linux/highmem.h>
>  
>  #include "ion.h"
>  
> @@ -51,6 +52,22 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
>  	if (!pages)
>  		return -ENOMEM;
>  
> +	if (PageHighMem(pages)) {
> +		unsigned long nr_clear_pages = nr_pages;
> +		struct page *page = pages;
> +
> +		while (nr_clear_pages > 0) {
> +			void *vaddr = kmap_atomic(page);
> +
> +			memset(vaddr, 0, PAGE_SIZE);
> +			kunmap_atomic(vaddr);

Here. This way may cause performance latency at mapping-memset-umap page one bye one.

Take a look at ion_heap_pages_zero.

Not very critical, arm64 always have linear mapping.


> +			page++;
> +			nr_clear_pages--;
> +		}
> +	} else {
> +		memset(page_address(pages), 0, size);
> +	}
> +
>  	table = kmalloc(sizeof(*table), GFP_KERNEL);
>  	if (!table)
>  		goto err;
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [Linaro-mm-sig] [PATCH v3] staging: android: ion: Zero CMA allocated memory
  2018-01-27  2:04 ` [Linaro-mm-sig] " Chen Feng
@ 2018-01-27  6:52   ` Laura Abbott
  0 siblings, 0 replies; 4+ messages in thread
From: Laura Abbott @ 2018-01-27  6:52 UTC (permalink / raw)
  To: Chen Feng, Liam Mark, Sumit Semwal
  Cc: devel, Greg KH, Zhuangluan Su, linux-kernel, linaro-mm-sig,
	Xiaqing (A),
	Dan Carpenter

On 01/26/2018 06:04 PM, Chen Feng wrote:
> 
> 
> On 2018/1/27 1:48, Liam Mark wrote:
>> Since commit 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
>> the CMA API is now used directly and therefore the allocated memory is no
>> longer automatically zeroed.
>>
>> Explicitly zero CMA allocated memory to ensure that no data is exposed to
>> userspace.
>>
>> Fixes: 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
>> Signed-off-by: Liam Mark <lmark@codeaurora.org>
>> ---
>> Changes in v2:
>>    - Clean up the commit message.
>>    - Add 'Fixes:'
>>
>> Changes in v3:
>>   - Add support for highmem pages
>>
>>   drivers/staging/android/ion/ion_cma_heap.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c
>> index 86196ffd2faf..fa3e4b7e0c9f 100644
>> --- a/drivers/staging/android/ion/ion_cma_heap.c
>> +++ b/drivers/staging/android/ion/ion_cma_heap.c
>> @@ -21,6 +21,7 @@
>>   #include <linux/err.h>
>>   #include <linux/cma.h>
>>   #include <linux/scatterlist.h>
>> +#include <linux/highmem.h>
>>   
>>   #include "ion.h"
>>   
>> @@ -51,6 +52,22 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
>>   	if (!pages)
>>   		return -ENOMEM;
>>   
>> +	if (PageHighMem(pages)) {
>> +		unsigned long nr_clear_pages = nr_pages;
>> +		struct page *page = pages;
>> +
>> +		while (nr_clear_pages > 0) {
>> +			void *vaddr = kmap_atomic(page);
>> +
>> +			memset(vaddr, 0, PAGE_SIZE);
>> +			kunmap_atomic(vaddr);
> 
> Here. This way may cause performance latency at mapping-memset-umap page one bye one.
> 
> Take a look at ion_heap_pages_zero.
> 
> Not very critical, arm64 always have linear mapping.
> 

This is under a PageHighMem check so arm64 isn't affected. It's also
the same algorithm arm32 dma-mapping.c uses so I'd like to see some
data about the performance improvement before we go changing things
too much.

Thanks,
Laura

> 
>> +			page++;
>> +			nr_clear_pages--;
>> +		}
>> +	} else {
>> +		memset(page_address(pages), 0, size);
>> +	}
>> +
>>   	table = kmalloc(sizeof(*table), GFP_KERNEL);
>>   	if (!table)
>>   		goto err;
>>
> 

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v3] staging: android: ion: Zero CMA allocated memory
  2018-01-26 17:48 [PATCH v3] staging: android: ion: Zero CMA allocated memory Liam Mark
  2018-01-27  2:04 ` [Linaro-mm-sig] " Chen Feng
@ 2018-02-06 22:49 ` Laura Abbott
  1 sibling, 0 replies; 4+ messages in thread
From: Laura Abbott @ 2018-02-06 22:49 UTC (permalink / raw)
  To: Liam Mark, Sumit Semwal
  Cc: Dan Carpenter, Greg KH, linux-kernel, devel, linaro-mm-sig

On 01/26/2018 09:48 AM, Liam Mark wrote:
> Since commit 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
> the CMA API is now used directly and therefore the allocated memory is no
> longer automatically zeroed.
> 
> Explicitly zero CMA allocated memory to ensure that no data is exposed to
> userspace.
> 

Acked-by: Laura Abbott <labbott@redhat.com>

> Fixes: 204f672255c2 ("staging: android: ion: Use CMA APIs directly")
> Signed-off-by: Liam Mark <lmark@codeaurora.org>
> ---
> Changes in v2:
>    - Clean up the commit message.
>    - Add 'Fixes:'
> 
> Changes in v3:
>   - Add support for highmem pages
> 
>   drivers/staging/android/ion/ion_cma_heap.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c
> index 86196ffd2faf..fa3e4b7e0c9f 100644
> --- a/drivers/staging/android/ion/ion_cma_heap.c
> +++ b/drivers/staging/android/ion/ion_cma_heap.c
> @@ -21,6 +21,7 @@
>   #include <linux/err.h>
>   #include <linux/cma.h>
>   #include <linux/scatterlist.h>
> +#include <linux/highmem.h>
>   
>   #include "ion.h"
>   
> @@ -51,6 +52,22 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
>   	if (!pages)
>   		return -ENOMEM;
>   
> +	if (PageHighMem(pages)) {
> +		unsigned long nr_clear_pages = nr_pages;
> +		struct page *page = pages;
> +
> +		while (nr_clear_pages > 0) {
> +			void *vaddr = kmap_atomic(page);
> +
> +			memset(vaddr, 0, PAGE_SIZE);
> +			kunmap_atomic(vaddr);
> +			page++;
> +			nr_clear_pages--;
> +		}
> +	} else {
> +		memset(page_address(pages), 0, size);
> +	}
> +
>   	table = kmalloc(sizeof(*table), GFP_KERNEL);
>   	if (!table)
>   		goto err;
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-06 22:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-26 17:48 [PATCH v3] staging: android: ion: Zero CMA allocated memory Liam Mark
2018-01-27  2:04 ` [Linaro-mm-sig] " Chen Feng
2018-01-27  6:52   ` Laura Abbott
2018-02-06 22:49 ` Laura Abbott

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).