[PATCH] dma-buf: dma-heap: Add a size limitation for allocation

[PATCH] dma-buf: dma-heap: Add a size limitation for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Currently, there is no size check for allocation.

If the alloc size is larger than DRAM, it will cause OOM issue.
Besides, if it runs on a process that won't be killed by OOM flow, it will
cause a kernel exception finally,  and we couldn't find the correct
memory usage by dma-buf dump api such as "dma_buf_debug_show" since the
allocation is still on going and the corresponding dmabuf is not exported.

However, it sounds not simple enough to adding a count to count how many
pages has been allocated before allocating done.
So adding a size limitation here to prevent this case.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
---
 drivers/dma-buf/dma-heap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 56bf5ad01ad5..8b75998a106c 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -107,6 +107,9 @@ static long dma_heap_ioctl_allocate(struct file *file, void *data)
 	if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
 		return -EINVAL;
 
+	if (heap_allocation->len / PAGE_SIZE > totalram_pages() / 2)
+		return -EINVAL;
+
 	fd = dma_heap_buffer_alloc(heap, heap_allocation->len,
 				   heap_allocation->fd_flags,
 				   heap_allocation->heap_flags);
-- 
2.17.1

[PATCH] dma-buf: dma-heap: Add a size limitation for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Currently, there is no size check for allocation.

If the alloc size is larger than DRAM, it will cause OOM issue.
Besides, if it runs on a process that won't be killed by OOM flow, it will
cause a kernel exception finally,  and we couldn't find the correct
memory usage by dma-buf dump api such as "dma_buf_debug_show" since the
allocation is still on going and the corresponding dmabuf is not exported.

However, it sounds not simple enough to adding a count to count how many
pages has been allocated before allocating done.
So adding a size limitation here to prevent this case.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
---
 drivers/dma-buf/dma-heap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 56bf5ad01ad5..8b75998a106c 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -107,6 +107,9 @@ static long dma_heap_ioctl_allocate(struct file *file, void *data)
 	if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
 		return -EINVAL;
 
+	if (heap_allocation->len / PAGE_SIZE > totalram_pages() / 2)
+		return -EINVAL;
+
 	fd = dma_heap_buffer_alloc(heap, heap_allocation->len,
 				   heap_allocation->fd_flags,
 				   heap_allocation->heap_flags);
-- 
2.17.1

[PATCH] dma-buf: dma-heap: Add a size limitation for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Currently, there is no size check for allocation.

If the alloc size is larger than DRAM, it will cause OOM issue.
Besides, if it runs on a process that won't be killed by OOM flow, it will
cause a kernel exception finally,  and we couldn't find the correct
memory usage by dma-buf dump api such as "dma_buf_debug_show" since the
allocation is still on going and the corresponding dmabuf is not exported.

However, it sounds not simple enough to adding a count to count how many
pages has been allocated before allocating done.
So adding a size limitation here to prevent this case.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
---
 drivers/dma-buf/dma-heap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 56bf5ad01ad5..8b75998a106c 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -107,6 +107,9 @@ static long dma_heap_ioctl_allocate(struct file *file, void *data)
 	if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
 		return -EINVAL;
 
+	if (heap_allocation->len / PAGE_SIZE > totalram_pages() / 2)
+		return -EINVAL;
+
 	fd = dma_heap_buffer_alloc(heap, heap_allocation->len,
 				   heap_allocation->fd_flags,
 				   heap_allocation->heap_flags);
-- 
2.17.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH] dma-buf: dma-heap: Add a size limitation for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Currently, there is no size check for allocation.

If the alloc size is larger than DRAM, it will cause OOM issue.
Besides, if it runs on a process that won't be killed by OOM flow, it will
cause a kernel exception finally,  and we couldn't find the correct
memory usage by dma-buf dump api such as "dma_buf_debug_show" since the
allocation is still on going and the corresponding dmabuf is not exported.

However, it sounds not simple enough to adding a count to count how many
pages has been allocated before allocating done.
So adding a size limitation here to prevent this case.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
---
 drivers/dma-buf/dma-heap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 56bf5ad01ad5..8b75998a106c 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -107,6 +107,9 @@ static long dma_heap_ioctl_allocate(struct file *file, void *data)
 	if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
 		return -EINVAL;
 
+	if (heap_allocation->len / PAGE_SIZE > totalram_pages() / 2)
+		return -EINVAL;
+
 	fd = dma_heap_buffer_alloc(heap, heap_allocation->len,
 				   heap_allocation->fd_flags,
 				   heap_allocation->heap_flags);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allcation since the allocation size is
always less than the total DRAM size.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
---
v2: 1. update size limitation as total_dram page size.
    2. update commit message
---
 drivers/dma-buf/dma-heap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 56bf5ad01ad5..e39d2be98d69 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
 	struct dma_buf *dmabuf;
 	int fd;
 
