* [PATCH] dma-buf: heaps: Set allocation limit for system heap
@ 2021-07-22 19:07 ` Hridya Valsaraju
0 siblings, 0 replies; 8+ messages in thread
From: Hridya Valsaraju @ 2021-07-22 19:07 UTC (permalink / raw)
To: Sumit Semwal, Benjamin Gaignard, Liam Mark, Laura Abbott,
Brian Starkey, John Stultz, Christian König, linux-media,
dri-devel, linaro-mm-sig, linux-kernel
Cc: kernel-team, gregkh, Hridya Valsaraju
This patch limits the size of total memory that can be requested in a
single allocation from the system heap. This would prevent a
buggy/malicious client from depleting system memory by requesting for an
extremely large allocation which might destabilize the system.
The limit is set to half the size of the device's total RAM which is the
same as what was set by the deprecated ION system heap.
Signed-off-by: Hridya Valsaraju <hridya@google.com>
---
drivers/dma-buf/heaps/system_heap.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index b7fbce66bcc0..099f5a8304b4 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -371,6 +371,12 @@ static struct dma_buf *system_heap_do_allocate(struct dma_heap *heap,
struct page *page, *tmp_page;
int i, ret = -ENOMEM;
+ if (len / PAGE_SIZE > totalram_pages() / 2) {
+ pr_err("pid %d requested too large an allocation(size %lu) from system heap\n",
+ current->pid, len);
+ return ERR_PTR(ret);
+ }
+
buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
if (!buffer)
return ERR_PTR(-ENOMEM);
--
2.32.0.432.gabb21c7263-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] dma-buf: heaps: Set allocation limit for system heap
@ 2021-07-22 19:07 ` Hridya Valsaraju
0 siblings, 0 replies; 8+ messages in thread
From: Hridya Valsaraju @ 2021-07-22 19:07 UTC (permalink / raw)
To: Sumit Semwal, Benjamin Gaignard, Liam Mark, Laura Abbott,
Brian Starkey, John Stultz, Christian König, linux-media,
dri-devel, linaro-mm-sig, linux-kernel
Cc: Hridya Valsaraju, gregkh, kernel-team
This patch limits the size of total memory that can be requested in a
single allocation from the system heap. This would prevent a
buggy/malicious client from depleting system memory by requesting for an
extremely large allocation which might destabilize the system.
The limit is set to half the size of the device's total RAM which is the
same as what was set by the deprecated ION system heap.
Signed-off-by: Hridya Valsaraju <hridya@google.com>
---
drivers/dma-buf/heaps/system_heap.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index b7fbce66bcc0..099f5a8304b4 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -371,6 +371,12 @@ static struct dma_buf *system_heap_do_allocate(struct dma_heap *heap,
struct page *page, *tmp_page;
int i, ret = -ENOMEM;
+ if (len / PAGE_SIZE > totalram_pages() / 2) {
+ pr_err("pid %d requested too large an allocation(size %lu) from system heap\n",
+ current->pid, len);
+ return ERR_PTR(ret);
+ }
+
buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
if (!buffer)
return ERR_PTR(-ENOMEM);
--
2.32.0.432.gabb21c7263-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] dma-buf: heaps: Set allocation limit for system heap
2021-07-22 19:07 ` Hridya Valsaraju
(?)
@ 2021-08-03 2:18 ` John Stultz
2021-08-03 21:39 ` Hridya Valsaraju
-1 siblings, 1 reply; 8+ messages in thread
From: John Stultz @ 2021-08-03 2:18 UTC (permalink / raw)
To: Hridya Valsaraju
Cc: Sumit Semwal, Benjamin Gaignard, Liam Mark, Laura Abbott,
Brian Starkey, Christian König, linux-media, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, lkml,
Android Kernel Team, Greg Kroah-Hartman
On Thu, Jul 22, 2021 at 12:07 PM Hridya Valsaraju <hridya@google.com> wrote:
> This patch limits the size of total memory that can be requested in a
> single allocation from the system heap. This would prevent a
> buggy/malicious client from depleting system memory by requesting for an
> extremely large allocation which might destabilize the system.
>
> The limit is set to half the size of the device's total RAM which is the
> same as what was set by the deprecated ION system heap.
>
> Signed-off-by: Hridya Valsaraju <hridya@google.com>
Seems sane to me, unless folks have better suggestions for allocation limits.
Reviewed-by: John Stultz <john.stultz@linaro.org>
thanks
-john
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dma-buf: heaps: Set allocation limit for system heap
2021-08-03 2:18 ` John Stultz
@ 2021-08-03 21:39 ` Hridya Valsaraju
2021-08-10 8:24 ` Sumit Semwal
2021-08-10 8:26 ` Sumit Semwal
0 siblings, 2 replies; 8+ messages in thread
From: Hridya Valsaraju @ 2021-08-03 21:39 UTC (permalink / raw)
To: John Stultz
Cc: Sumit Semwal, Benjamin Gaignard, Liam Mark, Laura Abbott,
Brian Starkey, Christian König, linux-media, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, lkml,
Android Kernel Team, Greg Kroah-Hartman
On Mon, Aug 2, 2021 at 7:18 PM John Stultz <john.stultz@linaro.org> wrote:
>
> On Thu, Jul 22, 2021 at 12:07 PM Hridya Valsaraju <hridya@google.com> wrote:
> > This patch limits the size of total memory that can be requested in a
> > single allocation from the system heap. This would prevent a
> > buggy/malicious client from depleting system memory by requesting for an
> > extremely large allocation which might destabilize the system.
> >
> > The limit is set to half the size of the device's total RAM which is the
> > same as what was set by the deprecated ION system heap.
> >
> > Signed-off-by: Hridya Valsaraju <hridya@google.com>
>
> Seems sane to me, unless folks have better suggestions for allocation limits.
>
> Reviewed-by: John Stultz <john.stultz@linaro.org>
Thank you for taking a look John!
Regards,
Hridya
>
> thanks
> -john
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dma-buf: heaps: Set allocation limit for system heap
2021-08-03 21:39 ` Hridya Valsaraju
@ 2021-08-10 8:24 ` Sumit Semwal
2021-08-10 8:40 ` Daniel Vetter
2021-08-10 8:26 ` Sumit Semwal
1 sibling, 1 reply; 8+ messages in thread
From: Sumit Semwal @ 2021-08-10 8:24 UTC (permalink / raw)
To: Hridya Valsaraju
Cc: John Stultz, Benjamin Gaignard, Liam Mark, Laura Abbott,
Brian Starkey, Christian König, linux-media, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, lkml,
Android Kernel Team, Greg Kroah-Hartman
[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]
Hi Hridya,
Apologies for the delay in responding.
On Wed, 4 Aug 2021 at 03:09, Hridya Valsaraju <hridya@google.com> wrote:
> On Mon, Aug 2, 2021 at 7:18 PM John Stultz <john.stultz@linaro.org> wrote:
> >
> > On Thu, Jul 22, 2021 at 12:07 PM Hridya Valsaraju <hridya@google.com>
> wrote:
> > > This patch limits the size of total memory that can be requested in a
> > > single allocation from the system heap. This would prevent a
> > > buggy/malicious client from depleting system memory by requesting for
> an
> > > extremely large allocation which might destabilize the system.
> > >
> > > The limit is set to half the size of the device's total RAM which is
> the
> > > same as what was set by the deprecated ION system heap.
> > >
> > > Signed-off-by: Hridya Valsaraju <hridya@google.com>
> >
> > Seems sane to me, unless folks have better suggestions for allocation
> limits.
> >
> > Reviewed-by: John Stultz <john.stultz@linaro.org>
>
> Thank you for taking a look John!
>
Looks good to me; I will apply it to drm-misc today.
>
> Regards,
> Hridya
>
> >
> > thanks
> > -john
>
Best,
Sumit.
--
Thanks and regards,
Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs
[-- Attachment #2: Type: text/html, Size: 2239 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dma-buf: heaps: Set allocation limit for system heap
2021-08-03 21:39 ` Hridya Valsaraju
2021-08-10 8:24 ` Sumit Semwal
@ 2021-08-10 8:26 ` Sumit Semwal
1 sibling, 0 replies; 8+ messages in thread
From: Sumit Semwal @ 2021-08-10 8:26 UTC (permalink / raw)
To: Hridya Valsaraju
Cc: John Stultz, Liam Mark, Laura Abbott, Brian Starkey,
Christian König, linux-media, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, lkml,
Android Kernel Team, Greg Kroah-Hartman
Hi Hridya,
Apologies for the delay in responding;
On Wed, 4 Aug 2021 at 03:09, Hridya Valsaraju <hridya@google.com> wrote:
>
> On Mon, Aug 2, 2021 at 7:18 PM John Stultz <john.stultz@linaro.org> wrote:
> >
> > On Thu, Jul 22, 2021 at 12:07 PM Hridya Valsaraju <hridya@google.com> wrote:
> > > This patch limits the size of total memory that can be requested in a
> > > single allocation from the system heap. This would prevent a
> > > buggy/malicious client from depleting system memory by requesting for an
> > > extremely large allocation which might destabilize the system.
> > >
> > > The limit is set to half the size of the device's total RAM which is the
> > > same as what was set by the deprecated ION system heap.
> > >
> > > Signed-off-by: Hridya Valsaraju <hridya@google.com>
> >
> > Seems sane to me, unless folks have better suggestions for allocation limits.
> >
> > Reviewed-by: John Stultz <john.stultz@linaro.org>
>
> Thank you for taking a look John!
Looks good to me; I will apply it to drm-misc today.
>
> Regards,
> Hridya
>
> >
> > thanks
> > -john
Best,
Sumit.
--
Thanks and regards,
Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dma-buf: heaps: Set allocation limit for system heap
2021-08-10 8:24 ` Sumit Semwal
@ 2021-08-10 8:40 ` Daniel Vetter
2021-08-10 17:48 ` Hridya Valsaraju
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2021-08-10 8:40 UTC (permalink / raw)
To: Sumit Semwal
Cc: Hridya Valsaraju, John Stultz, Benjamin Gaignard, Liam Mark,
Laura Abbott, Brian Starkey, Christian König, linux-media,
dri-devel, moderated list:DMA BUFFER SHARING FRAMEWORK, lkml,
Android Kernel Team, Greg Kroah-Hartman
On Tue, Aug 10, 2021 at 01:54:41PM +0530, Sumit Semwal wrote:
> Hi Hridya,
>
> Apologies for the delay in responding.
>
> On Wed, 4 Aug 2021 at 03:09, Hridya Valsaraju <hridya@google.com> wrote:
>
> > On Mon, Aug 2, 2021 at 7:18 PM John Stultz <john.stultz@linaro.org> wrote:
> > >
> > > On Thu, Jul 22, 2021 at 12:07 PM Hridya Valsaraju <hridya@google.com>
> > wrote:
> > > > This patch limits the size of total memory that can be requested in a
> > > > single allocation from the system heap. This would prevent a
> > > > buggy/malicious client from depleting system memory by requesting for
> > an
> > > > extremely large allocation which might destabilize the system.
> > > >
> > > > The limit is set to half the size of the device's total RAM which is
> > the
> > > > same as what was set by the deprecated ION system heap.
> > > >
> > > > Signed-off-by: Hridya Valsaraju <hridya@google.com>
> > >
> > > Seems sane to me, unless folks have better suggestions for allocation
> > limits.
> > >
> > > Reviewed-by: John Stultz <john.stultz@linaro.org>
> >
> > Thank you for taking a look John!
> >
> Looks good to me; I will apply it to drm-misc today.
Please don't, this doesn't really solve anything:
- it's easy to bypass, just allocate more buffers to get over the limit
- resource limit plan is cgroups, not hand-rolled limits in every
allocator
- the ttm "max half of system memory" is for pinned memory, to work around
locking inversion issues between dma_resv_lock and core mm shrinkers. It
does not actually impose an overall allocation limit, you can allocate
ttm bo until your entire memory (and swap) are full. Christian König has
merged a patch set to lift this by reworking the shrinker interaction,
but it had to be reverted again because of some fallout I can't remember
offhand. dma_resv_lock vs shrinkers is very tricky.
So if you want resource limits then you really want cgroups here.
Cheers, Daniel
>
>
> >
> > Regards,
> > Hridya
> >
> > >
> > > thanks
> > > -john
> >
> Best,
> Sumit.
>
> --
> Thanks and regards,
>
> Sumit Semwal (he / him)
> Tech Lead - LCG, Vertical Technologies
> Linaro.org │ Open source software for ARM SoCs
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dma-buf: heaps: Set allocation limit for system heap
2021-08-10 8:40 ` Daniel Vetter
@ 2021-08-10 17:48 ` Hridya Valsaraju
0 siblings, 0 replies; 8+ messages in thread
From: Hridya Valsaraju @ 2021-08-10 17:48 UTC (permalink / raw)
To: Sumit Semwal, Hridya Valsaraju, John Stultz, Benjamin Gaignard,
Liam Mark, Laura Abbott, Brian Starkey, Christian König,
linux-media, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, lkml,
Android Kernel Team, Greg Kroah-Hartman
On Tue, Aug 10, 2021 at 1:40 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Tue, Aug 10, 2021 at 01:54:41PM +0530, Sumit Semwal wrote:
> > Hi Hridya,
> >
> > Apologies for the delay in responding.
> >
> > On Wed, 4 Aug 2021 at 03:09, Hridya Valsaraju <hridya@google.com> wrote:
> >
> > > On Mon, Aug 2, 2021 at 7:18 PM John Stultz <john.stultz@linaro.org> wrote:
> > > >
> > > > On Thu, Jul 22, 2021 at 12:07 PM Hridya Valsaraju <hridya@google.com>
> > > wrote:
> > > > > This patch limits the size of total memory that can be requested in a
> > > > > single allocation from the system heap. This would prevent a
> > > > > buggy/malicious client from depleting system memory by requesting for
> > > an
> > > > > extremely large allocation which might destabilize the system.
> > > > >
> > > > > The limit is set to half the size of the device's total RAM which is
> > > the
> > > > > same as what was set by the deprecated ION system heap.
> > > > >
> > > > > Signed-off-by: Hridya Valsaraju <hridya@google.com>
> > > >
> > > > Seems sane to me, unless folks have better suggestions for allocation
> > > limits.
> > > >
> > > > Reviewed-by: John Stultz <john.stultz@linaro.org>
> > >
> > > Thank you for taking a look John!
> > >
> > Looks good to me; I will apply it to drm-misc today.
>
> Please don't, this doesn't really solve anything:
> - it's easy to bypass, just allocate more buffers to get over the limit
> - resource limit plan is cgroups, not hand-rolled limits in every
> allocator
> - the ttm "max half of system memory" is for pinned memory, to work around
> locking inversion issues between dma_resv_lock and core mm shrinkers. It
> does not actually impose an overall allocation limit, you can allocate
> ttm bo until your entire memory (and swap) are full. Christian König has
> merged a patch set to lift this by reworking the shrinker interaction,
> but it had to be reverted again because of some fallout I can't remember
> offhand. dma_resv_lock vs shrinkers is very tricky.
>
> So if you want resource limits then you really want cgroups here.
Thanks Daniel and Sumit, that makes sense. Once the GPU memory
accounting cgroups are ready, we should be able to set the limit using
the same.
Regards,
Hridya
>
> Cheers, Daniel
>
> >
> >
> > >
> > > Regards,
> > > Hridya
> > >
> > > >
> > > > thanks
> > > > -john
> > >
> > Best,
> > Sumit.
> >
> > --
> > Thanks and regards,
> >
> > Sumit Semwal (he / him)
> > Tech Lead - LCG, Vertical Technologies
> > Linaro.org │ Open source software for ARM SoCs
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-08-10 18:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-22 19:07 [PATCH] dma-buf: heaps: Set allocation limit for system heap Hridya Valsaraju
2021-07-22 19:07 ` Hridya Valsaraju
2021-08-03 2:18 ` John Stultz
2021-08-03 21:39 ` Hridya Valsaraju
2021-08-10 8:24 ` Sumit Semwal
2021-08-10 8:40 ` Daniel Vetter
2021-08-10 17:48 ` Hridya Valsaraju
2021-08-10 8:26 ` Sumit Semwal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.