+	if (len / PAGE_SIZE > totalram_pages())
+		return -EINVAL;
 	/*
 	 * Allocations from all heaps have to begin
 	 * and end on page boundaries.
-- 
2.17.1

[PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allcation since the allocation size is
always less than the total DRAM size.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
---
v2: 1. update size limitation as total_dram page size.
    2. update commit message
---
 drivers/dma-buf/dma-heap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 56bf5ad01ad5..e39d2be98d69 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
 	struct dma_buf *dmabuf;
 	int fd;
 
+	if (len / PAGE_SIZE > totalram_pages())
+		return -EINVAL;
 	/*
 	 * Allocations from all heaps have to begin
 	 * and end on page boundaries.
-- 
2.17.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allcation since the allocation size is
always less than the total DRAM size.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
---
v2: 1. update size limitation as total_dram page size.
    2. update commit message
---
 drivers/dma-buf/dma-heap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 56bf5ad01ad5..e39d2be98d69 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
 	struct dma_buf *dmabuf;
 	int fd;
 
+	if (len / PAGE_SIZE > totalram_pages())
+		return -EINVAL;
 	/*
 	 * Allocations from all heaps have to begin
 	 * and end on page boundaries.
-- 
2.17.1

[PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allcation since the allocation size is
always less than the total DRAM size.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
---
v2: 1. update size limitation as total_dram page size.
    2. update commit message
---
 drivers/dma-buf/dma-heap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 56bf5ad01ad5..e39d2be98d69 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
 	struct dma_buf *dmabuf;
 	int fd;
 
+	if (len / PAGE_SIZE > totalram_pages())
+		return -EINVAL;
 	/*
 	 * Allocations from all heaps have to begin
 	 * and end on page boundaries.
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
>
> From: Guangming <Guangming.Cao@mediatek.com>
>

Thanks for submitting this!

> Add a size check for allcation since the allocation size is

nit: "allocation" above.

> always less than the total DRAM size.

In general, it might be good to add more context to the commit message
to better answer *why* this change is needed rather than what the
change is doing.  ie: What negative thing happens without this change?
And so how does this change avoid or improve things?


> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> ---
> v2: 1. update size limitation as total_dram page size.
>     2. update commit message
> ---
>  drivers/dma-buf/dma-heap.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 56bf5ad01ad5..e39d2be98d69 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>         struct dma_buf *dmabuf;
>         int fd;
>
> +       if (len / PAGE_SIZE > totalram_pages())
> +               return -EINVAL;

This seems sane. I know ION used to have some 1/2 of memory cap to
avoid unnecessary memory pressure on crazy allocations.

Could you send again with an improved commit message?

thanks
-john

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
>
> From: Guangming <Guangming.Cao@mediatek.com>
>

Thanks for submitting this!

> Add a size check for allcation since the allocation size is

nit: "allocation" above.

> always less than the total DRAM size.

In general, it might be good to add more context to the commit message
to better answer *why* this change is needed rather than what the
change is doing.  ie: What negative thing happens without this change?
And so how does this change avoid or improve things?


> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> ---
> v2: 1. update size limitation as total_dram page size.
>     2. update commit message
> ---
>  drivers/dma-buf/dma-heap.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 56bf5ad01ad5..e39d2be98d69 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>         struct dma_buf *dmabuf;
>         int fd;
>
> +       if (len / PAGE_SIZE > totalram_pages())
> +               return -EINVAL;

This seems sane. I know ION used to have some 1/2 of memory cap to
avoid unnecessary memory pressure on crazy allocations.

Could you send again with an improved commit message?

thanks
-john

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
>
> From: Guangming <Guangming.Cao@mediatek.com>
>

Thanks for submitting this!

> Add a size check for allcation since the allocation size is

nit: "allocation" above.

> always less than the total DRAM size.

In general, it might be good to add more context to the commit message
to better answer *why* this change is needed rather than what the
change is doing.  ie: What negative thing happens without this change?
And so how does this change avoid or improve things?


> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> ---
> v2: 1. update size limitation as total_dram page size.
>     2. update commit message
> ---
>  drivers/dma-buf/dma-heap.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 56bf5ad01ad5..e39d2be98d69 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>         struct dma_buf *dmabuf;
>         int fd;
>
> +       if (len / PAGE_SIZE > totalram_pages())
> +               return -EINVAL;

This seems sane. I know ION used to have some 1/2 of memory cap to
avoid unnecessary memory pressure on crazy allocations.

Could you send again with an improved commit message?

thanks
-john

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
>
> From: Guangming <Guangming.Cao@mediatek.com>
>

Thanks for submitting this!

> Add a size check for allcation since the allocation size is

nit: "allocation" above.

> always less than the total DRAM size.

In general, it might be good to add more context to the commit message
to better answer *why* this change is needed rather than what the
change is doing.  ie: What negative thing happens without this change?
And so how does this change avoid or improve things?


> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> ---
> v2: 1. update size limitation as total_dram page size.
>     2. update commit message
> ---
>  drivers/dma-buf/dma-heap.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 56bf5ad01ad5..e39d2be98d69 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>         struct dma_buf *dmabuf;
>         int fd;
>
> +       if (len / PAGE_SIZE > totalram_pages())
> +               return -EINVAL;

This seems sane. I know ION used to have some 1/2 of memory cap to
avoid unnecessary memory pressure on crazy allocations.

Could you send again with an improved commit message?

thanks
-john

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[Christian König]

Am 03.01.22 um 19:57 schrieb John Stultz:
> On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
>> From: Guangming <Guangming.Cao@mediatek.com>
>>
> Thanks for submitting this!
>
>> Add a size check for allcation since the allocation size is
> nit: "allocation" above.
>
>> always less than the total DRAM size.
> In general, it might be good to add more context to the commit message
> to better answer *why* this change is needed rather than what the
> change is doing.  ie: What negative thing happens without this change?
> And so how does this change avoid or improve things?

Completely agree, just one little addition: Could you also add this why 
as comment to the code?

When we stumble over this five years from now it is absolutely not 
obvious why we do this.

Thanks,
Christian.

>
>
>> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>> ---
>> v2: 1. update size limitation as total_dram page size.
>>      2. update commit message
>> ---
>>   drivers/dma-buf/dma-heap.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>> index 56bf5ad01ad5..e39d2be98d69 100644
>> --- a/drivers/dma-buf/dma-heap.c
>> +++ b/drivers/dma-buf/dma-heap.c
>> @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>>          struct dma_buf *dmabuf;
>>          int fd;
>>
>> +       if (len / PAGE_SIZE > totalram_pages())
>> +               return -EINVAL;
> This seems sane. I know ION used to have some 1/2 of memory cap to
> avoid unnecessary memory pressure on crazy allocations.
>
> Could you send again with an improved commit message?
>
> thanks
> -john

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[Christian König]

Am 03.01.22 um 19:57 schrieb John Stultz:
> On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
>> From: Guangming <Guangming.Cao@mediatek.com>
>>
> Thanks for submitting this!
>
>> Add a size check for allcation since the allocation size is
> nit: "allocation" above.
>
>> always less than the total DRAM size.
> In general, it might be good to add more context to the commit message
> to better answer *why* this change is needed rather than what the
> change is doing.  ie: What negative thing happens without this change?
> And so how does this change avoid or improve things?

Completely agree, just one little addition: Could you also add this why 
as comment to the code?

When we stumble over this five years from now it is absolutely not 
obvious why we do this.

Thanks,
Christian.

>
>
>> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>> ---
>> v2: 1. update size limitation as total_dram page size.
>>      2. update commit message
>> ---
>>   drivers/dma-buf/dma-heap.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>> index 56bf5ad01ad5..e39d2be98d69 100644
>> --- a/drivers/dma-buf/dma-heap.c
>> +++ b/drivers/dma-buf/dma-heap.c
>> @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>>          struct dma_buf *dmabuf;
>>          int fd;
>>
>> +       if (len / PAGE_SIZE > totalram_pages())
>> +               return -EINVAL;
> This seems sane. I know ION used to have some 1/2 of memory cap to
> avoid unnecessary memory pressure on crazy allocations.
>
> Could you send again with an improved commit message?
>
> thanks
> -john


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[Christian König]

Am 03.01.22 um 19:57 schrieb John Stultz:
> On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
>> From: Guangming <Guangming.Cao@mediatek.com>
>>
> Thanks for submitting this!
>
>> Add a size check for allcation since the allocation size is
> nit: "allocation" above.
>
>> always less than the total DRAM size.
> In general, it might be good to add more context to the commit message
> to better answer *why* this change is needed rather than what the
> change is doing.  ie: What negative thing happens without this change?
> And so how does this change avoid or improve things?

Completely agree, just one little addition: Could you also add this why 
as comment to the code?

When we stumble over this five years from now it is absolutely not 
obvious why we do this.

Thanks,
Christian.

>
>
>> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>> ---
>> v2: 1. update size limitation as total_dram page size.
>>      2. update commit message
>> ---
>>   drivers/dma-buf/dma-heap.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>> index 56bf5ad01ad5..e39d2be98d69 100644
>> --- a/drivers/dma-buf/dma-heap.c
>> +++ b/drivers/dma-buf/dma-heap.c
>> @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>>          struct dma_buf *dmabuf;
>>          int fd;
>>
>> +       if (len / PAGE_SIZE > totalram_pages())
>> +               return -EINVAL;
> This seems sane. I know ION used to have some 1/2 of memory cap to
> avoid unnecessary memory pressure on crazy allocations.
>
> Could you send again with an improved commit message?
>
> thanks
> -john

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[Christian König]

Am 03.01.22 um 19:57 schrieb John Stultz:
> On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
>> From: Guangming <Guangming.Cao@mediatek.com>
>>
> Thanks for submitting this!
>
>> Add a size check for allcation since the allocation size is
> nit: "allocation" above.
>
>> always less than the total DRAM size.
> In general, it might be good to add more context to the commit message
> to better answer *why* this change is needed rather than what the
> change is doing.  ie: What negative thing happens without this change?
> And so how does this change avoid or improve things?

Completely agree, just one little addition: Could you also add this why 
as comment to the code?

When we stumble over this five years from now it is absolutely not 
obvious why we do this.

Thanks,
Christian.

>
>
>> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>> ---
>> v2: 1. update size limitation as total_dram page size.
>>      2. update commit message
>> ---
>>   drivers/dma-buf/dma-heap.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>> index 56bf5ad01ad5..e39d2be98d69 100644
>> --- a/drivers/dma-buf/dma-heap.c
>> +++ b/drivers/dma-buf/dma-heap.c
>> @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>>          struct dma_buf *dmabuf;
>>          int fd;
>>
>> +       if (len / PAGE_SIZE > totalram_pages())
>> +               return -EINVAL;
> This seems sane. I know ION used to have some 1/2 of memory cap to
> avoid unnecessary memory pressure on crazy allocations.
>
> Could you send again with an improved commit message?
>
> thanks
> -john


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[Guangming.Cao]

On Tue, 2022-01-04 at 08:47 +0100, Christian König wrote:
> Am 03.01.22 um 19:57 schrieb John Stultz:
> > On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
> > > From: Guangming <Guangming.Cao@mediatek.com>
> > > 
> > 
> > Thanks for submitting this!
> > 
> > > Add a size check for allcation since the allocation size is
> > 
> > nit: "allocation" above.
> > 
> > > always less than the total DRAM size.
> > 
> > In general, it might be good to add more context to the commit
> > message
> > to better answer *why* this change is needed rather than what the
> > change is doing.  ie: What negative thing happens without this
> > change?
> > And so how does this change avoid or improve things?
> 
> Completely agree, just one little addition: Could you also add this
> why 
> as comment to the code?
> 
> When we stumble over this five years from now it is absolutely not 
> obvious why we do this.
Thanks for your reply!
I will update the related reason in the patch later.

The reason for adding this check is that we met a case that the user
sent an invalid size(It seems it's a negative value, MSB is 0xff, it's
larger than DRAM size after convert it to size_t) to dma-heap to alloc
memory, and this allocation was running on a process(such as "gralloc"
on Android device) can't be killed by OOM flow, and we also couldn't
find the related dmabuf in "dma_buf_debug_show" because the related
dmabuf was not exported yet since the allocation is still on going.

Since this invalid argument case can be prevented at dma-heap side, so,
I added this size check, and moreover, to let debug it easily, I also
added logs when size is bigger than a threshold we set in mtk system
heap.
If you think that print logs in dma-heap framework is better, I will
update it in next version.

If you have better solution(such as dump the size under allocating
in "dma_buf_debug_show", which maybe need add global variable to record
it), please kindly let me know, thanks :)

> 
> Thanks,
> Christian.
> 
> > 
> > 
> > > Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> > > Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> > > ---
> > > v2: 1. update size limitation as total_dram page size.
> > >      2. update commit message
> > > ---
> > >   drivers/dma-buf/dma-heap.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-
> > > heap.c
> > > index 56bf5ad01ad5..e39d2be98d69 100644
> > > --- a/drivers/dma-buf/dma-heap.c
> > > +++ b/drivers/dma-buf/dma-heap.c
> > > @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct
> > > dma_heap *heap, size_t len,
> > >          struct dma_buf *dmabuf;
> > >          int fd;
> > > 
> > > +       if (len / PAGE_SIZE > totalram_pages())
> > > +               return -EINVAL;
> > 
> > This seems sane. I know ION used to have some 1/2 of memory cap to
> > avoid unnecessary memory pressure on crazy allocations.
> > 
> > Could you send again with an improved commit message?
> > 
> > thanks
> > -john
> 
> 

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[Guangming.Cao]

On Tue, 2022-01-04 at 08:47 +0100, Christian König wrote:
> Am 03.01.22 um 19:57 schrieb John Stultz:
> > On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
> > > From: Guangming <Guangming.Cao@mediatek.com>
> > > 
> > 
> > Thanks for submitting this!
> > 
> > > Add a size check for allcation since the allocation size is
> > 
> > nit: "allocation" above.
> > 
> > > always less than the total DRAM size.
> > 
> > In general, it might be good to add more context to the commit
> > message
> > to better answer *why* this change is needed rather than what the
> > change is doing.  ie: What negative thing happens without this
> > change?
> > And so how does this change avoid or improve things?
> 
> Completely agree, just one little addition: Could you also add this
> why 
> as comment to the code?
> 
> When we stumble over this five years from now it is absolutely not 
> obvious why we do this.
Thanks for your reply!
I will update the related reason in the patch later.

The reason for adding this check is that we met a case that the user
sent an invalid size(It seems it's a negative value, MSB is 0xff, it's
larger than DRAM size after convert it to size_t) to dma-heap to alloc
memory, and this allocation was running on a process(such as "gralloc"
on Android device) can't be killed by OOM flow, and we also couldn't
find the related dmabuf in "dma_buf_debug_show" because the related
dmabuf was not exported yet since the allocation is still on going.

Since this invalid argument case can be prevented at dma-heap side, so,
I added this size check, and moreover, to let debug it easily, I also
added logs when size is bigger than a threshold we set in mtk system
heap.
If you think that print logs in dma-heap framework is better, I will
update it in next version.

If you have better solution(such as dump the size under allocating
in "dma_buf_debug_show", which maybe need add global variable to record
it), please kindly let me know, thanks :)

> 
> Thanks,
> Christian.
> 
> > 
> > 
> > > Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> > > Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> > > ---
> > > v2: 1. update size limitation as total_dram page size.
> > >      2. update commit message
> > > ---
> > >   drivers/dma-buf/dma-heap.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-
> > > heap.c
> > > index 56bf5ad01ad5..e39d2be98d69 100644
> > > --- a/drivers/dma-buf/dma-heap.c
> > > +++ b/drivers/dma-buf/dma-heap.c
> > > @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct
> > > dma_heap *heap, size_t len,
> > >          struct dma_buf *dmabuf;
> > >          int fd;
> > > 
> > > +       if (len / PAGE_SIZE > totalram_pages())
> > > +               return -EINVAL;
> > 
> > This seems sane. I know ION used to have some 1/2 of memory cap to
> > avoid unnecessary memory pressure on crazy allocations.
> > 
> > Could you send again with an improved commit message?
> > 
> > thanks
> > -john
> 
> 
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[Guangming.Cao]

On Tue, 2022-01-04 at 08:47 +0100, Christian König wrote:
> Am 03.01.22 um 19:57 schrieb John Stultz:
> > On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
> > > From: Guangming <Guangming.Cao@mediatek.com>
> > > 
> > 
> > Thanks for submitting this!
> > 
> > > Add a size check for allcation since the allocation size is
> > 
> > nit: "allocation" above.
> > 
> > > always less than the total DRAM size.
> > 
> > In general, it might be good to add more context to the commit
> > message
> > to better answer *why* this change is needed rather than what the
> > change is doing.  ie: What negative thing happens without this
> > change?
> > And so how does this change avoid or improve things?
> 
> Completely agree, just one little addition: Could you also add this
> why 
> as comment to the code?
> 
> When we stumble over this five years from now it is absolutely not 
> obvious why we do this.
Thanks for your reply!
I will update the related reason in the patch later.

The reason for adding this check is that we met a case that the user
sent an invalid size(It seems it's a negative value, MSB is 0xff, it's
larger than DRAM size after convert it to size_t) to dma-heap to alloc
memory, and this allocation was running on a process(such as "gralloc"
on Android device) can't be killed by OOM flow, and we also couldn't
find the related dmabuf in "dma_buf_debug_show" because the related
dmabuf was not exported yet since the allocation is still on going.

Since this invalid argument case can be prevented at dma-heap side, so,
I added this size check, and moreover, to let debug it easily, I also
added logs when size is bigger than a threshold we set in mtk system
heap.
If you think that print logs in dma-heap framework is better, I will
update it in next version.

If you have better solution(such as dump the size under allocating
in "dma_buf_debug_show", which maybe need add global variable to record
it), please kindly let me know, thanks :)

> 
> Thanks,
> Christian.
> 
> > 
> > 
> > > Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> > > Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> > > ---
> > > v2: 1. update size limitation as total_dram page size.
> > >      2. update commit message
> > > ---
> > >   drivers/dma-buf/dma-heap.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-
> > > heap.c
> > > index 56bf5ad01ad5..e39d2be98d69 100644
> > > --- a/drivers/dma-buf/dma-heap.c
> > > +++ b/drivers/dma-buf/dma-heap.c
> > > @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct
> > > dma_heap *heap, size_t len,
> > >          struct dma_buf *dmabuf;
> > >          int fd;
> > > 
> > > +       if (len / PAGE_SIZE > totalram_pages())
> > > +               return -EINVAL;
> > 
> > This seems sane. I know ION used to have some 1/2 of memory cap to
> > avoid unnecessary memory pressure on crazy allocations.
> > 
> > Could you send again with an improved commit message?
> > 
> > thanks
> > -john
> 
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[guangming.cao]

From: Guangming.Cao <guangming.cao@mediatek.com>

On Tue, 2022-01-04 at 08:47 +0100, Christian K鰊ig wrote:
> Am 03.01.22 um 19:57 schrieb John Stultz:
> > On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
> > > From: Guangming <Guangming.Cao@mediatek.com>
> > > 
> > 
> > Thanks for submitting this!
> > 
> > > Add a size check for allcation since the allocation size is
> > 
> > nit: "allocation" above.
> > 
> > > always less than the total DRAM size.
> > 
> > In general, it might be good to add more context to the commit
> > message
> > to better answer *why* this change is needed rather than what the
> > change is doing.  ie: What negative thing happens without this
> > change?
> > And so how does this change avoid or improve things?
> 
> Completely agree, just one little addition: Could you also add this
> why 
> as comment to the code?
> 
> When we stumble over this five years from now it is absolutely not 
> obvious why we do this.
> 
> Thanks,
> Christian.
> 
Thanks for your reply!
I will update the related reason in the patch later.

The reason for adding this check is that we met a case that the user
sent an invalid size(It seems it's a negative value, MSB is 0xff, it's
larger than DRAM size after convert it to size_t) to dma-heap to alloc
memory, and this allocation was running on a process(such as "gralloc"
on Android device) can't be killed by OOM flow, and we also couldn't
find the related dmabuf in "dma_buf_debug_show" because the related
dmabuf was not exported yet since the allocation is still on going.

Since this invalid argument case can be prevented at dma-heap side, so,
I added this size check, and moreover, to let debug it easily, I also
added logs when size is bigger than a threshold we set in mtk system
heap.
If you think that print logs in dma-heap framework is better, I will
update it in next version.

If you have better solution(such as dump the size under allocating
in "dma_buf_debug_show", which maybe need add global variable to record
it), please kindly let me know.
Thanks :)
Guangming

> > 
> > 
> > > Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> > > Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> > > ---
> > > v2: 1. update size limitation as total_dram page size.
> > >      2. update commit message
> > > ---
> > >   drivers/dma-buf/dma-heap.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-
> > > heap.c
> > > index 56bf5ad01ad5..e39d2be98d69 100644
> > > --- a/drivers/dma-buf/dma-heap.c
> > > +++ b/drivers/dma-buf/dma-heap.c
> > > @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct
> > > dma_heap *heap, size_t len,
> > >          struct dma_buf *dmabuf;
> > >          int fd;
> > > 
> > > +       if (len / PAGE_SIZE > totalram_pages())
> > > +               return -EINVAL;
> > 
> > This seems sane. I know ION used to have some 1/2 of memory cap to
> > avoid unnecessary memory pressure on crazy allocations.
> > 
> > Could you send again with an improved commit message?
> > 
> > thanks
> > -john
> 
> 

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[guangming.cao]

From: Guangming.Cao <guangming.cao@mediatek.com>

On Tue, 2022-01-04 at 08:47 +0100, Christian K鰊ig wrote:
> Am 03.01.22 um 19:57 schrieb John Stultz:
> > On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
> > > From: Guangming <Guangming.Cao@mediatek.com>
> > > 
> > 
> > Thanks for submitting this!
> > 
> > > Add a size check for allcation since the allocation size is
> > 
> > nit: "allocation" above.
> > 
> > > always less than the total DRAM size.
> > 
> > In general, it might be good to add more context to the commit
> > message
> > to better answer *why* this change is needed rather than what the
> > change is doing.  ie: What negative thing happens without this
> > change?
> > And so how does this change avoid or improve things?
> 
> Completely agree, just one little addition: Could you also add this
> why 
> as comment to the code?
> 
> When we stumble over this five years from now it is absolutely not 
> obvious why we do this.
> 
> Thanks,
> Christian.
> 
Thanks for your reply!
I will update the related reason in the patch later.

The reason for adding this check is that we met a case that the user
sent an invalid size(It seems it's a negative value, MSB is 0xff, it's
larger than DRAM size after convert it to size_t) to dma-heap to alloc
memory, and this allocation was running on a process(such as "gralloc"
on Android device) can't be killed by OOM flow, and we also couldn't
find the related dmabuf in "dma_buf_debug_show" because the related
dmabuf was not exported yet since the allocation is still on going.

Since this invalid argument case can be prevented at dma-heap side, so,
I added this size check, and moreover, to let debug it easily, I also
added logs when size is bigger than a threshold we set in mtk system
heap.
If you think that print logs in dma-heap framework is better, I will
update it in next version.

If you have better solution(such as dump the size under allocating
in "dma_buf_debug_show", which maybe need add global variable to record
it), please kindly let me know.
Thanks :)
Guangming

> > 
> > 
> > > Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> > > Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> > > ---
> > > v2: 1. update size limitation as total_dram page size.
> > >      2. update commit message
> > > ---
> > >   drivers/dma-buf/dma-heap.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-
> > > heap.c
> > > index 56bf5ad01ad5..e39d2be98d69 100644
> > > --- a/drivers/dma-buf/dma-heap.c
> > > +++ b/drivers/dma-buf/dma-heap.c
> > > @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct
> > > dma_heap *heap, size_t len,
> > >          struct dma_buf *dmabuf;
> > >          int fd;
> > > 
> > > +       if (len / PAGE_SIZE > totalram_pages())
> > > +               return -EINVAL;
> > 
> > This seems sane. I know ION used to have some 1/2 of memory cap to
> > avoid unnecessary memory pressure on crazy allocations.
> > 
> > Could you send again with an improved commit message?
> > 
> > thanks
> > -john
> 
> 

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[guangming.cao]

From: Guangming.Cao <guangming.cao@mediatek.com>

On Tue, 2022-01-04 at 08:47 +0100, Christian K鰊ig wrote:
> Am 03.01.22 um 19:57 schrieb John Stultz:
> > On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
> > > From: Guangming <Guangming.Cao@mediatek.com>
> > > 
> > 
> > Thanks for submitting this!
> > 
> > > Add a size check for allcation since the allocation size is
> > 
> > nit: "allocation" above.
> > 
> > > always less than the total DRAM size.
> > 
> > In general, it might be good to add more context to the commit
> > message
> > to better answer *why* this change is needed rather than what the
> > change is doing.  ie: What negative thing happens without this
> > change?
> > And so how does this change avoid or improve things?
> 
> Completely agree, just one little addition: Could you also add this
> why 
> as comment to the code?
> 
> When we stumble over this five years from now it is absolutely not 
> obvious why we do this.
> 
> Thanks,
> Christian.
> 
Thanks for your reply!
I will update the related reason in the patch later.

The reason for adding this check is that we met a case that the user
sent an invalid size(It seems it's a negative value, MSB is 0xff, it's
larger than DRAM size after convert it to size_t) to dma-heap to alloc
memory, and this allocation was running on a process(such as "gralloc"
on Android device) can't be killed by OOM flow, and we also couldn't
find the related dmabuf in "dma_buf_debug_show" because the related
dmabuf was not exported yet since the allocation is still on going.

Since this invalid argument case can be prevented at dma-heap side, so,
I added this size check, and moreover, to let debug it easily, I also
added logs when size is bigger than a threshold we set in mtk system
heap.
If you think that print logs in dma-heap framework is better, I will
update it in next version.

If you have better solution(such as dump the size under allocating
in "dma_buf_debug_show", which maybe need add global variable to record
it), please kindly let me know.
Thanks :)
Guangming

> > 
> > 
> > > Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> > > Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> > > ---
> > > v2: 1. update size limitation as total_dram page size.
> > >      2. update commit message
> > > ---
> > >   drivers/dma-buf/dma-heap.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-
> > > heap.c
> > > index 56bf5ad01ad5..e39d2be98d69 100644
> > > --- a/drivers/dma-buf/dma-heap.c
> > > +++ b/drivers/dma-buf/dma-heap.c
> > > @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct
> > > dma_heap *heap, size_t len,
> > >          struct dma_buf *dmabuf;
> > >          int fd;
> > > 
> > > +       if (len / PAGE_SIZE > totalram_pages())
> > > +               return -EINVAL;
> > 
> > This seems sane. I know ION used to have some 1/2 of memory cap to
> > avoid unnecessary memory pressure on crazy allocations.
> > 
> > Could you send again with an improved commit message?
> > 
> > thanks
> > -john
> 
> 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[guangming.cao]

From: Guangming.Cao <guangming.cao@mediatek.com>

On Tue, 2022-01-04 at 08:47 +0100, Christian K鰊ig wrote:
> Am 03.01.22 um 19:57 schrieb John Stultz:
> > On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
> > > From: Guangming <Guangming.Cao@mediatek.com>
> > > 
> > 
> > Thanks for submitting this!
> > 
> > > Add a size check for allcation since the allocation size is
> > 
> > nit: "allocation" above.
> > 
> > > always less than the total DRAM size.
> > 
> > In general, it might be good to add more context to the commit
> > message
> > to better answer *why* this change is needed rather than what the
> > change is doing.  ie: What negative thing happens without this
> > change?
> > And so how does this change avoid or improve things?
> 
> Completely agree, just one little addition: Could you also add this
> why 
> as comment to the code?
> 
> When we stumble over this five years from now it is absolutely not 
> obvious why we do this.
> 
> Thanks,
> Christian.
> 
Thanks for your reply!
I will update the related reason in the patch later.

The reason for adding this check is that we met a case that the user
sent an invalid size(It seems it's a negative value, MSB is 0xff, it's
larger than DRAM size after convert it to size_t) to dma-heap to alloc
memory, and this allocation was running on a process(such as "gralloc"
on Android device) can't be killed by OOM flow, and we also couldn't
find the related dmabuf in "dma_buf_debug_show" because the related
dmabuf was not exported yet since the allocation is still on going.

Since this invalid argument case can be prevented at dma-heap side, so,
I added this size check, and moreover, to let debug it easily, I also
added logs when size is bigger than a threshold we set in mtk system
heap.
If you think that print logs in dma-heap framework is better, I will
update it in next version.

If you have better solution(such as dump the size under allocating
in "dma_buf_debug_show", which maybe need add global variable to record
it), please kindly let me know.
Thanks :)
Guangming

> > 
> > 
> > > Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> > > Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> > > ---
> > > v2: 1. update size limitation as total_dram page size.
> > >      2. update commit message
> > > ---
> > >   drivers/dma-buf/dma-heap.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-
> > > heap.c
> > > index 56bf5ad01ad5..e39d2be98d69 100644
> > > --- a/drivers/dma-buf/dma-heap.c
> > > +++ b/drivers/dma-buf/dma-heap.c
> > > @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct
> > > dma_heap *heap, size_t len,
> > >          struct dma_buf *dmabuf;
> > >          int fd;
> > > 
> > > +       if (len / PAGE_SIZE > totalram_pages())
> > > +               return -EINVAL;
> > 
> > This seems sane. I know ION used to have some 1/2 of memory cap to
> > avoid unnecessary memory pressure on crazy allocations.
> > 
> > Could you send again with an improved commit message?
> > 
> > thanks
> > -john
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[Sumit Semwal]

Hello Guangming,

On Wed, 5 Jan 2022 at 12:05, <guangming.cao@mediatek.com> wrote:
>
> From: Guangming.Cao <guangming.cao@mediatek.com>
>
> On Tue, 2022-01-04 at 08:47 +0100, Christian K鰊ig wrote:
> > Am 03.01.22 um 19:57 schrieb John Stultz:
> > > On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
> > > > From: Guangming <Guangming.Cao@mediatek.com>
> > > >
> > >
> > > Thanks for submitting this!
> > >
> > > > Add a size check for allcation since the allocation size is
> > >
> > > nit: "allocation" above.
> > >
> > > > always less than the total DRAM size.
> > >
> > > In general, it might be good to add more context to the commit
> > > message
> > > to better answer *why* this change is needed rather than what the
> > > change is doing.  ie: What negative thing happens without this
> > > change?
> > > And so how does this change avoid or improve things?
> >
> > Completely agree, just one little addition: Could you also add this
> > why
> > as comment to the code?
> >
> > When we stumble over this five years from now it is absolutely not
> > obvious why we do this.
> >
> > Thanks,
> > Christian.
> >
> Thanks for your reply!
> I will update the related reason in the patch later.
>
> The reason for adding this check is that we met a case that the user
> sent an invalid size(It seems it's a negative value, MSB is 0xff, it's
> larger than DRAM size after convert it to size_t) to dma-heap to alloc
> memory, and this allocation was running on a process(such as "gralloc"
> on Android device) can't be killed by OOM flow, and we also couldn't
> find the related dmabuf in "dma_buf_debug_show" because the related
> dmabuf was not exported yet since the allocation is still on going.
>
> Since this invalid argument case can be prevented at dma-heap side, so,
> I added this size check, and moreover, to let debug it easily, I also
> added logs when size is bigger than a threshold we set in mtk system
> heap.
> If you think that print logs in dma-heap framework is better, I will
> update it in next version.
>
> If you have better solution(such as dump the size under allocating
> in "dma_buf_debug_show", which maybe need add global variable to record
> it), please kindly let me know.

Thank you for the patch!

I think just adding the reasoning above as the commit message and a
comment in the code should be enough for now; the debug parts may be
easy to add in case someone runs into issues.

> Thanks :)
> Guangming

Best,
Sumit.

>
> > >
> > >
> > > > Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> > > > Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> > > > ---
> > > > v2: 1. update size limitation as total_dram page size.
> > > >      2. update commit message
> > > > ---
> > > >   drivers/dma-buf/dma-heap.c | 2 ++
> > > >   1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-
> > > > heap.c
> > > > index 56bf5ad01ad5..e39d2be98d69 100644
> > > > --- a/drivers/dma-buf/dma-heap.c
> > > > +++ b/drivers/dma-buf/dma-heap.c
> > > > @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct
> > > > dma_heap *heap, size_t len,
> > > >          struct dma_buf *dmabuf;
> > > >          int fd;
> > > >
> > > > +       if (len / PAGE_SIZE > totalram_pages())
> > > > +               return -EINVAL;
> > >
> > > This seems sane. I know ION used to have some 1/2 of memory cap to
> > > avoid unnecessary memory pressure on crazy allocations.
> > >
> > > Could you send again with an improved commit message?
> > >
> > > thanks
> > > -john
> >
> >



--
Thanks and regards,

Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[Sumit Semwal]

Hello Guangming,

On Wed, 5 Jan 2022 at 12:05, <guangming.cao@mediatek.com> wrote:
>
> From: Guangming.Cao <guangming.cao@mediatek.com>
>
> On Tue, 2022-01-04 at 08:47 +0100, Christian K鰊ig wrote:
> > Am 03.01.22 um 19:57 schrieb John Stultz:
> > > On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
> > > > From: Guangming <Guangming.Cao@mediatek.com>
> > > >
> > >
> > > Thanks for submitting this!
> > >
> > > > Add a size check for allcation since the allocation size is
> > >
> > > nit: "allocation" above.
> > >
> > > > always less than the total DRAM size.
> > >
> > > In general, it might be good to add more context to the commit
> > > message
> > > to better answer *why* this change is needed rather than what the
> > > change is doing.  ie: What negative thing happens without this
> > > change?
> > > And so how does this change avoid or improve things?
> >
> > Completely agree, just one little addition: Could you also add this
> > why
> > as comment to the code?
> >
> > When we stumble over this five years from now it is absolutely not
> > obvious why we do this.
> >
> > Thanks,
> > Christian.
> >
> Thanks for your reply!
> I will update the related reason in the patch later.
>
> The reason for adding this check is that we met a case that the user
> sent an invalid size(It seems it's a negative value, MSB is 0xff, it's
> larger than DRAM size after convert it to size_t) to dma-heap to alloc
> memory, and this allocation was running on a process(such as "gralloc"
> on Android device) can't be killed by OOM flow, and we also couldn't
> find the related dmabuf in "dma_buf_debug_show" because the related
> dmabuf was not exported yet since the allocation is still on going.
>
> Since this invalid argument case can be prevented at dma-heap side, so,
> I added this size check, and moreover, to let debug it easily, I also
> added logs when size is bigger than a threshold we set in mtk system
> heap.
> If you think that print logs in dma-heap framework is better, I will
> update it in next version.
>
> If you have better solution(such as dump the size under allocating
> in "dma_buf_debug_show", which maybe need add global variable to record
> it), please kindly let me know.

Thank you for the patch!

I think just adding the reasoning above as the commit message and a
comment in the code should be enough for now; the debug parts may be
easy to add in case someone runs into issues.

> Thanks :)
> Guangming

Best,
Sumit.

>
> > >
> > >
> > > > Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> > > > Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> > > > ---
> > > > v2: 1. update size limitation as total_dram page size.
> > > >      2. update commit message
> > > > ---
> > > >   drivers/dma-buf/dma-heap.c | 2 ++
> > > >   1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-
> > > > heap.c
> > > > index 56bf5ad01ad5..e39d2be98d69 100644
> > > > --- a/drivers/dma-buf/dma-heap.c
> > > > +++ b/drivers/dma-buf/dma-heap.c
> > > > @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct
> > > > dma_heap *heap, size_t len,
> > > >          struct dma_buf *dmabuf;
> > > >          int fd;
> > > >
> > > > +       if (len / PAGE_SIZE > totalram_pages())
> > > > +               return -EINVAL;
> > >
> > > This seems sane. I know ION used to have some 1/2 of memory cap to
> > > avoid unnecessary memory pressure on crazy allocations.
> > >
> > > Could you send again with an improved commit message?
> > >
> > > thanks
> > > -john
> >
> >



--
Thanks and regards,

Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[Sumit Semwal]

Hello Guangming,

On Wed, 5 Jan 2022 at 12:05, <guangming.cao@mediatek.com> wrote:
>
> From: Guangming.Cao <guangming.cao@mediatek.com>
>
> On Tue, 2022-01-04 at 08:47 +0100, Christian K鰊ig wrote:
> > Am 03.01.22 um 19:57 schrieb John Stultz:
> > > On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
> > > > From: Guangming <Guangming.Cao@mediatek.com>
> > > >
> > >
> > > Thanks for submitting this!
> > >
> > > > Add a size check for allcation since the allocation size is
> > >
> > > nit: "allocation" above.
> > >
> > > > always less than the total DRAM size.
> > >
> > > In general, it might be good to add more context to the commit
> > > message
> > > to better answer *why* this change is needed rather than what the
> > > change is doing.  ie: What negative thing happens without this
> > > change?
> > > And so how does this change avoid or improve things?
> >
> > Completely agree, just one little addition: Could you also add this
> > why
> > as comment to the code?
> >
> > When we stumble over this five years from now it is absolutely not
> > obvious why we do this.
> >
> > Thanks,
> > Christian.
> >
> Thanks for your reply!
> I will update the related reason in the patch later.
>
> The reason for adding this check is that we met a case that the user
> sent an invalid size(It seems it's a negative value, MSB is 0xff, it's
> larger than DRAM size after convert it to size_t) to dma-heap to alloc
> memory, and this allocation was running on a process(such as "gralloc"
> on Android device) can't be killed by OOM flow, and we also couldn't
> find the related dmabuf in "dma_buf_debug_show" because the related
> dmabuf was not exported yet since the allocation is still on going.
>
> Since this invalid argument case can be prevented at dma-heap side, so,
> I added this size check, and moreover, to let debug it easily, I also
> added logs when size is bigger than a threshold we set in mtk system
> heap.
> If you think that print logs in dma-heap framework is better, I will
> update it in next version.
>
> If you have better solution(such as dump the size under allocating
> in "dma_buf_debug_show", which maybe need add global variable to record
> it), please kindly let me know.

Thank you for the patch!

I think just adding the reasoning above as the commit message and a
comment in the code should be enough for now; the debug parts may be
easy to add in case someone runs into issues.

> Thanks :)
> Guangming

Best,
Sumit.

>
> > >
> > >
> > > > Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> > > > Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> > > > ---
> > > > v2: 1. update size limitation as total_dram page size.
> > > >      2. update commit message
> > > > ---
> > > >   drivers/dma-buf/dma-heap.c | 2 ++
> > > >   1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-
> > > > heap.c
> > > > index 56bf5ad01ad5..e39d2be98d69 100644
> > > > --- a/drivers/dma-buf/dma-heap.c
> > > > +++ b/drivers/dma-buf/dma-heap.c
> > > > @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct
> > > > dma_heap *heap, size_t len,
> > > >          struct dma_buf *dmabuf;
> > > >          int fd;
> > > >
> > > > +       if (len / PAGE_SIZE > totalram_pages())
> > > > +               return -EINVAL;
> > >
> > > This seems sane. I know ION used to have some 1/2 of memory cap to
> > > avoid unnecessary memory pressure on crazy allocations.
> > >
> > > Could you send again with an improved commit message?
> > >
> > > thanks
> > > -john
> >
> >



--
Thanks and regards,

Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs

Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation

[Sumit Semwal]

Hello Guangming,

On Wed, 5 Jan 2022 at 12:05, <guangming.cao@mediatek.com> wrote:
>
> From: Guangming.Cao <guangming.cao@mediatek.com>
>
> On Tue, 2022-01-04 at 08:47 +0100, Christian K鰊ig wrote:
> > Am 03.01.22 um 19:57 schrieb John Stultz:
> > > On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
> > > > From: Guangming <Guangming.Cao@mediatek.com>
> > > >
> > >
> > > Thanks for submitting this!
> > >
> > > > Add a size check for allcation since the allocation size is
> > >
> > > nit: "allocation" above.
> > >
> > > > always less than the total DRAM size.
> > >
> > > In general, it might be good to add more context to the commit
> > > message
> > > to better answer *why* this change is needed rather than what the
> > > change is doing.  ie: What negative thing happens without this
> > > change?
> > > And so how does this change avoid or improve things?
> >
> > Completely agree, just one little addition: Could you also add this
> > why
> > as comment to the code?
> >
> > When we stumble over this five years from now it is absolutely not
> > obvious why we do this.
> >
> > Thanks,
> > Christian.
> >
> Thanks for your reply!
> I will update the related reason in the patch later.
>
> The reason for adding this check is that we met a case that the user
> sent an invalid size(It seems it's a negative value, MSB is 0xff, it's
> larger than DRAM size after convert it to size_t) to dma-heap to alloc
> memory, and this allocation was running on a process(such as "gralloc"
> on Android device) can't be killed by OOM flow, and we also couldn't
> find the related dmabuf in "dma_buf_debug_show" because the related
> dmabuf was not exported yet since the allocation is still on going.
>
> Since this invalid argument case can be prevented at dma-heap side, so,
> I added this size check, and moreover, to let debug it easily, I also
> added logs when size is bigger than a threshold we set in mtk system
> heap.
> If you think that print logs in dma-heap framework is better, I will
> update it in next version.
>
> If you have better solution(such as dump the size under allocating
> in "dma_buf_debug_show", which maybe need add global variable to record
> it), please kindly let me know.

Thank you for the patch!

I think just adding the reasoning above as the commit message and a
comment in the code should be enough for now; the debug parts may be
easy to add in case someone runs into issues.

> Thanks :)
> Guangming

Best,
Sumit.

>
> > >
> > >
> > > > Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> > > > Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> > > > ---
> > > > v2: 1. update size limitation as total_dram page size.
> > > >      2. update commit message
> > > > ---
> > > >   drivers/dma-buf/dma-heap.c | 2 ++
> > > >   1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-
> > > > heap.c
> > > > index 56bf5ad01ad5..e39d2be98d69 100644
> > > > --- a/drivers/dma-buf/dma-heap.c
> > > > +++ b/drivers/dma-buf/dma-heap.c
> > > > @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct
> > > > dma_heap *heap, size_t len,
> > > >          struct dma_buf *dmabuf;
> > > >          int fd;
> > > >
> > > > +       if (len / PAGE_SIZE > totalram_pages())
> > > > +               return -EINVAL;
> > >
> > > This seems sane. I know ION used to have some 1/2 of memory cap to
> > > avoid unnecessary memory pressure on crazy allocations.
> > >
> > > Could you send again with an improved commit message?
> > >
> > > thanks
> > > -john
> >
> >



--
Thanks and regards,

Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size is
always less than the total DRAM size.

Without this check, once the invalid size allocation runs on a process that
can't be killed by OOM flow(such as "gralloc" on Android devices), it will
cause a kernel exception, and to make matters worse, we can't find who are using
so many memory with "dma_buf_debug_show" since the relevant dma-buf hasn't exported.

To make OOM issue easier, maybe need dma-buf framework to dump the buffer size
under allocating in "dma_buf_debug_show".

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
---
v3: 1. update patch, use right shift to replace division.
    2. update patch, add reason in code and commit message.
v2: 1. update size limitation as total_dram page size.
    2. update commit message
---
 drivers/dma-buf/dma-heap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 56bf5ad01ad5..1fd382712584 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
 	struct dma_buf *dmabuf;
 	int fd;
 
+	/*
+	 * Invalid size check. The "len" should be less than totalram.
+	 *
+	 * Without this check, once the invalid size allocation runs on a process that
+	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it will
+	 * cause a kernel exception, and to make matters worse, we can't find who are using
+	 * so many memory with "dma_buf_debug_show" since the relevant dma-buf hasn't exported.
+	 */
+	if (len >> PAGE_SHIFT > totalram_pages())
+		return -EINVAL;
 	/*
 	 * Allocations from all heaps have to begin
 	 * and end on page boundaries.
-- 
2.17.1

[PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size is
always less than the total DRAM size.

Without this check, once the invalid size allocation runs on a process that
can't be killed by OOM flow(such as "gralloc" on Android devices), it will
cause a kernel exception, and to make matters worse, we can't find who are using
so many memory with "dma_buf_debug_show" since the relevant dma-buf hasn't exported.

To make OOM issue easier, maybe need dma-buf framework to dump the buffer size
under allocating in "dma_buf_debug_show".

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
---
v3: 1. update patch, use right shift to replace division.
    2. update patch, add reason in code and commit message.
v2: 1. update size limitation as total_dram page size.
    2. update commit message
---
 drivers/dma-buf/dma-heap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 56bf5ad01ad5..1fd382712584 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
 	struct dma_buf *dmabuf;
 	int fd;
 
+	/*
+	 * Invalid size check. The "len" should be less than totalram.
+	 *
+	 * Without this check, once the invalid size allocation runs on a process that
+	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it will
+	 * cause a kernel exception, and to make matters worse, we can't find who are using
+	 * so many memory with "dma_buf_debug_show" since the relevant dma-buf hasn't exported.
+	 */
+	if (len >> PAGE_SHIFT > totalram_pages())
+		return -EINVAL;
 	/*
 	 * Allocations from all heaps have to begin
 	 * and end on page boundaries.
-- 
2.17.1

[PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size is
always less than the total DRAM size.

Without this check, once the invalid size allocation runs on a process that
can't be killed by OOM flow(such as "gralloc" on Android devices), it will
cause a kernel exception, and to make matters worse, we can't find who are using
so many memory with "dma_buf_debug_show" since the relevant dma-buf hasn't exported.

To make OOM issue easier, maybe need dma-buf framework to dump the buffer size
under allocating in "dma_buf_debug_show".

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
---
v3: 1. update patch, use right shift to replace division.
    2. update patch, add reason in code and commit message.
v2: 1. update size limitation as total_dram page size.
    2. update commit message
---
 drivers/dma-buf/dma-heap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 56bf5ad01ad5..1fd382712584 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
 	struct dma_buf *dmabuf;
 	int fd;
 
+	/*
+	 * Invalid size check. The "len" should be less than totalram.
+	 *
+	 * Without this check, once the invalid size allocation runs on a process that
+	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it will
+	 * cause a kernel exception, and to make matters worse, we can't find who are using
+	 * so many memory with "dma_buf_debug_show" since the relevant dma-buf hasn't exported.
+	 */
+	if (len >> PAGE_SHIFT > totalram_pages())
+		return -EINVAL;
 	/*
 	 * Allocations from all heaps have to begin
 	 * and end on page boundaries.
-- 
2.17.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size is
always less than the total DRAM size.

Without this check, once the invalid size allocation runs on a process that
can't be killed by OOM flow(such as "gralloc" on Android devices), it will
cause a kernel exception, and to make matters worse, we can't find who are using
so many memory with "dma_buf_debug_show" since the relevant dma-buf hasn't exported.

To make OOM issue easier, maybe need dma-buf framework to dump the buffer size
under allocating in "dma_buf_debug_show".

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
---
v3: 1. update patch, use right shift to replace division.
    2. update patch, add reason in code and commit message.
v2: 1. update size limitation as total_dram page size.
    2. update commit message
---
 drivers/dma-buf/dma-heap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 56bf5ad01ad5..1fd382712584 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
 	struct dma_buf *dmabuf;
 	int fd;
 
+	/*
+	 * Invalid size check. The "len" should be less than totalram.
+	 *
+	 * Without this check, once the invalid size allocation runs on a process that
+	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it will
+	 * cause a kernel exception, and to make matters worse, we can't find who are using
+	 * so many memory with "dma_buf_debug_show" since the relevant dma-buf hasn't exported.
+	 */
+	if (len >> PAGE_SHIFT > totalram_pages())
+		return -EINVAL;
 	/*
 	 * Allocations from all heaps have to begin
 	 * and end on page boundaries.
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Ruhl, Michael J]

>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>guangming.cao@mediatek.com
>Sent: Thursday, January 13, 2022 7:34 AM
>To: sumit.semwal@linaro.org
>Cc: linux-arm-kernel@lists.infradead.org; mingyuan.ma@mediatek.com;
>Guangming <Guangming.Cao@mediatek.com>;
>wsd_upstream@mediatek.com; linux-kernel@vger.kernel.org; dri-
>devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org;
>yf.wang@mediatek.com; libo.kang@mediatek.com;
>benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>matthias.bgg@gmail.com; linux-mediatek@lists.infradead.org;
>lmark@codeaurora.org; labbott@redhat.com; christian.koenig@amd.com;
>jianjiao.zeng@mediatek.com; linux-media@vger.kernel.org
>Subject: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>
>From: Guangming <Guangming.Cao@mediatek.com>
>
>Add a size check for allocation since the allocation size is
>always less than the total DRAM size.
>
>Without this check, once the invalid size allocation runs on a process that
>can't be killed by OOM flow(such as "gralloc" on Android devices), it will
>cause a kernel exception, and to make matters worse, we can't find who are
>using
>so many memory with "dma_buf_debug_show" since the relevant dma-buf
>hasn't exported.
>
>To make OOM issue easier, maybe need dma-buf framework to dump the
>buffer size
>under allocating in "dma_buf_debug_show".
>
>Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>---
>v3: 1. update patch, use right shift to replace division.
>    2. update patch, add reason in code and commit message.
>v2: 1. update size limitation as total_dram page size.
>    2. update commit message
>---
> drivers/dma-buf/dma-heap.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
>diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>index 56bf5ad01ad5..1fd382712584 100644
>--- a/drivers/dma-buf/dma-heap.c
>+++ b/drivers/dma-buf/dma-heap.c
>@@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap
>*heap, size_t len,
> 	struct dma_buf *dmabuf;
> 	int fd;
>
>+	/*
>+	 * Invalid size check. The "len" should be less than totalram.
>+	 *
>+	 * Without this check, once the invalid size allocation runs on a process
>that
>+	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>will
>+	 * cause a kernel exception, and to make matters worse, we can't find
>who are using
>+	 * so many memory with "dma_buf_debug_show" since the relevant
>dma-buf hasn't exported.
>+	 */
>+	if (len >> PAGE_SHIFT > totalram_pages())

If your "heap" is from cma, is this still a valid check?

M

>+		return -EINVAL;
> 	/*
> 	 * Allocations from all heaps have to begin
> 	 * and end on page boundaries.
>--
>2.17.1

RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Ruhl, Michael J]

>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>guangming.cao@mediatek.com
>Sent: Thursday, January 13, 2022 7:34 AM
>To: sumit.semwal@linaro.org
>Cc: linux-arm-kernel@lists.infradead.org; mingyuan.ma@mediatek.com;
>Guangming <Guangming.Cao@mediatek.com>;
>wsd_upstream@mediatek.com; linux-kernel@vger.kernel.org; dri-
>devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org;
>yf.wang@mediatek.com; libo.kang@mediatek.com;
>benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>matthias.bgg@gmail.com; linux-mediatek@lists.infradead.org;
>lmark@codeaurora.org; labbott@redhat.com; christian.koenig@amd.com;
>jianjiao.zeng@mediatek.com; linux-media@vger.kernel.org
>Subject: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>
>From: Guangming <Guangming.Cao@mediatek.com>
>
>Add a size check for allocation since the allocation size is
>always less than the total DRAM size.
>
>Without this check, once the invalid size allocation runs on a process that
>can't be killed by OOM flow(such as "gralloc" on Android devices), it will
>cause a kernel exception, and to make matters worse, we can't find who are
>using
>so many memory with "dma_buf_debug_show" since the relevant dma-buf
>hasn't exported.
>
>To make OOM issue easier, maybe need dma-buf framework to dump the
>buffer size
>under allocating in "dma_buf_debug_show".
>
>Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>---
>v3: 1. update patch, use right shift to replace division.
>    2. update patch, add reason in code and commit message.
>v2: 1. update size limitation as total_dram page size.
>    2. update commit message
>---
> drivers/dma-buf/dma-heap.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
>diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>index 56bf5ad01ad5..1fd382712584 100644
>--- a/drivers/dma-buf/dma-heap.c
>+++ b/drivers/dma-buf/dma-heap.c
>@@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap
>*heap, size_t len,
> 	struct dma_buf *dmabuf;
> 	int fd;
>
>+	/*
>+	 * Invalid size check. The "len" should be less than totalram.
>+	 *
>+	 * Without this check, once the invalid size allocation runs on a process
>that
>+	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>will
>+	 * cause a kernel exception, and to make matters worse, we can't find
>who are using
>+	 * so many memory with "dma_buf_debug_show" since the relevant
>dma-buf hasn't exported.
>+	 */
>+	if (len >> PAGE_SHIFT > totalram_pages())

If your "heap" is from cma, is this still a valid check?

M

>+		return -EINVAL;
> 	/*
> 	 * Allocations from all heaps have to begin
> 	 * and end on page boundaries.
>--
>2.17.1

RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Ruhl, Michael J]

>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>guangming.cao@mediatek.com
>Sent: Thursday, January 13, 2022 7:34 AM
>To: sumit.semwal@linaro.org
>Cc: linux-arm-kernel@lists.infradead.org; mingyuan.ma@mediatek.com;
>Guangming <Guangming.Cao@mediatek.com>;
>wsd_upstream@mediatek.com; linux-kernel@vger.kernel.org; dri-
>devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org;
>yf.wang@mediatek.com; libo.kang@mediatek.com;
>benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>matthias.bgg@gmail.com; linux-mediatek@lists.infradead.org;
>lmark@codeaurora.org; labbott@redhat.com; christian.koenig@amd.com;
>jianjiao.zeng@mediatek.com; linux-media@vger.kernel.org
>Subject: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>
>From: Guangming <Guangming.Cao@mediatek.com>
>
>Add a size check for allocation since the allocation size is
>always less than the total DRAM size.
>
>Without this check, once the invalid size allocation runs on a process that
>can't be killed by OOM flow(such as "gralloc" on Android devices), it will
>cause a kernel exception, and to make matters worse, we can't find who are
>using
>so many memory with "dma_buf_debug_show" since the relevant dma-buf
>hasn't exported.
>
>To make OOM issue easier, maybe need dma-buf framework to dump the
>buffer size
>under allocating in "dma_buf_debug_show".
>
>Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>---
>v3: 1. update patch, use right shift to replace division.
>    2. update patch, add reason in code and commit message.
>v2: 1. update size limitation as total_dram page size.
>    2. update commit message
>---
> drivers/dma-buf/dma-heap.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
>diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>index 56bf5ad01ad5..1fd382712584 100644
>--- a/drivers/dma-buf/dma-heap.c
>+++ b/drivers/dma-buf/dma-heap.c
>@@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap
>*heap, size_t len,
> 	struct dma_buf *dmabuf;
> 	int fd;
>
>+	/*
>+	 * Invalid size check. The "len" should be less than totalram.
>+	 *
>+	 * Without this check, once the invalid size allocation runs on a process
>that
>+	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>will
>+	 * cause a kernel exception, and to make matters worse, we can't find
>who are using
>+	 * so many memory with "dma_buf_debug_show" since the relevant
>dma-buf hasn't exported.
>+	 */
>+	if (len >> PAGE_SHIFT > totalram_pages())

If your "heap" is from cma, is this still a valid check?

M

>+		return -EINVAL;
> 	/*
> 	 * Allocations from all heaps have to begin
> 	 * and end on page boundaries.
>--
>2.17.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Ruhl, Michael J]

>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>guangming.cao@mediatek.com
>Sent: Thursday, January 13, 2022 7:34 AM
>To: sumit.semwal@linaro.org
>Cc: linux-arm-kernel@lists.infradead.org; mingyuan.ma@mediatek.com;
>Guangming <Guangming.Cao@mediatek.com>;
>wsd_upstream@mediatek.com; linux-kernel@vger.kernel.org; dri-
>devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org;
>yf.wang@mediatek.com; libo.kang@mediatek.com;
>benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>matthias.bgg@gmail.com; linux-mediatek@lists.infradead.org;
>lmark@codeaurora.org; labbott@redhat.com; christian.koenig@amd.com;
>jianjiao.zeng@mediatek.com; linux-media@vger.kernel.org
>Subject: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>
>From: Guangming <Guangming.Cao@mediatek.com>
>
>Add a size check for allocation since the allocation size is
>always less than the total DRAM size.
>
>Without this check, once the invalid size allocation runs on a process that
>can't be killed by OOM flow(such as "gralloc" on Android devices), it will
>cause a kernel exception, and to make matters worse, we can't find who are
>using
>so many memory with "dma_buf_debug_show" since the relevant dma-buf
>hasn't exported.
>
>To make OOM issue easier, maybe need dma-buf framework to dump the
>buffer size
>under allocating in "dma_buf_debug_show".
>
>Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>---
>v3: 1. update patch, use right shift to replace division.
>    2. update patch, add reason in code and commit message.
>v2: 1. update size limitation as total_dram page size.
>    2. update commit message
>---
> drivers/dma-buf/dma-heap.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
>diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>index 56bf5ad01ad5..1fd382712584 100644
>--- a/drivers/dma-buf/dma-heap.c
>+++ b/drivers/dma-buf/dma-heap.c
>@@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap
>*heap, size_t len,
> 	struct dma_buf *dmabuf;
> 	int fd;
>
>+	/*
>+	 * Invalid size check. The "len" should be less than totalram.
>+	 *
>+	 * Without this check, once the invalid size allocation runs on a process
>that
>+	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>will
>+	 * cause a kernel exception, and to make matters worse, we can't find
>who are using
>+	 * so many memory with "dma_buf_debug_show" since the relevant
>dma-buf hasn't exported.
>+	 */
>+	if (len >> PAGE_SHIFT > totalram_pages())

If your "heap" is from cma, is this still a valid check?

M

>+		return -EINVAL;
> 	/*
> 	 * Allocations from all heaps have to begin
> 	 * and end on page boundaries.
>--
>2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Ruhl, Michael J]

>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>Ruhl, Michael J
>Sent: Thursday, January 13, 2022 7:58 AM
>To: guangming.cao@mediatek.com; sumit.semwal@linaro.org
>Cc: jianjiao.zeng@mediatek.com; lmark@codeaurora.org;
>wsd_upstream@mediatek.com; christian.koenig@amd.com; linux-
>kernel@vger.kernel.org; dri-devel@lists.freedesktop.org;
>yf.wang@mediatek.com; linaro-mm-sig@lists.linaro.org; linux-
>mediatek@lists.infradead.org; libo.kang@mediatek.com;
>benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>matthias.bgg@gmail.com; labbott@redhat.com;
>mingyuan.ma@mediatek.com; linux-arm-kernel@lists.infradead.org; linux-
>media@vger.kernel.org
>Subject: RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>
>
>>-----Original Message-----
>>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>guangming.cao@mediatek.com
>>Sent: Thursday, January 13, 2022 7:34 AM
>>To: sumit.semwal@linaro.org
>>Cc: linux-arm-kernel@lists.infradead.org; mingyuan.ma@mediatek.com;
>>Guangming <Guangming.Cao@mediatek.com>;
>>wsd_upstream@mediatek.com; linux-kernel@vger.kernel.org; dri-
>>devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org;
>>yf.wang@mediatek.com; libo.kang@mediatek.com;
>>benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>>matthias.bgg@gmail.com; linux-mediatek@lists.infradead.org;
>>lmark@codeaurora.org; labbott@redhat.com; christian.koenig@amd.com;
>>jianjiao.zeng@mediatek.com; linux-media@vger.kernel.org
>>Subject: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>>
>>From: Guangming <Guangming.Cao@mediatek.com>
>>
>>Add a size check for allocation since the allocation size is
>>always less than the total DRAM size.
>>
>>Without this check, once the invalid size allocation runs on a process that
>>can't be killed by OOM flow(such as "gralloc" on Android devices), it will
>>cause a kernel exception, and to make matters worse, we can't find who are
>>using
>>so many memory with "dma_buf_debug_show" since the relevant dma-buf
>>hasn't exported.
>>
>>To make OOM issue easier, maybe need dma-buf framework to dump the
>>buffer size
>>under allocating in "dma_buf_debug_show".
>>
>>Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>>Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>>---
>>v3: 1. update patch, use right shift to replace division.
>>    2. update patch, add reason in code and commit message.
>>v2: 1. update size limitation as total_dram page size.
>>    2. update commit message
>>---
>> drivers/dma-buf/dma-heap.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>>diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>>index 56bf5ad01ad5..1fd382712584 100644
>>--- a/drivers/dma-buf/dma-heap.c
>>+++ b/drivers/dma-buf/dma-heap.c
>>@@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap
>>*heap, size_t len,
>> 	struct dma_buf *dmabuf;
>> 	int fd;
>>
>>+	/*
>>+	 * Invalid size check. The "len" should be less than totalram.
>>+	 *
>>+	 * Without this check, once the invalid size allocation runs on a process
>>that
>>+	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>>will
>>+	 * cause a kernel exception, and to make matters worse, we can't find
>>who are using
>>+	 * so many memory with "dma_buf_debug_show" since the relevant
>>dma-buf hasn't exported.
>>+	 */
>>+	if (len >> PAGE_SHIFT > totalram_pages())
>
>If your "heap" is from cma, is this still a valid check?

And thinking a bit further, if I create a heap from something else (say device memory),
you will need to be able to figure out the maximum allowable check for the specific
heap.

Maybe the heap needs a callback for max size?

m
>M
>
>>+		return -EINVAL;
>> 	/*
>> 	 * Allocations from all heaps have to begin
>> 	 * and end on page boundaries.
>>--
>>2.17.1

RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Ruhl, Michael J]

>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>Ruhl, Michael J
>Sent: Thursday, January 13, 2022 7:58 AM
>To: guangming.cao@mediatek.com; sumit.semwal@linaro.org
>Cc: jianjiao.zeng@mediatek.com; lmark@codeaurora.org;
>wsd_upstream@mediatek.com; christian.koenig@amd.com; linux-
>kernel@vger.kernel.org; dri-devel@lists.freedesktop.org;
>yf.wang@mediatek.com; linaro-mm-sig@lists.linaro.org; linux-
>mediatek@lists.infradead.org; libo.kang@mediatek.com;
>benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>matthias.bgg@gmail.com; labbott@redhat.com;
>mingyuan.ma@mediatek.com; linux-arm-kernel@lists.infradead.org; linux-
>media@vger.kernel.org
>Subject: RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>
>
>>-----Original Message-----
>>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>guangming.cao@mediatek.com
>>Sent: Thursday, January 13, 2022 7:34 AM
>>To: sumit.semwal@linaro.org
>>Cc: linux-arm-kernel@lists.infradead.org; mingyuan.ma@mediatek.com;
>>Guangming <Guangming.Cao@mediatek.com>;
>>wsd_upstream@mediatek.com; linux-kernel@vger.kernel.org; dri-
>>devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org;
>>yf.wang@mediatek.com; libo.kang@mediatek.com;
>>benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>>matthias.bgg@gmail.com; linux-mediatek@lists.infradead.org;
>>lmark@codeaurora.org; labbott@redhat.com; christian.koenig@amd.com;
>>jianjiao.zeng@mediatek.com; linux-media@vger.kernel.org
>>Subject: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>>
>>From: Guangming <Guangming.Cao@mediatek.com>
>>
>>Add a size check for allocation since the allocation size is
>>always less than the total DRAM size.
>>
>>Without this check, once the invalid size allocation runs on a process that
>>can't be killed by OOM flow(such as "gralloc" on Android devices), it will
>>cause a kernel exception, and to make matters worse, we can't find who are
>>using
>>so many memory with "dma_buf_debug_show" since the relevant dma-buf
>>hasn't exported.
>>
>>To make OOM issue easier, maybe need dma-buf framework to dump the
>>buffer size
>>under allocating in "dma_buf_debug_show".
>>
>>Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>>Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>>---
>>v3: 1. update patch, use right shift to replace division.
>>    2. update patch, add reason in code and commit message.
>>v2: 1. update size limitation as total_dram page size.
>>    2. update commit message
>>---
>> drivers/dma-buf/dma-heap.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>>diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>>index 56bf5ad01ad5..1fd382712584 100644
>>--- a/drivers/dma-buf/dma-heap.c
>>+++ b/drivers/dma-buf/dma-heap.c
>>@@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap
>>*heap, size_t len,
>> 	struct dma_buf *dmabuf;
>> 	int fd;
>>
>>+	/*
>>+	 * Invalid size check. The "len" should be less than totalram.
>>+	 *
>>+	 * Without this check, once the invalid size allocation runs on a process
>>that
>>+	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>>will
>>+	 * cause a kernel exception, and to make matters worse, we can't find
>>who are using
>>+	 * so many memory with "dma_buf_debug_show" since the relevant
>>dma-buf hasn't exported.
>>+	 */
>>+	if (len >> PAGE_SHIFT > totalram_pages())
>
>If your "heap" is from cma, is this still a valid check?

And thinking a bit further, if I create a heap from something else (say device memory),
you will need to be able to figure out the maximum allowable check for the specific
heap.

Maybe the heap needs a callback for max size?

m
>M
>
>>+		return -EINVAL;
>> 	/*
>> 	 * Allocations from all heaps have to begin
>> 	 * and end on page boundaries.
>>--
>>2.17.1

RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Ruhl, Michael J]

>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>Ruhl, Michael J
>Sent: Thursday, January 13, 2022 7:58 AM
>To: guangming.cao@mediatek.com; sumit.semwal@linaro.org
>Cc: jianjiao.zeng@mediatek.com; lmark@codeaurora.org;
>wsd_upstream@mediatek.com; christian.koenig@amd.com; linux-
>kernel@vger.kernel.org; dri-devel@lists.freedesktop.org;
>yf.wang@mediatek.com; linaro-mm-sig@lists.linaro.org; linux-
>mediatek@lists.infradead.org; libo.kang@mediatek.com;
>benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>matthias.bgg@gmail.com; labbott@redhat.com;
>mingyuan.ma@mediatek.com; linux-arm-kernel@lists.infradead.org; linux-
>media@vger.kernel.org
>Subject: RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>
>
>>-----Original Message-----
>>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>guangming.cao@mediatek.com
>>Sent: Thursday, January 13, 2022 7:34 AM
>>To: sumit.semwal@linaro.org
>>Cc: linux-arm-kernel@lists.infradead.org; mingyuan.ma@mediatek.com;
>>Guangming <Guangming.Cao@mediatek.com>;
>>wsd_upstream@mediatek.com; linux-kernel@vger.kernel.org; dri-
>>devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org;
>>yf.wang@mediatek.com; libo.kang@mediatek.com;
>>benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>>matthias.bgg@gmail.com; linux-mediatek@lists.infradead.org;
>>lmark@codeaurora.org; labbott@redhat.com; christian.koenig@amd.com;
>>jianjiao.zeng@mediatek.com; linux-media@vger.kernel.org
>>Subject: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>>
>>From: Guangming <Guangming.Cao@mediatek.com>
>>
>>Add a size check for allocation since the allocation size is
>>always less than the total DRAM size.
>>
>>Without this check, once the invalid size allocation runs on a process that
>>can't be killed by OOM flow(such as "gralloc" on Android devices), it will
>>cause a kernel exception, and to make matters worse, we can't find who are
>>using
>>so many memory with "dma_buf_debug_show" since the relevant dma-buf
>>hasn't exported.
>>
>>To make OOM issue easier, maybe need dma-buf framework to dump the
>>buffer size
>>under allocating in "dma_buf_debug_show".
>>
>>Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>>Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>>---
>>v3: 1. update patch, use right shift to replace division.
>>    2. update patch, add reason in code and commit message.
>>v2: 1. update size limitation as total_dram page size.
>>    2. update commit message
>>---
>> drivers/dma-buf/dma-heap.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>>diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>>index 56bf5ad01ad5..1fd382712584 100644
>>--- a/drivers/dma-buf/dma-heap.c
>>+++ b/drivers/dma-buf/dma-heap.c
>>@@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap
>>*heap, size_t len,
>> 	struct dma_buf *dmabuf;
>> 	int fd;
>>
>>+	/*
>>+	 * Invalid size check. The "len" should be less than totalram.
>>+	 *
>>+	 * Without this check, once the invalid size allocation runs on a process
>>that
>>+	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>>will
>>+	 * cause a kernel exception, and to make matters worse, we can't find
>>who are using
>>+	 * so many memory with "dma_buf_debug_show" since the relevant
>>dma-buf hasn't exported.
>>+	 */
>>+	if (len >> PAGE_SHIFT > totalram_pages())
>
>If your "heap" is from cma, is this still a valid check?

And thinking a bit further, if I create a heap from something else (say device memory),
you will need to be able to figure out the maximum allowable check for the specific
heap.

Maybe the heap needs a callback for max size?

m
>M
>
>>+		return -EINVAL;
>> 	/*
>> 	 * Allocations from all heaps have to begin
>> 	 * and end on page boundaries.
>>--
>>2.17.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Ruhl, Michael J]

>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>Ruhl, Michael J
>Sent: Thursday, January 13, 2022 7:58 AM
>To: guangming.cao@mediatek.com; sumit.semwal@linaro.org
>Cc: jianjiao.zeng@mediatek.com; lmark@codeaurora.org;
>wsd_upstream@mediatek.com; christian.koenig@amd.com; linux-
>kernel@vger.kernel.org; dri-devel@lists.freedesktop.org;
>yf.wang@mediatek.com; linaro-mm-sig@lists.linaro.org; linux-
>mediatek@lists.infradead.org; libo.kang@mediatek.com;
>benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>matthias.bgg@gmail.com; labbott@redhat.com;
>mingyuan.ma@mediatek.com; linux-arm-kernel@lists.infradead.org; linux-
>media@vger.kernel.org
>Subject: RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>
>
>>-----Original Message-----
>>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>guangming.cao@mediatek.com
>>Sent: Thursday, January 13, 2022 7:34 AM
>>To: sumit.semwal@linaro.org
>>Cc: linux-arm-kernel@lists.infradead.org; mingyuan.ma@mediatek.com;
>>Guangming <Guangming.Cao@mediatek.com>;
>>wsd_upstream@mediatek.com; linux-kernel@vger.kernel.org; dri-
>>devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org;
>>yf.wang@mediatek.com; libo.kang@mediatek.com;
>>benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>>matthias.bgg@gmail.com; linux-mediatek@lists.infradead.org;
>>lmark@codeaurora.org; labbott@redhat.com; christian.koenig@amd.com;
>>jianjiao.zeng@mediatek.com; linux-media@vger.kernel.org
>>Subject: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>>
>>From: Guangming <Guangming.Cao@mediatek.com>
>>
>>Add a size check for allocation since the allocation size is
>>always less than the total DRAM size.
>>
>>Without this check, once the invalid size allocation runs on a process that
>>can't be killed by OOM flow(such as "gralloc" on Android devices), it will
>>cause a kernel exception, and to make matters worse, we can't find who are
>>using
>>so many memory with "dma_buf_debug_show" since the relevant dma-buf
>>hasn't exported.
>>
>>To make OOM issue easier, maybe need dma-buf framework to dump the
>>buffer size
>>under allocating in "dma_buf_debug_show".
>>
>>Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>>Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>>---
>>v3: 1. update patch, use right shift to replace division.
>>    2. update patch, add reason in code and commit message.
>>v2: 1. update size limitation as total_dram page size.
>>    2. update commit message
>>---
>> drivers/dma-buf/dma-heap.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>>diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>>index 56bf5ad01ad5..1fd382712584 100644
>>--- a/drivers/dma-buf/dma-heap.c
>>+++ b/drivers/dma-buf/dma-heap.c
>>@@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap
>>*heap, size_t len,
>> 	struct dma_buf *dmabuf;
>> 	int fd;
>>
>>+	/*
>>+	 * Invalid size check. The "len" should be less than totalram.
>>+	 *
>>+	 * Without this check, once the invalid size allocation runs on a process
>>that
>>+	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>>will
>>+	 * cause a kernel exception, and to make matters worse, we can't find
>>who are using
>>+	 * so many memory with "dma_buf_debug_show" since the relevant
>>dma-buf hasn't exported.
>>+	 */
>>+	if (len >> PAGE_SHIFT > totalram_pages())
>
>If your "heap" is from cma, is this still a valid check?

And thinking a bit further, if I create a heap from something else (say device memory),
you will need to be able to figure out the maximum allowable check for the specific
heap.

Maybe the heap needs a callback for max size?

m
>M
>
>>+		return -EINVAL;
>> 	/*
>> 	 * Allocations from all heaps have to begin
>> 	 * and end on page boundaries.
>>--
>>2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Christian König]

Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
>> -----Original Message-----
>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>> Ruhl, Michael J
>> Sent: Thursday, January 13, 2022 7:58 AM
>> To: guangming.cao@mediatek.com; sumit.semwal@linaro.org
>> Cc: jianjiao.zeng@mediatek.com; lmark@codeaurora.org;
>> wsd_upstream@mediatek.com; christian.koenig@amd.com; linux-
>> kernel@vger.kernel.org; dri-devel@lists.freedesktop.org;
>> yf.wang@mediatek.com; linaro-mm-sig@lists.linaro.org; linux-
>> mediatek@lists.infradead.org; libo.kang@mediatek.com;
>> benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>> matthias.bgg@gmail.com; labbott@redhat.com;
>> mingyuan.ma@mediatek.com; linux-arm-kernel@lists.infradead.org; linux-
>> media@vger.kernel.org
>> Subject: RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>>
>>
>>> -----Original Message-----
>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>> guangming.cao@mediatek.com
>>> Sent: Thursday, January 13, 2022 7:34 AM
>>> To: sumit.semwal@linaro.org
>>> Cc: linux-arm-kernel@lists.infradead.org; mingyuan.ma@mediatek.com;
>>> Guangming <Guangming.Cao@mediatek.com>;
>>> wsd_upstream@mediatek.com; linux-kernel@vger.kernel.org; dri-
>>> devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org;
>>> yf.wang@mediatek.com; libo.kang@mediatek.com;
>>> benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>>> matthias.bgg@gmail.com; linux-mediatek@lists.infradead.org;
>>> lmark@codeaurora.org; labbott@redhat.com; christian.koenig@amd.com;
>>> jianjiao.zeng@mediatek.com; linux-media@vger.kernel.org
>>> Subject: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>>>
>>> From: Guangming <Guangming.Cao@mediatek.com>
>>>
>>> Add a size check for allocation since the allocation size is
>>> always less than the total DRAM size.
>>>
>>> Without this check, once the invalid size allocation runs on a process that
>>> can't be killed by OOM flow(such as "gralloc" on Android devices), it will
>>> cause a kernel exception, and to make matters worse, we can't find who are
>>> using
>>> so many memory with "dma_buf_debug_show" since the relevant dma-buf
>>> hasn't exported.
>>>
>>> To make OOM issue easier, maybe need dma-buf framework to dump the
>>> buffer size
>>> under allocating in "dma_buf_debug_show".
>>>
>>> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>>> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>>> ---
>>> v3: 1. update patch, use right shift to replace division.
>>>     2. update patch, add reason in code and commit message.
>>> v2: 1. update size limitation as total_dram page size.
>>>     2. update commit message
>>> ---
>>> drivers/dma-buf/dma-heap.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>>> index 56bf5ad01ad5..1fd382712584 100644
>>> --- a/drivers/dma-buf/dma-heap.c
>>> +++ b/drivers/dma-buf/dma-heap.c
>>> @@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap
>>> *heap, size_t len,
>>> 	struct dma_buf *dmabuf;
>>> 	int fd;
>>>
>>> +	/*
>>> +	 * Invalid size check. The "len" should be less than totalram.
>>> +	 *
>>> +	 * Without this check, once the invalid size allocation runs on a process
>>> that
>>> +	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>>> will
>>> +	 * cause a kernel exception, and to make matters worse, we can't find
>>> who are using
>>> +	 * so many memory with "dma_buf_debug_show" since the relevant
>>> dma-buf hasn't exported.
>>> +	 */
>>> +	if (len >> PAGE_SHIFT > totalram_pages())
>> If your "heap" is from cma, is this still a valid check?
> And thinking a bit further, if I create a heap from something else (say device memory),
> you will need to be able to figure out the maximum allowable check for the specific
> heap.
>
> Maybe the heap needs a callback for max size?

Well we currently maintain a separate allocator and don't use dma-heap, 
but yes we have systems with 16GiB device and only 8GiB system memory so 
that check here is certainly not correct.

In general I would rather let the system run into -ENOMEM or -EINVAL 
from the allocator instead.

Regards,
Christian.

>
> m
>> M
>>
>>> +		return -EINVAL;
>>> 	/*
>>> 	 * Allocations from all heaps have to begin
>>> 	 * and end on page boundaries.
>>> --
>>> 2.17.1

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Christian König]

Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
>> -----Original Message-----
>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>> Ruhl, Michael J
>> Sent: Thursday, January 13, 2022 7:58 AM
>> To: guangming.cao@mediatek.com; sumit.semwal@linaro.org
>> Cc: jianjiao.zeng@mediatek.com; lmark@codeaurora.org;
>> wsd_upstream@mediatek.com; christian.koenig@amd.com; linux-
>> kernel@vger.kernel.org; dri-devel@lists.freedesktop.org;
>> yf.wang@mediatek.com; linaro-mm-sig@lists.linaro.org; linux-
>> mediatek@lists.infradead.org; libo.kang@mediatek.com;
>> benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>> matthias.bgg@gmail.com; labbott@redhat.com;
>> mingyuan.ma@mediatek.com; linux-arm-kernel@lists.infradead.org; linux-
>> media@vger.kernel.org
>> Subject: RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>>
>>
>>> -----Original Message-----
>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>> guangming.cao@mediatek.com
>>> Sent: Thursday, January 13, 2022 7:34 AM
>>> To: sumit.semwal@linaro.org
>>> Cc: linux-arm-kernel@lists.infradead.org; mingyuan.ma@mediatek.com;
>>> Guangming <Guangming.Cao@mediatek.com>;
>>> wsd_upstream@mediatek.com; linux-kernel@vger.kernel.org; dri-
>>> devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org;
>>> yf.wang@mediatek.com; libo.kang@mediatek.com;
>>> benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>>> matthias.bgg@gmail.com; linux-mediatek@lists.infradead.org;
>>> lmark@codeaurora.org; labbott@redhat.com; christian.koenig@amd.com;
>>> jianjiao.zeng@mediatek.com; linux-media@vger.kernel.org
>>> Subject: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>>>
>>> From: Guangming <Guangming.Cao@mediatek.com>
>>>
>>> Add a size check for allocation since the allocation size is
>>> always less than the total DRAM size.
>>>
>>> Without this check, once the invalid size allocation runs on a process that
>>> can't be killed by OOM flow(such as "gralloc" on Android devices), it will
>>> cause a kernel exception, and to make matters worse, we can't find who are
>>> using
>>> so many memory with "dma_buf_debug_show" since the relevant dma-buf
>>> hasn't exported.
>>>
>>> To make OOM issue easier, maybe need dma-buf framework to dump the
>>> buffer size
>>> under allocating in "dma_buf_debug_show".
>>>
>>> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>>> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>>> ---
>>> v3: 1. update patch, use right shift to replace division.
>>>     2. update patch, add reason in code and commit message.
>>> v2: 1. update size limitation as total_dram page size.
>>>     2. update commit message
>>> ---
>>> drivers/dma-buf/dma-heap.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>>> index 56bf5ad01ad5..1fd382712584 100644
>>> --- a/drivers/dma-buf/dma-heap.c
>>> +++ b/drivers/dma-buf/dma-heap.c
>>> @@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap
>>> *heap, size_t len,
>>> 	struct dma_buf *dmabuf;
>>> 	int fd;
>>>
>>> +	/*
>>> +	 * Invalid size check. The "len" should be less than totalram.
>>> +	 *
>>> +	 * Without this check, once the invalid size allocation runs on a process
>>> that
>>> +	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>>> will
>>> +	 * cause a kernel exception, and to make matters worse, we can't find
>>> who are using
>>> +	 * so many memory with "dma_buf_debug_show" since the relevant
>>> dma-buf hasn't exported.
>>> +	 */
>>> +	if (len >> PAGE_SHIFT > totalram_pages())
>> If your "heap" is from cma, is this still a valid check?
> And thinking a bit further, if I create a heap from something else (say device memory),
> you will need to be able to figure out the maximum allowable check for the specific
> heap.
>
> Maybe the heap needs a callback for max size?

Well we currently maintain a separate allocator and don't use dma-heap, 
but yes we have systems with 16GiB device and only 8GiB system memory so 
that check here is certainly not correct.

In general I would rather let the system run into -ENOMEM or -EINVAL 
from the allocator instead.

Regards,
Christian.

>
> m
>> M
>>
>>> +		return -EINVAL;
>>> 	/*
>>> 	 * Allocations from all heaps have to begin
>>> 	 * and end on page boundaries.
>>> --
>>> 2.17.1

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Christian König]

Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
>> -----Original Message-----
>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>> Ruhl, Michael J
>> Sent: Thursday, January 13, 2022 7:58 AM
>> To: guangming.cao@mediatek.com; sumit.semwal@linaro.org
>> Cc: jianjiao.zeng@mediatek.com; lmark@codeaurora.org;
>> wsd_upstream@mediatek.com; christian.koenig@amd.com; linux-
>> kernel@vger.kernel.org; dri-devel@lists.freedesktop.org;
>> yf.wang@mediatek.com; linaro-mm-sig@lists.linaro.org; linux-
>> mediatek@lists.infradead.org; libo.kang@mediatek.com;
>> benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>> matthias.bgg@gmail.com; labbott@redhat.com;
>> mingyuan.ma@mediatek.com; linux-arm-kernel@lists.infradead.org; linux-
>> media@vger.kernel.org
>> Subject: RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>>
>>
>>> -----Original Message-----
>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>> guangming.cao@mediatek.com
>>> Sent: Thursday, January 13, 2022 7:34 AM
>>> To: sumit.semwal@linaro.org
>>> Cc: linux-arm-kernel@lists.infradead.org; mingyuan.ma@mediatek.com;
>>> Guangming <Guangming.Cao@mediatek.com>;
>>> wsd_upstream@mediatek.com; linux-kernel@vger.kernel.org; dri-
>>> devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org;
>>> yf.wang@mediatek.com; libo.kang@mediatek.com;
>>> benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>>> matthias.bgg@gmail.com; linux-mediatek@lists.infradead.org;
>>> lmark@codeaurora.org; labbott@redhat.com; christian.koenig@amd.com;
>>> jianjiao.zeng@mediatek.com; linux-media@vger.kernel.org
>>> Subject: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>>>
>>> From: Guangming <Guangming.Cao@mediatek.com>
>>>
>>> Add a size check for allocation since the allocation size is
>>> always less than the total DRAM size.
>>>
>>> Without this check, once the invalid size allocation runs on a process that
>>> can't be killed by OOM flow(such as "gralloc" on Android devices), it will
>>> cause a kernel exception, and to make matters worse, we can't find who are
>>> using
>>> so many memory with "dma_buf_debug_show" since the relevant dma-buf
>>> hasn't exported.
>>>
>>> To make OOM issue easier, maybe need dma-buf framework to dump the
>>> buffer size
>>> under allocating in "dma_buf_debug_show".
>>>
>>> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>>> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>>> ---
>>> v3: 1. update patch, use right shift to replace division.
>>>     2. update patch, add reason in code and commit message.
>>> v2: 1. update size limitation as total_dram page size.
>>>     2. update commit message
>>> ---
>>> drivers/dma-buf/dma-heap.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>>> index 56bf5ad01ad5..1fd382712584 100644
>>> --- a/drivers/dma-buf/dma-heap.c
>>> +++ b/drivers/dma-buf/dma-heap.c
>>> @@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap
>>> *heap, size_t len,
>>> 	struct dma_buf *dmabuf;
>>> 	int fd;
>>>
>>> +	/*
>>> +	 * Invalid size check. The "len" should be less than totalram.
>>> +	 *
>>> +	 * Without this check, once the invalid size allocation runs on a process
>>> that
>>> +	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>>> will
>>> +	 * cause a kernel exception, and to make matters worse, we can't find
>>> who are using
>>> +	 * so many memory with "dma_buf_debug_show" since the relevant
>>> dma-buf hasn't exported.
>>> +	 */
>>> +	if (len >> PAGE_SHIFT > totalram_pages())
>> If your "heap" is from cma, is this still a valid check?
> And thinking a bit further, if I create a heap from something else (say device memory),
> you will need to be able to figure out the maximum allowable check for the specific
> heap.
>
> Maybe the heap needs a callback for max size?

Well we currently maintain a separate allocator and don't use dma-heap, 
but yes we have systems with 16GiB device and only 8GiB system memory so 
that check here is certainly not correct.

In general I would rather let the system run into -ENOMEM or -EINVAL 
from the allocator instead.

Regards,
Christian.

>
> m
>> M
>>
>>> +		return -EINVAL;
>>> 	/*
>>> 	 * Allocations from all heaps have to begin
>>> 	 * and end on page boundaries.
>>> --
>>> 2.17.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Christian König]

Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
>> -----Original Message-----
>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>> Ruhl, Michael J
>> Sent: Thursday, January 13, 2022 7:58 AM
>> To: guangming.cao@mediatek.com; sumit.semwal@linaro.org
>> Cc: jianjiao.zeng@mediatek.com; lmark@codeaurora.org;
>> wsd_upstream@mediatek.com; christian.koenig@amd.com; linux-
>> kernel@vger.kernel.org; dri-devel@lists.freedesktop.org;
>> yf.wang@mediatek.com; linaro-mm-sig@lists.linaro.org; linux-
>> mediatek@lists.infradead.org; libo.kang@mediatek.com;
>> benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>> matthias.bgg@gmail.com; labbott@redhat.com;
>> mingyuan.ma@mediatek.com; linux-arm-kernel@lists.infradead.org; linux-
>> media@vger.kernel.org
>> Subject: RE: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>>
>>
>>> -----Original Message-----
>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>> guangming.cao@mediatek.com
>>> Sent: Thursday, January 13, 2022 7:34 AM
>>> To: sumit.semwal@linaro.org
>>> Cc: linux-arm-kernel@lists.infradead.org; mingyuan.ma@mediatek.com;
>>> Guangming <Guangming.Cao@mediatek.com>;
>>> wsd_upstream@mediatek.com; linux-kernel@vger.kernel.org; dri-
>>> devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org;
>>> yf.wang@mediatek.com; libo.kang@mediatek.com;
>>> benjamin.gaignard@linaro.org; bo.song@mediatek.com;
>>> matthias.bgg@gmail.com; linux-mediatek@lists.infradead.org;
>>> lmark@codeaurora.org; labbott@redhat.com; christian.koenig@amd.com;
>>> jianjiao.zeng@mediatek.com; linux-media@vger.kernel.org
>>> Subject: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation
>>>
>>> From: Guangming <Guangming.Cao@mediatek.com>
>>>
>>> Add a size check for allocation since the allocation size is
>>> always less than the total DRAM size.
>>>
>>> Without this check, once the invalid size allocation runs on a process that
>>> can't be killed by OOM flow(such as "gralloc" on Android devices), it will
>>> cause a kernel exception, and to make matters worse, we can't find who are
>>> using
>>> so many memory with "dma_buf_debug_show" since the relevant dma-buf
>>> hasn't exported.
>>>
>>> To make OOM issue easier, maybe need dma-buf framework to dump the
>>> buffer size
>>> under allocating in "dma_buf_debug_show".
>>>
>>> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
>>> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
>>> ---
>>> v3: 1. update patch, use right shift to replace division.
>>>     2. update patch, add reason in code and commit message.
>>> v2: 1. update size limitation as total_dram page size.
>>>     2. update commit message
>>> ---
>>> drivers/dma-buf/dma-heap.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
>>> index 56bf5ad01ad5..1fd382712584 100644
>>> --- a/drivers/dma-buf/dma-heap.c
>>> +++ b/drivers/dma-buf/dma-heap.c
>>> @@ -55,6 +55,16 @@ static int dma_heap_buffer_alloc(struct dma_heap
>>> *heap, size_t len,
>>> 	struct dma_buf *dmabuf;
>>> 	int fd;
>>>
>>> +	/*
>>> +	 * Invalid size check. The "len" should be less than totalram.
>>> +	 *
>>> +	 * Without this check, once the invalid size allocation runs on a process
>>> that
>>> +	 * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>>> will
>>> +	 * cause a kernel exception, and to make matters worse, we can't find
>>> who are using
>>> +	 * so many memory with "dma_buf_debug_show" since the relevant
>>> dma-buf hasn't exported.
>>> +	 */
>>> +	if (len >> PAGE_SHIFT > totalram_pages())
>> If your "heap" is from cma, is this still a valid check?
> And thinking a bit further, if I create a heap from something else (say device memory),
> you will need to be able to figure out the maximum allowable check for the specific
> heap.
>
> Maybe the heap needs a callback for max size?

Well we currently maintain a separate allocator and don't use dma-heap, 
but yes we have systems with 16GiB device and only 8GiB system memory so 
that check here is certainly not correct.

In general I would rather let the system run into -ENOMEM or -EINVAL 
from the allocator instead.

Regards,
Christian.

>
> m
>> M
>>
>>> +		return -EINVAL;
>>> 	/*
>>> 	 * Allocations from all heaps have to begin
>>> 	 * and end on page boundaries.
>>> --
>>> 2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Thu, Jan 13, 2022 at 5:05 AM Christian König
<christian.koenig@amd.com> wrote:
> Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> >> -----Original Message-----
> >> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> >> Ruhl, Michael J
> >>> -----Original Message-----
> >>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> >>> guangming.cao@mediatek.com
> >>> +   /*
> >>> +    * Invalid size check. The "len" should be less than totalram.
> >>> +    *
> >>> +    * Without this check, once the invalid size allocation runs on a process
> >>> that
> >>> +    * can't be killed by OOM flow(such as "gralloc" on Android devices), it
> >>> will
> >>> +    * cause a kernel exception, and to make matters worse, we can't find
> >>> who are using
> >>> +    * so many memory with "dma_buf_debug_show" since the relevant
> >>> dma-buf hasn't exported.
> >>> +    */
> >>> +   if (len >> PAGE_SHIFT > totalram_pages())
> >> If your "heap" is from cma, is this still a valid check?
> > And thinking a bit further, if I create a heap from something else (say device memory),
> > you will need to be able to figure out the maximum allowable check for the specific
> > heap.
> >
> > Maybe the heap needs a callback for max size?
>
> Well we currently maintain a separate allocator and don't use dma-heap,
> but yes we have systems with 16GiB device and only 8GiB system memory so
> that check here is certainly not correct.

Good point.

> In general I would rather let the system run into -ENOMEM or -EINVAL
> from the allocator instead.

Probably the simpler solution is to push the allocation check to the
heap driver, rather than doing it at the top level here.

For CMA or other contiguous heaps, letting the allocator fail is fast
enough. For noncontiguous buffers, like the system heap, the
allocation can burn a lot of time and consume a lot of memory (causing
other trouble) before a large allocation might naturally fail.

thanks
-john

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Thu, Jan 13, 2022 at 5:05 AM Christian König
<christian.koenig@amd.com> wrote:
> Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> >> -----Original Message-----
> >> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> >> Ruhl, Michael J
> >>> -----Original Message-----
> >>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> >>> guangming.cao@mediatek.com
> >>> +   /*
> >>> +    * Invalid size check. The "len" should be less than totalram.
> >>> +    *
> >>> +    * Without this check, once the invalid size allocation runs on a process
> >>> that
> >>> +    * can't be killed by OOM flow(such as "gralloc" on Android devices), it
> >>> will
> >>> +    * cause a kernel exception, and to make matters worse, we can't find
> >>> who are using
> >>> +    * so many memory with "dma_buf_debug_show" since the relevant
> >>> dma-buf hasn't exported.
> >>> +    */
> >>> +   if (len >> PAGE_SHIFT > totalram_pages())
> >> If your "heap" is from cma, is this still a valid check?
> > And thinking a bit further, if I create a heap from something else (say device memory),
> > you will need to be able to figure out the maximum allowable check for the specific
> > heap.
> >
> > Maybe the heap needs a callback for max size?
>
> Well we currently maintain a separate allocator and don't use dma-heap,
> but yes we have systems with 16GiB device and only 8GiB system memory so
> that check here is certainly not correct.

Good point.

> In general I would rather let the system run into -ENOMEM or -EINVAL
> from the allocator instead.

Probably the simpler solution is to push the allocation check to the
heap driver, rather than doing it at the top level here.

For CMA or other contiguous heaps, letting the allocator fail is fast
enough. For noncontiguous buffers, like the system heap, the
allocation can burn a lot of time and consume a lot of memory (causing
other trouble) before a large allocation might naturally fail.

thanks
-john

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Thu, Jan 13, 2022 at 5:05 AM Christian König
<christian.koenig@amd.com> wrote:
> Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> >> -----Original Message-----
> >> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> >> Ruhl, Michael J
> >>> -----Original Message-----
> >>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> >>> guangming.cao@mediatek.com
> >>> +   /*
> >>> +    * Invalid size check. The "len" should be less than totalram.
> >>> +    *
> >>> +    * Without this check, once the invalid size allocation runs on a process
> >>> that
> >>> +    * can't be killed by OOM flow(such as "gralloc" on Android devices), it
> >>> will
> >>> +    * cause a kernel exception, and to make matters worse, we can't find
> >>> who are using
> >>> +    * so many memory with "dma_buf_debug_show" since the relevant
> >>> dma-buf hasn't exported.
> >>> +    */
> >>> +   if (len >> PAGE_SHIFT > totalram_pages())
> >> If your "heap" is from cma, is this still a valid check?
> > And thinking a bit further, if I create a heap from something else (say device memory),
> > you will need to be able to figure out the maximum allowable check for the specific
> > heap.
> >
> > Maybe the heap needs a callback for max size?
>
> Well we currently maintain a separate allocator and don't use dma-heap,
> but yes we have systems with 16GiB device and only 8GiB system memory so
> that check here is certainly not correct.

Good point.

> In general I would rather let the system run into -ENOMEM or -EINVAL
> from the allocator instead.

Probably the simpler solution is to push the allocation check to the
heap driver, rather than doing it at the top level here.

For CMA or other contiguous heaps, letting the allocator fail is fast
enough. For noncontiguous buffers, like the system heap, the
allocation can burn a lot of time and consume a lot of memory (causing
other trouble) before a large allocation might naturally fail.

thanks
-john

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Thu, Jan 13, 2022 at 5:05 AM Christian König
<christian.koenig@amd.com> wrote:
> Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> >> -----Original Message-----
> >> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> >> Ruhl, Michael J
> >>> -----Original Message-----
> >>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> >>> guangming.cao@mediatek.com
> >>> +   /*
> >>> +    * Invalid size check. The "len" should be less than totalram.
> >>> +    *
> >>> +    * Without this check, once the invalid size allocation runs on a process
> >>> that
> >>> +    * can't be killed by OOM flow(such as "gralloc" on Android devices), it
> >>> will
> >>> +    * cause a kernel exception, and to make matters worse, we can't find
> >>> who are using
> >>> +    * so many memory with "dma_buf_debug_show" since the relevant
> >>> dma-buf hasn't exported.
> >>> +    */
> >>> +   if (len >> PAGE_SHIFT > totalram_pages())
> >> If your "heap" is from cma, is this still a valid check?
> > And thinking a bit further, if I create a heap from something else (say device memory),
> > you will need to be able to figure out the maximum allowable check for the specific
> > heap.
> >
> > Maybe the heap needs a callback for max size?
>
> Well we currently maintain a separate allocator and don't use dma-heap,
> but yes we have systems with 16GiB device and only 8GiB system memory so
> that check here is certainly not correct.

Good point.

> In general I would rather let the system run into -ENOMEM or -EINVAL
> from the allocator instead.

Probably the simpler solution is to push the allocation check to the
heap driver, rather than doing it at the top level here.

For CMA or other contiguous heaps, letting the allocator fail is fast
enough. For noncontiguous buffers, like the system heap, the
allocation can burn a lot of time and consume a lot of memory (causing
other trouble) before a large allocation might naturally fail.

thanks
-john

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Christian König]

Am 14.01.22 um 00:26 schrieb John Stultz:
> On Thu, Jan 13, 2022 at 5:05 AM Christian König
> <christian.koenig@amd.com> wrote:
>> Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
>>>> -----Original Message-----
>>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>>> Ruhl, Michael J
>>>>> -----Original Message-----
>>>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>>>> guangming.cao@mediatek.com
>>>>> +   /*
>>>>> +    * Invalid size check. The "len" should be less than totalram.
>>>>> +    *
>>>>> +    * Without this check, once the invalid size allocation runs on a process
>>>>> that
>>>>> +    * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>>>>> will
>>>>> +    * cause a kernel exception, and to make matters worse, we can't find
>>>>> who are using
>>>>> +    * so many memory with "dma_buf_debug_show" since the relevant
>>>>> dma-buf hasn't exported.
>>>>> +    */
>>>>> +   if (len >> PAGE_SHIFT > totalram_pages())
>>>> If your "heap" is from cma, is this still a valid check?
>>> And thinking a bit further, if I create a heap from something else (say device memory),
>>> you will need to be able to figure out the maximum allowable check for the specific
>>> heap.
>>>
>>> Maybe the heap needs a callback for max size?
>> Well we currently maintain a separate allocator and don't use dma-heap,
>> but yes we have systems with 16GiB device and only 8GiB system memory so
>> that check here is certainly not correct.
> Good point.
>
>> In general I would rather let the system run into -ENOMEM or -EINVAL
>> from the allocator instead.
> Probably the simpler solution is to push the allocation check to the
> heap driver, rather than doing it at the top level here.
>
> For CMA or other contiguous heaps, letting the allocator fail is fast
> enough. For noncontiguous buffers, like the system heap, the
> allocation can burn a lot of time and consume a lot of memory (causing
> other trouble) before a large allocation might naturally fail.

Yeah, letting a alloc_page() loop run for a while is usually not nice at 
all :)

You can still do a sanity check here, e.g. the size should never have 
the most significant bit set for example.

Regards,
Christian.

>
> thanks
> -john

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Christian König]

Am 14.01.22 um 00:26 schrieb John Stultz:
> On Thu, Jan 13, 2022 at 5:05 AM Christian König
> <christian.koenig@amd.com> wrote:
>> Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
>>>> -----Original Message-----
>>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>>> Ruhl, Michael J
>>>>> -----Original Message-----
>>>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>>>> guangming.cao@mediatek.com
>>>>> +   /*
>>>>> +    * Invalid size check. The "len" should be less than totalram.
>>>>> +    *
>>>>> +    * Without this check, once the invalid size allocation runs on a process
>>>>> that
>>>>> +    * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>>>>> will
>>>>> +    * cause a kernel exception, and to make matters worse, we can't find
>>>>> who are using
>>>>> +    * so many memory with "dma_buf_debug_show" since the relevant
>>>>> dma-buf hasn't exported.
>>>>> +    */
>>>>> +   if (len >> PAGE_SHIFT > totalram_pages())
>>>> If your "heap" is from cma, is this still a valid check?
>>> And thinking a bit further, if I create a heap from something else (say device memory),
>>> you will need to be able to figure out the maximum allowable check for the specific
>>> heap.
>>>
>>> Maybe the heap needs a callback for max size?
>> Well we currently maintain a separate allocator and don't use dma-heap,
>> but yes we have systems with 16GiB device and only 8GiB system memory so
>> that check here is certainly not correct.
> Good point.
>
>> In general I would rather let the system run into -ENOMEM or -EINVAL
>> from the allocator instead.
> Probably the simpler solution is to push the allocation check to the
> heap driver, rather than doing it at the top level here.
>
> For CMA or other contiguous heaps, letting the allocator fail is fast
> enough. For noncontiguous buffers, like the system heap, the
> allocation can burn a lot of time and consume a lot of memory (causing
> other trouble) before a large allocation might naturally fail.

Yeah, letting a alloc_page() loop run for a while is usually not nice at 
all :)

You can still do a sanity check here, e.g. the size should never have 
the most significant bit set for example.

Regards,
Christian.

>
> thanks
> -john


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Christian König]

Am 14.01.22 um 00:26 schrieb John Stultz:
> On Thu, Jan 13, 2022 at 5:05 AM Christian König
> <christian.koenig@amd.com> wrote:
>> Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
>>>> -----Original Message-----
>>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>>> Ruhl, Michael J
>>>>> -----Original Message-----
>>>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>>>> guangming.cao@mediatek.com
>>>>> +   /*
>>>>> +    * Invalid size check. The "len" should be less than totalram.
>>>>> +    *
>>>>> +    * Without this check, once the invalid size allocation runs on a process
>>>>> that
>>>>> +    * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>>>>> will
>>>>> +    * cause a kernel exception, and to make matters worse, we can't find
>>>>> who are using
>>>>> +    * so many memory with "dma_buf_debug_show" since the relevant
>>>>> dma-buf hasn't exported.
>>>>> +    */
>>>>> +   if (len >> PAGE_SHIFT > totalram_pages())
>>>> If your "heap" is from cma, is this still a valid check?
>>> And thinking a bit further, if I create a heap from something else (say device memory),
>>> you will need to be able to figure out the maximum allowable check for the specific
>>> heap.
>>>
>>> Maybe the heap needs a callback for max size?
>> Well we currently maintain a separate allocator and don't use dma-heap,
>> but yes we have systems with 16GiB device and only 8GiB system memory so
>> that check here is certainly not correct.
> Good point.
>
>> In general I would rather let the system run into -ENOMEM or -EINVAL
>> from the allocator instead.
> Probably the simpler solution is to push the allocation check to the
> heap driver, rather than doing it at the top level here.
>
> For CMA or other contiguous heaps, letting the allocator fail is fast
> enough. For noncontiguous buffers, like the system heap, the
> allocation can burn a lot of time and consume a lot of memory (causing
> other trouble) before a large allocation might naturally fail.

Yeah, letting a alloc_page() loop run for a while is usually not nice at 
all :)

You can still do a sanity check here, e.g. the size should never have 
the most significant bit set for example.

Regards,
Christian.

>
> thanks
> -john

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Christian König]

Am 14.01.22 um 00:26 schrieb John Stultz:
> On Thu, Jan 13, 2022 at 5:05 AM Christian König
> <christian.koenig@amd.com> wrote:
>> Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
>>>> -----Original Message-----
>>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>>> Ruhl, Michael J
>>>>> -----Original Message-----
>>>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>>>> guangming.cao@mediatek.com
>>>>> +   /*
>>>>> +    * Invalid size check. The "len" should be less than totalram.
>>>>> +    *
>>>>> +    * Without this check, once the invalid size allocation runs on a process
>>>>> that
>>>>> +    * can't be killed by OOM flow(such as "gralloc" on Android devices), it
>>>>> will
>>>>> +    * cause a kernel exception, and to make matters worse, we can't find
>>>>> who are using
>>>>> +    * so many memory with "dma_buf_debug_show" since the relevant
>>>>> dma-buf hasn't exported.
>>>>> +    */
>>>>> +   if (len >> PAGE_SHIFT > totalram_pages())
>>>> If your "heap" is from cma, is this still a valid check?
>>> And thinking a bit further, if I create a heap from something else (say device memory),
>>> you will need to be able to figure out the maximum allowable check for the specific
>>> heap.
>>>
>>> Maybe the heap needs a callback for max size?
>> Well we currently maintain a separate allocator and don't use dma-heap,
>> but yes we have systems with 16GiB device and only 8GiB system memory so
>> that check here is certainly not correct.
> Good point.
>
>> In general I would rather let the system run into -ENOMEM or -EINVAL
>> from the allocator instead.
> Probably the simpler solution is to push the allocation check to the
> heap driver, rather than doing it at the top level here.
>
> For CMA or other contiguous heaps, letting the allocator fail is fast
> enough. For noncontiguous buffers, like the system heap, the
> allocation can burn a lot of time and consume a lot of memory (causing
> other trouble) before a large allocation might naturally fail.

Yeah, letting a alloc_page() loop run for a while is usually not nice at 
all :)

You can still do a sanity check here, e.g. the size should never have 
the most significant bit set for example.

Regards,
Christian.

>
> thanks
> -john


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Guangming.Cao]

On Fri, 2022-01-14 at 08:16 +0100, Christian König wrote:
> Am 14.01.22 um 00:26 schrieb John Stultz:
> > On Thu, Jan 13, 2022 at 5:05 AM Christian König
> > <christian.koenig@amd.com> wrote:
> > > Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> > > > > -----Original Message-----
> > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On
> > > > > Behalf Of
> > > > > Ruhl, Michael J
> > > > > > -----Original Message-----
> > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org>
> > > > > > On Behalf Of
> > > > > > guangming.cao@mediatek.com
> > > > > > +   /*
> > > > > > +    * Invalid size check. The "len" should be less than
> > > > > > totalram.
> > > > > > +    *
> > > > > > +    * Without this check, once the invalid size allocation
> > > > > > runs on a process
> > > > > > that
> > > > > > +    * can't be killed by OOM flow(such as "gralloc" on
> > > > > > Android devices), it
> > > > > > will
> > > > > > +    * cause a kernel exception, and to make matters worse,
> > > > > > we can't find
> > > > > > who are using
> > > > > > +    * so many memory with "dma_buf_debug_show" since the
> > > > > > relevant
> > > > > > dma-buf hasn't exported.
> > > > > > +    */
> > > > > > +   if (len >> PAGE_SHIFT > totalram_pages())
> > > > > 
> > > > > If your "heap" is from cma, is this still a valid check?
> > > > 
> > > > And thinking a bit further, if I create a heap from something
> > > > else (say device memory),
> > > > you will need to be able to figure out the maximum allowable
> > > > check for the specific
> > > > heap.
> > > > 
> > > > Maybe the heap needs a callback for max size?
Yes, I agree with this solution.
If dma-heap framework support this via adding a callback to support it,
seems it's more clear than adding a limitation in dma-heap framework
since each heap maybe has different limitation.
If you prefer adding callback, I can update this patch and add totalram
limitation to system dma-heap.

Thanks!
Guangming
> > > 
> > > Well we currently maintain a separate allocator and don't use
> > > dma-heap,
> > > but yes we have systems with 16GiB device and only 8GiB system
> > > memory so
> > > that check here is certainly not correct.
> > 
> > Good point.
> > 
> > > In general I would rather let the system run into -ENOMEM or
> > > -EINVAL
> > > from the allocator instead.

For system dma-heap, it doesn't know how memory is avaliable when
allocating memory, so, use totalram_pages() just to prevent cases which
will cause oom definitely.

Just like PAGE align, this check is can be used for all heaps since
there is no dma-heap can alloc memory larger than totalram. Futhermore,
if vendors implement a variety of dma-heap like system heap for special
usages, seems need to add this check to each dma-heap, and I think this
is unnecessary.
If the dma-heap has it's own special limitations for size, and add it
into heap implementation is good.

Thanks!
Guangming
> > 
> > Probably the simpler solution is to push the allocation check to
> > the
> > heap driver, rather than doing it at the top level here.
> > 
> > For CMA or other contiguous heaps, letting the allocator fail is
> > fast
> > enough. For noncontiguous buffers, like the system heap, the
> > allocation can burn a lot of time and consume a lot of memory
> > (causing
> > other trouble) before a large allocation might naturally fail.
> 
> Yeah, letting a alloc_page() loop run for a while is usually not nice
> at 
> all :)
> 
> You can still do a sanity check here, e.g. the size should never
> have 
> the most significant bit set for example.
> 
Yes, this is a good solution. But if this a positive value, larger than
totalram, it can also pass this check, and cause OOM after some time.

From dicussion above, seems finding a proper solution that can judge
the size is valid or not for each dma-heap is more important.

Thanks!
Guangming

> Regards,
> Christian.
> 
> > 
> > thanks
> > -john
> 
> 

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Guangming.Cao]

On Fri, 2022-01-14 at 08:16 +0100, Christian König wrote:
> Am 14.01.22 um 00:26 schrieb John Stultz:
> > On Thu, Jan 13, 2022 at 5:05 AM Christian König
> > <christian.koenig@amd.com> wrote:
> > > Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> > > > > -----Original Message-----
> > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On
> > > > > Behalf Of
> > > > > Ruhl, Michael J
> > > > > > -----Original Message-----
> > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org>
> > > > > > On Behalf Of
> > > > > > guangming.cao@mediatek.com
> > > > > > +   /*
> > > > > > +    * Invalid size check. The "len" should be less than
> > > > > > totalram.
> > > > > > +    *
> > > > > > +    * Without this check, once the invalid size allocation
> > > > > > runs on a process
> > > > > > that
> > > > > > +    * can't be killed by OOM flow(such as "gralloc" on
> > > > > > Android devices), it
> > > > > > will
> > > > > > +    * cause a kernel exception, and to make matters worse,
> > > > > > we can't find
> > > > > > who are using
> > > > > > +    * so many memory with "dma_buf_debug_show" since the
> > > > > > relevant
> > > > > > dma-buf hasn't exported.
> > > > > > +    */
> > > > > > +   if (len >> PAGE_SHIFT > totalram_pages())
> > > > > 
> > > > > If your "heap" is from cma, is this still a valid check?
> > > > 
> > > > And thinking a bit further, if I create a heap from something
> > > > else (say device memory),
> > > > you will need to be able to figure out the maximum allowable
> > > > check for the specific
> > > > heap.
> > > > 
> > > > Maybe the heap needs a callback for max size?
Yes, I agree with this solution.
If dma-heap framework support this via adding a callback to support it,
seems it's more clear than adding a limitation in dma-heap framework
since each heap maybe has different limitation.
If you prefer adding callback, I can update this patch and add totalram
limitation to system dma-heap.

Thanks!
Guangming
> > > 
> > > Well we currently maintain a separate allocator and don't use
> > > dma-heap,
> > > but yes we have systems with 16GiB device and only 8GiB system
> > > memory so
> > > that check here is certainly not correct.
> > 
> > Good point.
> > 
> > > In general I would rather let the system run into -ENOMEM or
> > > -EINVAL
> > > from the allocator instead.

For system dma-heap, it doesn't know how memory is avaliable when
allocating memory, so, use totalram_pages() just to prevent cases which
will cause oom definitely.

Just like PAGE align, this check is can be used for all heaps since
there is no dma-heap can alloc memory larger than totalram. Futhermore,
if vendors implement a variety of dma-heap like system heap for special
usages, seems need to add this check to each dma-heap, and I think this
is unnecessary.
If the dma-heap has it's own special limitations for size, and add it
into heap implementation is good.

Thanks!
Guangming
> > 
> > Probably the simpler solution is to push the allocation check to
> > the
> > heap driver, rather than doing it at the top level here.
> > 
> > For CMA or other contiguous heaps, letting the allocator fail is
> > fast
> > enough. For noncontiguous buffers, like the system heap, the
> > allocation can burn a lot of time and consume a lot of memory
> > (causing
> > other trouble) before a large allocation might naturally fail.
> 
> Yeah, letting a alloc_page() loop run for a while is usually not nice
> at 
> all :)
> 
> You can still do a sanity check here, e.g. the size should never
> have 
> the most significant bit set for example.
> 
Yes, this is a good solution. But if this a positive value, larger than
totalram, it can also pass this check, and cause OOM after some time.

From dicussion above, seems finding a proper solution that can judge
the size is valid or not for each dma-heap is more important.

Thanks!
Guangming

> Regards,
> Christian.
> 
> > 
> > thanks
> > -john
> 
> 
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Guangming.Cao]

On Fri, 2022-01-14 at 08:16 +0100, Christian König wrote:
> Am 14.01.22 um 00:26 schrieb John Stultz:
> > On Thu, Jan 13, 2022 at 5:05 AM Christian König
> > <christian.koenig@amd.com> wrote:
> > > Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> > > > > -----Original Message-----
> > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On
> > > > > Behalf Of
> > > > > Ruhl, Michael J
> > > > > > -----Original Message-----
> > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org>
> > > > > > On Behalf Of
> > > > > > guangming.cao@mediatek.com
> > > > > > +   /*
> > > > > > +    * Invalid size check. The "len" should be less than
> > > > > > totalram.
> > > > > > +    *
> > > > > > +    * Without this check, once the invalid size allocation
> > > > > > runs on a process
> > > > > > that
> > > > > > +    * can't be killed by OOM flow(such as "gralloc" on
> > > > > > Android devices), it
> > > > > > will
> > > > > > +    * cause a kernel exception, and to make matters worse,
> > > > > > we can't find
> > > > > > who are using
> > > > > > +    * so many memory with "dma_buf_debug_show" since the
> > > > > > relevant
> > > > > > dma-buf hasn't exported.
> > > > > > +    */
> > > > > > +   if (len >> PAGE_SHIFT > totalram_pages())
> > > > > 
> > > > > If your "heap" is from cma, is this still a valid check?
> > > > 
> > > > And thinking a bit further, if I create a heap from something
> > > > else (say device memory),
> > > > you will need to be able to figure out the maximum allowable
> > > > check for the specific
> > > > heap.
> > > > 
> > > > Maybe the heap needs a callback for max size?
Yes, I agree with this solution.
If dma-heap framework support this via adding a callback to support it,
seems it's more clear than adding a limitation in dma-heap framework
since each heap maybe has different limitation.
If you prefer adding callback, I can update this patch and add totalram
limitation to system dma-heap.

Thanks!
Guangming
> > > 
> > > Well we currently maintain a separate allocator and don't use
> > > dma-heap,
> > > but yes we have systems with 16GiB device and only 8GiB system
> > > memory so
> > > that check here is certainly not correct.
> > 
> > Good point.
> > 
> > > In general I would rather let the system run into -ENOMEM or
> > > -EINVAL
> > > from the allocator instead.

For system dma-heap, it doesn't know how memory is avaliable when
allocating memory, so, use totalram_pages() just to prevent cases which
will cause oom definitely.

Just like PAGE align, this check is can be used for all heaps since
there is no dma-heap can alloc memory larger than totalram. Futhermore,
if vendors implement a variety of dma-heap like system heap for special
usages, seems need to add this check to each dma-heap, and I think this
is unnecessary.
If the dma-heap has it's own special limitations for size, and add it
into heap implementation is good.

Thanks!
Guangming
> > 
> > Probably the simpler solution is to push the allocation check to
> > the
> > heap driver, rather than doing it at the top level here.
> > 
> > For CMA or other contiguous heaps, letting the allocator fail is
> > fast
> > enough. For noncontiguous buffers, like the system heap, the
> > allocation can burn a lot of time and consume a lot of memory
> > (causing
> > other trouble) before a large allocation might naturally fail.
> 
> Yeah, letting a alloc_page() loop run for a while is usually not nice
> at 
> all :)
> 
> You can still do a sanity check here, e.g. the size should never
> have 
> the most significant bit set for example.
> 
Yes, this is a good solution. But if this a positive value, larger than
totalram, it can also pass this check, and cause OOM after some time.

From dicussion above, seems finding a proper solution that can judge
the size is valid or not for each dma-heap is more important.

Thanks!
Guangming

> Regards,
> Christian.
> 
> > 
> > thanks
> > -john
> 
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Fri, Jan 14, 2022 at 4:04 AM Guangming.Cao
<guangming.cao@mediatek.com> wrote:
>
> On Fri, 2022-01-14 at 08:16 +0100, Christian König wrote:
> > Am 14.01.22 um 00:26 schrieb John Stultz:
> > > On Thu, Jan 13, 2022 at 5:05 AM Christian König
> > > <christian.koenig@amd.com> wrote:
> > > > Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> > > > > > -----Original Message-----
> > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On
> > > > > > Behalf Of
> > > > > > Ruhl, Michael J
> > > > > > > -----Original Message-----
> > > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org>
> > > > > > > On Behalf Of
> > > > > > > guangming.cao@mediatek.com
> > > > > > > +   /*
> > > > > > > +    * Invalid size check. The "len" should be less than
> > > > > > > totalram.
> > > > > > > +    *
> > > > > > > +    * Without this check, once the invalid size allocation
> > > > > > > runs on a process
> > > > > > > that
> > > > > > > +    * can't be killed by OOM flow(such as "gralloc" on
> > > > > > > Android devices), it
> > > > > > > will
> > > > > > > +    * cause a kernel exception, and to make matters worse,
> > > > > > > we can't find
> > > > > > > who are using
> > > > > > > +    * so many memory with "dma_buf_debug_show" since the
> > > > > > > relevant
> > > > > > > dma-buf hasn't exported.
> > > > > > > +    */
> > > > > > > +   if (len >> PAGE_SHIFT > totalram_pages())
> > > > > >
> > > > > > If your "heap" is from cma, is this still a valid check?
> > > > >
> > > > > And thinking a bit further, if I create a heap from something
> > > > > else (say device memory),
> > > > > you will need to be able to figure out the maximum allowable
> > > > > check for the specific
> > > > > heap.
> > > > >
> > > > > Maybe the heap needs a callback for max size?
> Yes, I agree with this solution.
> If dma-heap framework support this via adding a callback to support it,
> seems it's more clear than adding a limitation in dma-heap framework
> since each heap maybe has different limitation.
> If you prefer adding callback, I can update this patch and add totalram
> limitation to system dma-heap.

If the max value is per-heap, why not enforce that value in the
per-heap allocation function?

Moving the check to the heap alloc to me seems simpler to me than
adding complexity to the infrastructure to add a heap max_size
callback. Is there some other use for the callback that you envision?

thanks
-john

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Fri, Jan 14, 2022 at 4:04 AM Guangming.Cao
<guangming.cao@mediatek.com> wrote:
>
> On Fri, 2022-01-14 at 08:16 +0100, Christian König wrote:
> > Am 14.01.22 um 00:26 schrieb John Stultz:
> > > On Thu, Jan 13, 2022 at 5:05 AM Christian König
> > > <christian.koenig@amd.com> wrote:
> > > > Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> > > > > > -----Original Message-----
> > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On
> > > > > > Behalf Of
> > > > > > Ruhl, Michael J
> > > > > > > -----Original Message-----
> > > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org>
> > > > > > > On Behalf Of
> > > > > > > guangming.cao@mediatek.com
> > > > > > > +   /*
> > > > > > > +    * Invalid size check. The "len" should be less than
> > > > > > > totalram.
> > > > > > > +    *
> > > > > > > +    * Without this check, once the invalid size allocation
> > > > > > > runs on a process
> > > > > > > that
> > > > > > > +    * can't be killed by OOM flow(such as "gralloc" on
> > > > > > > Android devices), it
> > > > > > > will
> > > > > > > +    * cause a kernel exception, and to make matters worse,
> > > > > > > we can't find
> > > > > > > who are using
> > > > > > > +    * so many memory with "dma_buf_debug_show" since the
> > > > > > > relevant
> > > > > > > dma-buf hasn't exported.
> > > > > > > +    */
> > > > > > > +   if (len >> PAGE_SHIFT > totalram_pages())
> > > > > >
> > > > > > If your "heap" is from cma, is this still a valid check?
> > > > >
> > > > > And thinking a bit further, if I create a heap from something
> > > > > else (say device memory),
> > > > > you will need to be able to figure out the maximum allowable
> > > > > check for the specific
> > > > > heap.
> > > > >
> > > > > Maybe the heap needs a callback for max size?
> Yes, I agree with this solution.
> If dma-heap framework support this via adding a callback to support it,
> seems it's more clear than adding a limitation in dma-heap framework
> since each heap maybe has different limitation.
> If you prefer adding callback, I can update this patch and add totalram
> limitation to system dma-heap.

If the max value is per-heap, why not enforce that value in the
per-heap allocation function?

Moving the check to the heap alloc to me seems simpler to me than
adding complexity to the infrastructure to add a heap max_size
callback. Is there some other use for the callback that you envision?

thanks
-john

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Fri, Jan 14, 2022 at 4:04 AM Guangming.Cao
<guangming.cao@mediatek.com> wrote:
>
> On Fri, 2022-01-14 at 08:16 +0100, Christian König wrote:
> > Am 14.01.22 um 00:26 schrieb John Stultz:
> > > On Thu, Jan 13, 2022 at 5:05 AM Christian König
> > > <christian.koenig@amd.com> wrote:
> > > > Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> > > > > > -----Original Message-----
> > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On
> > > > > > Behalf Of
> > > > > > Ruhl, Michael J
> > > > > > > -----Original Message-----
> > > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org>
> > > > > > > On Behalf Of
> > > > > > > guangming.cao@mediatek.com
> > > > > > > +   /*
> > > > > > > +    * Invalid size check. The "len" should be less than
> > > > > > > totalram.
> > > > > > > +    *
> > > > > > > +    * Without this check, once the invalid size allocation
> > > > > > > runs on a process
> > > > > > > that
> > > > > > > +    * can't be killed by OOM flow(such as "gralloc" on
> > > > > > > Android devices), it
> > > > > > > will
> > > > > > > +    * cause a kernel exception, and to make matters worse,
> > > > > > > we can't find
> > > > > > > who are using
> > > > > > > +    * so many memory with "dma_buf_debug_show" since the
> > > > > > > relevant
> > > > > > > dma-buf hasn't exported.
> > > > > > > +    */
> > > > > > > +   if (len >> PAGE_SHIFT > totalram_pages())
> > > > > >
> > > > > > If your "heap" is from cma, is this still a valid check?
> > > > >
> > > > > And thinking a bit further, if I create a heap from something
> > > > > else (say device memory),
> > > > > you will need to be able to figure out the maximum allowable
> > > > > check for the specific
> > > > > heap.
> > > > >
> > > > > Maybe the heap needs a callback for max size?
> Yes, I agree with this solution.
> If dma-heap framework support this via adding a callback to support it,
> seems it's more clear than adding a limitation in dma-heap framework
> since each heap maybe has different limitation.
> If you prefer adding callback, I can update this patch and add totalram
> limitation to system dma-heap.

If the max value is per-heap, why not enforce that value in the
per-heap allocation function?

Moving the check to the heap alloc to me seems simpler to me than
adding complexity to the infrastructure to add a heap max_size
callback. Is there some other use for the callback that you envision?

thanks
-john

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Fri, Jan 14, 2022 at 4:04 AM Guangming.Cao
<guangming.cao@mediatek.com> wrote:
>
> On Fri, 2022-01-14 at 08:16 +0100, Christian König wrote:
> > Am 14.01.22 um 00:26 schrieb John Stultz:
> > > On Thu, Jan 13, 2022 at 5:05 AM Christian König
> > > <christian.koenig@amd.com> wrote:
> > > > Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> > > > > > -----Original Message-----
> > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On
> > > > > > Behalf Of
> > > > > > Ruhl, Michael J
> > > > > > > -----Original Message-----
> > > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org>
> > > > > > > On Behalf Of
> > > > > > > guangming.cao@mediatek.com
> > > > > > > +   /*
> > > > > > > +    * Invalid size check. The "len" should be less than
> > > > > > > totalram.
> > > > > > > +    *
> > > > > > > +    * Without this check, once the invalid size allocation
> > > > > > > runs on a process
> > > > > > > that
> > > > > > > +    * can't be killed by OOM flow(such as "gralloc" on
> > > > > > > Android devices), it
> > > > > > > will
> > > > > > > +    * cause a kernel exception, and to make matters worse,
> > > > > > > we can't find
> > > > > > > who are using
> > > > > > > +    * so many memory with "dma_buf_debug_show" since the
> > > > > > > relevant
> > > > > > > dma-buf hasn't exported.
> > > > > > > +    */
> > > > > > > +   if (len >> PAGE_SHIFT > totalram_pages())
> > > > > >
> > > > > > If your "heap" is from cma, is this still a valid check?
> > > > >
> > > > > And thinking a bit further, if I create a heap from something
> > > > > else (say device memory),
> > > > > you will need to be able to figure out the maximum allowable
> > > > > check for the specific
> > > > > heap.
> > > > >
> > > > > Maybe the heap needs a callback for max size?
> Yes, I agree with this solution.
> If dma-heap framework support this via adding a callback to support it,
> seems it's more clear than adding a limitation in dma-heap framework
> since each heap maybe has different limitation.
> If you prefer adding callback, I can update this patch and add totalram
> limitation to system dma-heap.

If the max value is per-heap, why not enforce that value in the
per-heap allocation function?

Moving the check to the heap alloc to me seems simpler to me than
adding complexity to the infrastructure to add a heap max_size
callback. Is there some other use for the callback that you envision?

thanks
-john

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Guangming.Cao]

On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote:
> On Fri, Jan 14, 2022 at 4:04 AM Guangming.Cao
> <guangming.cao@mediatek.com> wrote:
> > 
> > On Fri, 2022-01-14 at 08:16 +0100, Christian König wrote:
> > > Am 14.01.22 um 00:26 schrieb John Stultz:
> > > > On Thu, Jan 13, 2022 at 5:05 AM Christian König
> > > > <christian.koenig@amd.com> wrote:
> > > > > Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> > > > > > > -----Original Message-----
> > > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org>
> > > > > > > On
> > > > > > > Behalf Of
> > > > > > > Ruhl, Michael J
> > > > > > > > -----Original Message-----
> > > > > > > > From: dri-devel <
> > > > > > > > dri-devel-bounces@lists.freedesktop.org>
> > > > > > > > On Behalf Of
> > > > > > > > guangming.cao@mediatek.com
> > > > > > > > +   /*
> > > > > > > > +    * Invalid size check. The "len" should be less
> > > > > > > > than
> > > > > > > > totalram.
> > > > > > > > +    *
> > > > > > > > +    * Without this check, once the invalid size
> > > > > > > > allocation
> > > > > > > > runs on a process
> > > > > > > > that
> > > > > > > > +    * can't be killed by OOM flow(such as "gralloc" on
> > > > > > > > Android devices), it
> > > > > > > > will
> > > > > > > > +    * cause a kernel exception, and to make matters
> > > > > > > > worse,
> > > > > > > > we can't find
> > > > > > > > who are using
> > > > > > > > +    * so many memory with "dma_buf_debug_show" since
> > > > > > > > the
> > > > > > > > relevant
> > > > > > > > dma-buf hasn't exported.
> > > > > > > > +    */
> > > > > > > > +   if (len >> PAGE_SHIFT > totalram_pages())
> > > > > > > 
> > > > > > > If your "heap" is from cma, is this still a valid check?
> > > > > > 
> > > > > > And thinking a bit further, if I create a heap from
> > > > > > something
> > > > > > else (say device memory),
> > > > > > you will need to be able to figure out the maximum
> > > > > > allowable
> > > > > > check for the specific
> > > > > > heap.
> > > > > > 
> > > > > > Maybe the heap needs a callback for max size?
> > 
> > Yes, I agree with this solution.
> > If dma-heap framework support this via adding a callback to support
> > it,
> > seems it's more clear than adding a limitation in dma-heap
> > framework
> > since each heap maybe has different limitation.
> > If you prefer adding callback, I can update this patch and add
> > totalram
> > limitation to system dma-heap.
> 
> If the max value is per-heap, why not enforce that value in the
> per-heap allocation function?
> 
> Moving the check to the heap alloc to me seems simpler to me than
> adding complexity to the infrastructure to add a heap max_size
> callback. Is there some other use for the callback that you envision?
> 
> thanks
> -john

Thanks for your comment.

If you think max the value is per-heap, why not add an optional
callback for dma-heap to solve this issue(prevent consuming too much
time for a doomed to fail allocation), if the dma-heap doesn't have a
special size check, just use the default value(totalram) in dma-heap 
framework to do the size check.

Yes, for linux dma-heaps, only system-heap needs it, so adding it in
system heap is the simplest. However, there are many vendor dma-heaps
like system-heap which won't be uploaded to linux codebase, and maybe
have same limitation, all these heaps need to add the same limitation.
I just think it's boring. However, If you think discussing these absent
cases based on current linux code is meaningless, I also agree to it.

So, to summarize, if you still think adding it in system_heap.c is
better, I also agree and I will update the patch to add it in
system_heap.c

Thanks~
Guangming

> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Guangming.Cao]

On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote:
> On Fri, Jan 14, 2022 at 4:04 AM Guangming.Cao
> <guangming.cao@mediatek.com> wrote:
> > 
> > On Fri, 2022-01-14 at 08:16 +0100, Christian König wrote:
> > > Am 14.01.22 um 00:26 schrieb John Stultz:
> > > > On Thu, Jan 13, 2022 at 5:05 AM Christian König
> > > > <christian.koenig@amd.com> wrote:
> > > > > Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> > > > > > > -----Original Message-----
> > > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org>
> > > > > > > On
> > > > > > > Behalf Of
> > > > > > > Ruhl, Michael J
> > > > > > > > -----Original Message-----
> > > > > > > > From: dri-devel <
> > > > > > > > dri-devel-bounces@lists.freedesktop.org>
> > > > > > > > On Behalf Of
> > > > > > > > guangming.cao@mediatek.com
> > > > > > > > +   /*
> > > > > > > > +    * Invalid size check. The "len" should be less
> > > > > > > > than
> > > > > > > > totalram.
> > > > > > > > +    *
> > > > > > > > +    * Without this check, once the invalid size
> > > > > > > > allocation
> > > > > > > > runs on a process
> > > > > > > > that
> > > > > > > > +    * can't be killed by OOM flow(such as "gralloc" on
> > > > > > > > Android devices), it
> > > > > > > > will
> > > > > > > > +    * cause a kernel exception, and to make matters
> > > > > > > > worse,
> > > > > > > > we can't find
> > > > > > > > who are using
> > > > > > > > +    * so many memory with "dma_buf_debug_show" since
> > > > > > > > the
> > > > > > > > relevant
> > > > > > > > dma-buf hasn't exported.
> > > > > > > > +    */
> > > > > > > > +   if (len >> PAGE_SHIFT > totalram_pages())
> > > > > > > 
> > > > > > > If your "heap" is from cma, is this still a valid check?
> > > > > > 
> > > > > > And thinking a bit further, if I create a heap from
> > > > > > something
> > > > > > else (say device memory),
> > > > > > you will need to be able to figure out the maximum
> > > > > > allowable
> > > > > > check for the specific
> > > > > > heap.
> > > > > > 
> > > > > > Maybe the heap needs a callback for max size?
> > 
> > Yes, I agree with this solution.
> > If dma-heap framework support this via adding a callback to support
> > it,
> > seems it's more clear than adding a limitation in dma-heap
> > framework
> > since each heap maybe has different limitation.
> > If you prefer adding callback, I can update this patch and add
> > totalram
> > limitation to system dma-heap.
> 
> If the max value is per-heap, why not enforce that value in the
> per-heap allocation function?
> 
> Moving the check to the heap alloc to me seems simpler to me than
> adding complexity to the infrastructure to add a heap max_size
> callback. Is there some other use for the callback that you envision?
> 
> thanks
> -john

Thanks for your comment.

If you think max the value is per-heap, why not add an optional
callback for dma-heap to solve this issue(prevent consuming too much
time for a doomed to fail allocation), if the dma-heap doesn't have a
special size check, just use the default value(totalram) in dma-heap 
framework to do the size check.

Yes, for linux dma-heaps, only system-heap needs it, so adding it in
system heap is the simplest. However, there are many vendor dma-heaps
like system-heap which won't be uploaded to linux codebase, and maybe
have same limitation, all these heaps need to add the same limitation.
I just think it's boring. However, If you think discussing these absent
cases based on current linux code is meaningless, I also agree to it.

So, to summarize, if you still think adding it in system_heap.c is
better, I also agree and I will update the patch to add it in
system_heap.c

Thanks~
Guangming

> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[Guangming.Cao]

On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote:
> On Fri, Jan 14, 2022 at 4:04 AM Guangming.Cao
> <guangming.cao@mediatek.com> wrote:
> > 
> > On Fri, 2022-01-14 at 08:16 +0100, Christian König wrote:
> > > Am 14.01.22 um 00:26 schrieb John Stultz:
> > > > On Thu, Jan 13, 2022 at 5:05 AM Christian König
> > > > <christian.koenig@amd.com> wrote:
> > > > > Am 13.01.22 um 14:00 schrieb Ruhl, Michael J:
> > > > > > > -----Original Message-----
> > > > > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org>
> > > > > > > On
> > > > > > > Behalf Of
> > > > > > > Ruhl, Michael J
> > > > > > > > -----Original Message-----
> > > > > > > > From: dri-devel <
> > > > > > > > dri-devel-bounces@lists.freedesktop.org>
> > > > > > > > On Behalf Of
> > > > > > > > guangming.cao@mediatek.com
> > > > > > > > +   /*
> > > > > > > > +    * Invalid size check. The "len" should be less
> > > > > > > > than
> > > > > > > > totalram.
> > > > > > > > +    *
> > > > > > > > +    * Without this check, once the invalid size
> > > > > > > > allocation
> > > > > > > > runs on a process
> > > > > > > > that
> > > > > > > > +    * can't be killed by OOM flow(such as "gralloc" on
> > > > > > > > Android devices), it
> > > > > > > > will
> > > > > > > > +    * cause a kernel exception, and to make matters
> > > > > > > > worse,
> > > > > > > > we can't find
> > > > > > > > who are using
> > > > > > > > +    * so many memory with "dma_buf_debug_show" since
> > > > > > > > the
> > > > > > > > relevant
> > > > > > > > dma-buf hasn't exported.
> > > > > > > > +    */
> > > > > > > > +   if (len >> PAGE_SHIFT > totalram_pages())
> > > > > > > 
> > > > > > > If your "heap" is from cma, is this still a valid check?
> > > > > > 
> > > > > > And thinking a bit further, if I create a heap from
> > > > > > something
> > > > > > else (say device memory),
> > > > > > you will need to be able to figure out the maximum
> > > > > > allowable
> > > > > > check for the specific
> > > > > > heap.
> > > > > > 
> > > > > > Maybe the heap needs a callback for max size?
> > 
> > Yes, I agree with this solution.
> > If dma-heap framework support this via adding a callback to support
> > it,
> > seems it's more clear than adding a limitation in dma-heap
> > framework
> > since each heap maybe has different limitation.
> > If you prefer adding callback, I can update this patch and add
> > totalram
> > limitation to system dma-heap.
> 
> If the max value is per-heap, why not enforce that value in the
> per-heap allocation function?
> 
> Moving the check to the heap alloc to me seems simpler to me than
> adding complexity to the infrastructure to add a heap max_size
> callback. Is there some other use for the callback that you envision?
> 
> thanks
> -john

Thanks for your comment.

If you think max the value is per-heap, why not add an optional
callback for dma-heap to solve this issue(prevent consuming too much
time for a doomed to fail allocation), if the dma-heap doesn't have a
special size check, just use the default value(totalram) in dma-heap 
framework to do the size check.

Yes, for linux dma-heaps, only system-heap needs it, so adding it in
system heap is the simplest. However, there are many vendor dma-heaps
like system-heap which won't be uploaded to linux codebase, and maybe
have same limitation, all these heaps need to add the same limitation.
I just think it's boring. However, If you think discussing these absent
cases based on current linux code is meaningless, I also agree to it.

So, to summarize, if you still think adding it in system_heap.c is
better, I also agree and I will update the patch to add it in
system_heap.c

Thanks~
Guangming

> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Wed, Jan 19, 2022 at 1:58 AM Guangming.Cao
<guangming.cao@mediatek.com> wrote:
> On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote:
> > If the max value is per-heap, why not enforce that value in the
> > per-heap allocation function?
> >
> > Moving the check to the heap alloc to me seems simpler to me than
> > adding complexity to the infrastructure to add a heap max_size
> > callback. Is there some other use for the callback that you envision?
> >
>
> If you think max the value is per-heap, why not add an optional
> callback for dma-heap to solve this issue(prevent consuming too much
> time for a doomed to fail allocation), if the dma-heap doesn't have a
> special size check, just use the default value(totalram) in dma-heap
> framework to do the size check.

As the totalram default isn't correct for all heaps (or necessarily
even most heaps), so those heaps would need to implement the callback.

I'm just not sure adding complexity to the framework to address this
is useful. Instead of an additional check in the allocation function,
heap implementers will need to assess if the default logic in a
framework is correct, and then possibly implement the callback.

> Yes, for linux dma-heaps, only system-heap needs it, so adding it in
> system heap is the simplest. However, there are many vendor dma-heaps
> like system-heap which won't be uploaded to linux codebase, and maybe
> have same limitation, all these heaps need to add the same limitation.

My worry is that without seeing these vendor heaps, this is a bit of a
theoretical concern. We don't have the data on how common this is.
I very much hope that vendors can start submitting their heaps
upstream (along with drivers that benefit from the heaps). Then we can
really assess what makes the most sense for the community maintained
code.


> I just think it's boring. However, If you think discussing these absent
> cases based on current linux code is meaningless, I also agree to it.

So, as a rule, the upstream kernel doesn't create/maintain logic to
accommodate out of tree code.

Now, I agree there is the potential for some duplication in the checks
in the allocation logic, but until it affects the upstream kernel,
community maintainers can't really make an appropriate evaluation.

As a contra-example, if the allocation is some extreme hotpath, adding
an extra un-inlinable function pointer traversal for the size callback
may actually have a negative impact. This isn't likely but again, if
we cannot demonstrate it one way or the other against the upstream
tree, we can't figure out what the best solution might be.


> So, to summarize, if you still think adding it in system_heap.c is
> better, I also agree and I will update the patch to add it in
> system_heap.c

I think this is the best solution for now. As this is not part of an
userland ABI, we can always change it in the future once we see the
need.

thanks
-john

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Wed, Jan 19, 2022 at 1:58 AM Guangming.Cao
<guangming.cao@mediatek.com> wrote:
> On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote:
> > If the max value is per-heap, why not enforce that value in the
> > per-heap allocation function?
> >
> > Moving the check to the heap alloc to me seems simpler to me than
> > adding complexity to the infrastructure to add a heap max_size
> > callback. Is there some other use for the callback that you envision?
> >
>
> If you think max the value is per-heap, why not add an optional
> callback for dma-heap to solve this issue(prevent consuming too much
> time for a doomed to fail allocation), if the dma-heap doesn't have a
> special size check, just use the default value(totalram) in dma-heap
> framework to do the size check.

As the totalram default isn't correct for all heaps (or necessarily
even most heaps), so those heaps would need to implement the callback.

I'm just not sure adding complexity to the framework to address this
is useful. Instead of an additional check in the allocation function,
heap implementers will need to assess if the default logic in a
framework is correct, and then possibly implement the callback.

> Yes, for linux dma-heaps, only system-heap needs it, so adding it in
> system heap is the simplest. However, there are many vendor dma-heaps
> like system-heap which won't be uploaded to linux codebase, and maybe
> have same limitation, all these heaps need to add the same limitation.

My worry is that without seeing these vendor heaps, this is a bit of a
theoretical concern. We don't have the data on how common this is.
I very much hope that vendors can start submitting their heaps
upstream (along with drivers that benefit from the heaps). Then we can
really assess what makes the most sense for the community maintained
code.


> I just think it's boring. However, If you think discussing these absent
> cases based on current linux code is meaningless, I also agree to it.

So, as a rule, the upstream kernel doesn't create/maintain logic to
accommodate out of tree code.

Now, I agree there is the potential for some duplication in the checks
in the allocation logic, but until it affects the upstream kernel,
community maintainers can't really make an appropriate evaluation.

As a contra-example, if the allocation is some extreme hotpath, adding
an extra un-inlinable function pointer traversal for the size callback
may actually have a negative impact. This isn't likely but again, if
we cannot demonstrate it one way or the other against the upstream
tree, we can't figure out what the best solution might be.


> So, to summarize, if you still think adding it in system_heap.c is
> better, I also agree and I will update the patch to add it in
> system_heap.c

I think this is the best solution for now. As this is not part of an
userland ABI, we can always change it in the future once we see the
need.

thanks
-john

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Wed, Jan 19, 2022 at 1:58 AM Guangming.Cao
<guangming.cao@mediatek.com> wrote:
> On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote:
> > If the max value is per-heap, why not enforce that value in the
> > per-heap allocation function?
> >
> > Moving the check to the heap alloc to me seems simpler to me than
> > adding complexity to the infrastructure to add a heap max_size
> > callback. Is there some other use for the callback that you envision?
> >
>
> If you think max the value is per-heap, why not add an optional
> callback for dma-heap to solve this issue(prevent consuming too much
> time for a doomed to fail allocation), if the dma-heap doesn't have a
> special size check, just use the default value(totalram) in dma-heap
> framework to do the size check.

As the totalram default isn't correct for all heaps (or necessarily
even most heaps), so those heaps would need to implement the callback.

I'm just not sure adding complexity to the framework to address this
is useful. Instead of an additional check in the allocation function,
heap implementers will need to assess if the default logic in a
framework is correct, and then possibly implement the callback.

> Yes, for linux dma-heaps, only system-heap needs it, so adding it in
> system heap is the simplest. However, there are many vendor dma-heaps
> like system-heap which won't be uploaded to linux codebase, and maybe
> have same limitation, all these heaps need to add the same limitation.

My worry is that without seeing these vendor heaps, this is a bit of a
theoretical concern. We don't have the data on how common this is.
I very much hope that vendors can start submitting their heaps
upstream (along with drivers that benefit from the heaps). Then we can
really assess what makes the most sense for the community maintained
code.


> I just think it's boring. However, If you think discussing these absent
> cases based on current linux code is meaningless, I also agree to it.

So, as a rule, the upstream kernel doesn't create/maintain logic to
accommodate out of tree code.

Now, I agree there is the potential for some duplication in the checks
in the allocation logic, but until it affects the upstream kernel,
community maintainers can't really make an appropriate evaluation.

As a contra-example, if the allocation is some extreme hotpath, adding
an extra un-inlinable function pointer traversal for the size callback
may actually have a negative impact. This isn't likely but again, if
we cannot demonstrate it one way or the other against the upstream
tree, we can't figure out what the best solution might be.


> So, to summarize, if you still think adding it in system_heap.c is
> better, I also agree and I will update the patch to add it in
> system_heap.c

I think this is the best solution for now. As this is not part of an
userland ABI, we can always change it in the future once we see the
need.

thanks
-john

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation

[John Stultz]

On Wed, Jan 19, 2022 at 1:58 AM Guangming.Cao
<guangming.cao@mediatek.com> wrote:
> On Fri, 2022-01-14 at 17:17 -0800, John Stultz wrote:
> > If the max value is per-heap, why not enforce that value in the
> > per-heap allocation function?
> >
> > Moving the check to the heap alloc to me seems simpler to me than
> > adding complexity to the infrastructure to add a heap max_size
> > callback. Is there some other use for the callback that you envision?
> >
>
> If you think max the value is per-heap, why not add an optional
> callback for dma-heap to solve this issue(prevent consuming too much
> time for a doomed to fail allocation), if the dma-heap doesn't have a
> special size check, just use the default value(totalram) in dma-heap
> framework to do the size check.

As the totalram default isn't correct for all heaps (or necessarily
even most heaps), so those heaps would need to implement the callback.

I'm just not sure adding complexity to the framework to address this
is useful. Instead of an additional check in the allocation function,
heap implementers will need to assess if the default logic in a
framework is correct, and then possibly implement the callback.

> Yes, for linux dma-heaps, only system-heap needs it, so adding it in
> system heap is the simplest. However, there are many vendor dma-heaps
> like system-heap which won't be uploaded to linux codebase, and maybe
> have same limitation, all these heaps need to add the same limitation.

My worry is that without seeing these vendor heaps, this is a bit of a
theoretical concern. We don't have the data on how common this is.
I very much hope that vendors can start submitting their heaps
upstream (along with drivers that benefit from the heaps). Then we can
really assess what makes the most sense for the community maintained
code.


> I just think it's boring. However, If you think discussing these absent
> cases based on current linux code is meaningless, I also agree to it.

So, as a rule, the upstream kernel doesn't create/maintain logic to
accommodate out of tree code.

Now, I agree there is the potential for some duplication in the checks
in the allocation logic, but until it affects the upstream kernel,
community maintainers can't really make an appropriate evaluation.

As a contra-example, if the allocation is some extreme hotpath, adding
an extra un-inlinable function pointer traversal for the size callback
may actually have a negative impact. This isn't likely but again, if
we cannot demonstrate it one way or the other against the upstream
tree, we can't figure out what the best solution might be.


> So, to summarize, if you still think adding it in system_heap.c is
> better, I also agree and I will update the patch to add it in
> system_heap.c

I think this is the best solution for now. As this is not part of an
userland ABI, we can always change it in the future once we see the
need.

thanks
-john

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
And it can prevent consuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
---
 drivers/dma-buf/heaps/system_heap.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..bd6f255620e2 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,13 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent consuming
+	 * too much time for invalid allocations.
+	 */
+	if (len >> PAGE_SHIFT > totalram_pages())
+		return -EINVAL;
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1

[PATCH v4] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
And it can prevent consuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
---
 drivers/dma-buf/heaps/system_heap.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..bd6f255620e2 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,13 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent consuming
+	 * too much time for invalid allocations.
+	 */
+	if (len >> PAGE_SHIFT > totalram_pages())
+		return -EINVAL;
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1

[PATCH v4] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
And it can prevent consuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
---
 drivers/dma-buf/heaps/system_heap.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..bd6f255620e2 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,13 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent consuming
+	 * too much time for invalid allocations.
+	 */
+	if (len >> PAGE_SHIFT > totalram_pages())
+		return -EINVAL;
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v4] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
And it can prevent consuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
---
 drivers/dma-buf/heaps/system_heap.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..bd6f255620e2 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,13 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent consuming
+	 * too much time for invalid allocations.
+	 */
+	if (len >> PAGE_SHIFT > totalram_pages())
+		return -EINVAL;
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4] dma-buf: system_heap: Add a size check for allocation

[John Stultz]

On Wed, Jan 19, 2022 at 7:34 PM <guangming.cao@mediatek.com> wrote:
>
> From: Guangming <Guangming.Cao@mediatek.com>
>
> Add a size check for allocation since the allocation size should be
> always less than the total DRAM size on system heap.
> And it can prevent consuming too much time for invalid allocations.
>
> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> ---
>  drivers/dma-buf/heaps/system_heap.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..bd6f255620e2 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -347,6 +347,13 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>         struct page *page, *tmp_page;
>         int i, ret = -ENOMEM;
>
> +       /*
> +        * Size check. The "len" should be less than totalram since system_heap
> +        * memory is comes from system. Adding check here can prevent consuming
> +        * too much time for invalid allocations.
> +        */
> +       if (len >> PAGE_SHIFT > totalram_pages())
> +               return -EINVAL;

Thanks so much for revising and sending this along! It looks good to me.

Acked-by: John Stultz <john.stultz@linaro.org>

thanks again
-john

Re: [PATCH v4] dma-buf: system_heap: Add a size check for allocation

[John Stultz]

On Wed, Jan 19, 2022 at 7:34 PM <guangming.cao@mediatek.com> wrote:
>
> From: Guangming <Guangming.Cao@mediatek.com>
>
> Add a size check for allocation since the allocation size should be
> always less than the total DRAM size on system heap.
> And it can prevent consuming too much time for invalid allocations.
>
> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> ---
>  drivers/dma-buf/heaps/system_heap.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..bd6f255620e2 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -347,6 +347,13 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>         struct page *page, *tmp_page;
>         int i, ret = -ENOMEM;
>
> +       /*
> +        * Size check. The "len" should be less than totalram since system_heap
> +        * memory is comes from system. Adding check here can prevent consuming
> +        * too much time for invalid allocations.
> +        */
> +       if (len >> PAGE_SHIFT > totalram_pages())
> +               return -EINVAL;

Thanks so much for revising and sending this along! It looks good to me.

Acked-by: John Stultz <john.stultz@linaro.org>

thanks again
-john

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v4] dma-buf: system_heap: Add a size check for allocation

[John Stultz]

On Wed, Jan 19, 2022 at 7:34 PM <guangming.cao@mediatek.com> wrote:
>
> From: Guangming <Guangming.Cao@mediatek.com>
>
> Add a size check for allocation since the allocation size should be
> always less than the total DRAM size on system heap.
> And it can prevent consuming too much time for invalid allocations.
>
> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> ---
>  drivers/dma-buf/heaps/system_heap.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..bd6f255620e2 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -347,6 +347,13 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>         struct page *page, *tmp_page;
>         int i, ret = -ENOMEM;
>
> +       /*
> +        * Size check. The "len" should be less than totalram since system_heap
> +        * memory is comes from system. Adding check here can prevent consuming
> +        * too much time for invalid allocations.
> +        */
> +       if (len >> PAGE_SHIFT > totalram_pages())
> +               return -EINVAL;

Thanks so much for revising and sending this along! It looks good to me.

Acked-by: John Stultz <john.stultz@linaro.org>

thanks again
-john

Re: [PATCH v4] dma-buf: system_heap: Add a size check for allocation

[John Stultz]

On Wed, Jan 19, 2022 at 7:34 PM <guangming.cao@mediatek.com> wrote:
>
> From: Guangming <Guangming.Cao@mediatek.com>
>
> Add a size check for allocation since the allocation size should be
> always less than the total DRAM size on system heap.
> And it can prevent consuming too much time for invalid allocations.
>
> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> ---
>  drivers/dma-buf/heaps/system_heap.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..bd6f255620e2 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -347,6 +347,13 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>         struct page *page, *tmp_page;
>         int i, ret = -ENOMEM;
>
> +       /*
> +        * Size check. The "len" should be less than totalram since system_heap
> +        * memory is comes from system. Adding check here can prevent consuming
> +        * too much time for invalid allocations.
> +        */
> +       if (len >> PAGE_SHIFT > totalram_pages())
> +               return -EINVAL;

Thanks so much for revising and sending this along! It looks good to me.

Acked-by: John Stultz <john.stultz@linaro.org>

thanks again
-john

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
Adding this check can prevent comsuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
---
 drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..459dc18bc4a2 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent comsuming
+	 * too much time for invalid allocations.
+	 */
+	if (len >> PAGE_SHIFT > totalram_pages())
+		return ERR_PTR(-EINVAL);
+
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1

[PATCH v5] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
Adding this check can prevent comsuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
---
 drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..459dc18bc4a2 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent comsuming
+	 * too much time for invalid allocations.
+	 */
+	if (len >> PAGE_SHIFT > totalram_pages())
+		return ERR_PTR(-EINVAL);
+
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1

[PATCH v5] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
Adding this check can prevent comsuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
---
 drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..459dc18bc4a2 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent comsuming
+	 * too much time for invalid allocations.
+	 */
+	if (len >> PAGE_SHIFT > totalram_pages())
+		return ERR_PTR(-EINVAL);
+
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v5] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
Adding this check can prevent comsuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
---
 drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..459dc18bc4a2 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent comsuming
+	 * too much time for invalid allocations.
+	 */
+	if (len >> PAGE_SHIFT > totalram_pages())
+		return ERR_PTR(-EINVAL);
+
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5] dma-buf: system_heap: Add a size check for allocation

[Christian König]

Am 20.01.22 um 08:08 schrieb guangming.cao@mediatek.com:
> From: Guangming <Guangming.Cao@mediatek.com>
>
> Add a size check for allocation since the allocation size should be
> always less than the total DRAM size on system heap.
> Adding this check can prevent comsuming too much time for invalid allocations.
>
> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> ---
>   drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..459dc18bc4a2 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>   	struct page *page, *tmp_page;
>   	int i, ret = -ENOMEM;
>   
> +	/*
> +	 * Size check. The "len" should be less than totalram since system_heap
> +	 * memory is comes from system. Adding check here can prevent comsuming
> +	 * too much time for invalid allocations.
> +	 */
> +	if (len >> PAGE_SHIFT > totalram_pages())

Maybe use PFN_UP() or PFN_DOWN() here instead of open coding this.

Apart from that looks good to me.

Christian.

> +		return ERR_PTR(-EINVAL);
> +
>   	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
>   	if (!buffer)
>   		return ERR_PTR(-ENOMEM);

Re: [PATCH v5] dma-buf: system_heap: Add a size check for allocation

[Christian König]

Am 20.01.22 um 08:08 schrieb guangming.cao@mediatek.com:
> From: Guangming <Guangming.Cao@mediatek.com>
>
> Add a size check for allocation since the allocation size should be
> always less than the total DRAM size on system heap.
> Adding this check can prevent comsuming too much time for invalid allocations.
>
> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> ---
>   drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..459dc18bc4a2 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>   	struct page *page, *tmp_page;
>   	int i, ret = -ENOMEM;
>   
> +	/*
> +	 * Size check. The "len" should be less than totalram since system_heap
> +	 * memory is comes from system. Adding check here can prevent comsuming
> +	 * too much time for invalid allocations.
> +	 */
> +	if (len >> PAGE_SHIFT > totalram_pages())

Maybe use PFN_UP() or PFN_DOWN() here instead of open coding this.

Apart from that looks good to me.

Christian.

> +		return ERR_PTR(-EINVAL);
> +
>   	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
>   	if (!buffer)
>   		return ERR_PTR(-ENOMEM);


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v5] dma-buf: system_heap: Add a size check for allocation

[Christian König]

Am 20.01.22 um 08:08 schrieb guangming.cao@mediatek.com:
> From: Guangming <Guangming.Cao@mediatek.com>
>
> Add a size check for allocation since the allocation size should be
> always less than the total DRAM size on system heap.
> Adding this check can prevent comsuming too much time for invalid allocations.
>
> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> ---
>   drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..459dc18bc4a2 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>   	struct page *page, *tmp_page;
>   	int i, ret = -ENOMEM;
>   
> +	/*
> +	 * Size check. The "len" should be less than totalram since system_heap
> +	 * memory is comes from system. Adding check here can prevent comsuming
> +	 * too much time for invalid allocations.
> +	 */
> +	if (len >> PAGE_SHIFT > totalram_pages())

Maybe use PFN_UP() or PFN_DOWN() here instead of open coding this.

Apart from that looks good to me.

Christian.

> +		return ERR_PTR(-EINVAL);
> +
>   	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
>   	if (!buffer)
>   		return ERR_PTR(-ENOMEM);

Re: [PATCH v5] dma-buf: system_heap: Add a size check for allocation

[Christian König]

Am 20.01.22 um 08:08 schrieb guangming.cao@mediatek.com:
> From: Guangming <Guangming.Cao@mediatek.com>
>
> Add a size check for allocation since the allocation size should be
> always less than the total DRAM size on system heap.
> Adding this check can prevent comsuming too much time for invalid allocations.
>
> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> ---
>   drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..459dc18bc4a2 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>   	struct page *page, *tmp_page;
>   	int i, ret = -ENOMEM;
>   
> +	/*
> +	 * Size check. The "len" should be less than totalram since system_heap
> +	 * memory is comes from system. Adding check here can prevent comsuming
> +	 * too much time for invalid allocations.
> +	 */
> +	if (len >> PAGE_SHIFT > totalram_pages())

Maybe use PFN_UP() or PFN_DOWN() here instead of open coding this.

Apart from that looks good to me.

Christian.

> +		return ERR_PTR(-EINVAL);
> +
>   	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
>   	if (!buffer)
>   		return ERR_PTR(-ENOMEM);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v6] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
Adding this check can prevent comsuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
---
 drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..b65e597a742f 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent comsuming
+	 * too much time for invalid allocations.
+	 */
+	if (PFN_DOWN(len) > totalram_pages())
+		return ERR_PTR(-EINVAL);
+
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1

[PATCH v6] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
Adding this check can prevent comsuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
---
 drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..b65e597a742f 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent comsuming
+	 * too much time for invalid allocations.
+	 */
+	if (PFN_DOWN(len) > totalram_pages())
+		return ERR_PTR(-EINVAL);
+
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1

[PATCH v6] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
Adding this check can prevent comsuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
---
 drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..b65e597a742f 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent comsuming
+	 * too much time for invalid allocations.
+	 */
+	if (PFN_DOWN(len) > totalram_pages())
+		return ERR_PTR(-EINVAL);
+
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v6] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
Adding this check can prevent comsuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
---
 drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..b65e597a742f 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent comsuming
+	 * too much time for invalid allocations.
+	 */
+	if (PFN_DOWN(len) > totalram_pages())
+		return ERR_PTR(-EINVAL);
+
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v6 RESEND] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
Adding this check can prevent comsuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Acked-by: John Stultz <john.stultz@linaro.org>
---
 drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..b65e597a742f 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent comsuming
+	 * too much time for invalid allocations.
+	 */
+	if (PFN_DOWN(len) > totalram_pages())
+		return ERR_PTR(-EINVAL);
+
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1

[PATCH v6 RESEND] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
Adding this check can prevent comsuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Acked-by: John Stultz <john.stultz@linaro.org>
---
 drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..b65e597a742f 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent comsuming
+	 * too much time for invalid allocations.
+	 */
+	if (PFN_DOWN(len) > totalram_pages())
+		return ERR_PTR(-EINVAL);
+
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1

[PATCH v6 RESEND] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
Adding this check can prevent comsuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Acked-by: John Stultz <john.stultz@linaro.org>
---
 drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..b65e597a742f 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent comsuming
+	 * too much time for invalid allocations.
+	 */
+	if (PFN_DOWN(len) > totalram_pages())
+		return ERR_PTR(-EINVAL);
+
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v6 RESEND] dma-buf: system_heap: Add a size check for allocation

[guangming.cao]

From: Guangming <Guangming.Cao@mediatek.com>

Add a size check for allocation since the allocation size should be
always less than the total DRAM size on system heap.
Adding this check can prevent comsuming too much time for invalid allocations.

Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
Acked-by: John Stultz <john.stultz@linaro.org>
---
 drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 23a7e74ef966..b65e597a742f 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
 	struct page *page, *tmp_page;
 	int i, ret = -ENOMEM;
 
+	/*
+	 * Size check. The "len" should be less than totalram since system_heap
+	 * memory is comes from system. Adding check here can prevent comsuming
+	 * too much time for invalid allocations.
+	 */
+	if (PFN_DOWN(len) > totalram_pages())
+		return ERR_PTR(-EINVAL);
+
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6 RESEND] dma-buf: system_heap: Add a size check for allocation

[Christian König]

Am 20.01.22 um 11:00 schrieb guangming.cao@mediatek.com:
> From: Guangming <Guangming.Cao@mediatek.com>
>
> Add a size check for allocation since the allocation size should be
> always less than the total DRAM size on system heap.
> Adding this check can prevent comsuming too much time for invalid allocations.
>
> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> Acked-by: John Stultz <john.stultz@linaro.org>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..b65e597a742f 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>   	struct page *page, *tmp_page;
>   	int i, ret = -ENOMEM;
>   
> +	/*
> +	 * Size check. The "len" should be less than totalram since system_heap
> +	 * memory is comes from system. Adding check here can prevent comsuming
> +	 * too much time for invalid allocations.
> +	 */
> +	if (PFN_DOWN(len) > totalram_pages())
> +		return ERR_PTR(-EINVAL);
> +
>   	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
>   	if (!buffer)
>   		return ERR_PTR(-ENOMEM);

Re: [PATCH v6 RESEND] dma-buf: system_heap: Add a size check for allocation

[Christian König]

Am 20.01.22 um 11:00 schrieb guangming.cao@mediatek.com:
> From: Guangming <Guangming.Cao@mediatek.com>
>
> Add a size check for allocation since the allocation size should be
> always less than the total DRAM size on system heap.
> Adding this check can prevent comsuming too much time for invalid allocations.
>
> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> Acked-by: John Stultz <john.stultz@linaro.org>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..b65e597a742f 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>   	struct page *page, *tmp_page;
>   	int i, ret = -ENOMEM;
>   
> +	/*
> +	 * Size check. The "len" should be less than totalram since system_heap
> +	 * memory is comes from system. Adding check here can prevent comsuming
> +	 * too much time for invalid allocations.
> +	 */
> +	if (PFN_DOWN(len) > totalram_pages())
> +		return ERR_PTR(-EINVAL);
> +
>   	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
>   	if (!buffer)
>   		return ERR_PTR(-ENOMEM);


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v6 RESEND] dma-buf: system_heap: Add a size check for allocation

[Christian König]

Am 20.01.22 um 11:00 schrieb guangming.cao@mediatek.com:
> From: Guangming <Guangming.Cao@mediatek.com>
>
> Add a size check for allocation since the allocation size should be
> always less than the total DRAM size on system heap.
> Adding this check can prevent comsuming too much time for invalid allocations.
>
> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> Acked-by: John Stultz <john.stultz@linaro.org>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..b65e597a742f 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>   	struct page *page, *tmp_page;
>   	int i, ret = -ENOMEM;
>   
> +	/*
> +	 * Size check. The "len" should be less than totalram since system_heap
> +	 * memory is comes from system. Adding check here can prevent comsuming
> +	 * too much time for invalid allocations.
> +	 */
> +	if (PFN_DOWN(len) > totalram_pages())
> +		return ERR_PTR(-EINVAL);
> +
>   	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
>   	if (!buffer)
>   		return ERR_PTR(-ENOMEM);

Re: [PATCH v6 RESEND] dma-buf: system_heap: Add a size check for allocation

[Christian König]

Am 20.01.22 um 11:00 schrieb guangming.cao@mediatek.com:
> From: Guangming <Guangming.Cao@mediatek.com>
>
> Add a size check for allocation since the allocation size should be
> always less than the total DRAM size on system heap.
> Adding this check can prevent comsuming too much time for invalid allocations.
>
> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> Acked-by: John Stultz <john.stultz@linaro.org>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/dma-buf/heaps/system_heap.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 23a7e74ef966..b65e597a742f 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -347,6 +347,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>   	struct page *page, *tmp_page;
>   	int i, ret = -ENOMEM;
>   
> +	/*
> +	 * Size check. The "len" should be less than totalram since system_heap
> +	 * memory is comes from system. Adding check here can prevent comsuming
> +	 * too much time for invalid allocations.
> +	 */
> +	if (PFN_DOWN(len) > totalram_pages())
> +		return ERR_PTR(-EINVAL);
> +
>   	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
>   	if (!buffer)
>   		return ERR_PTR(-ENOMEM);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel