Re: [PATCH v3] dma-buf/heaps: system_heap: avoid too much allocation

[Michal Hocko <mhocko@suse.com>]

On Wed 12-04-23 17:57:26, Jaewon Kim wrote:
> >Sorry for being late. I know there was some pre-existing discussion
> >around that but I didn't have time to participate.
> >
> >On Mon 10-04-23 16:32:28, Jaewon Kim wrote:
> >> @@ -350,6 +350,9 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
> >>  	struct page *page, *tmp_page;
> >>  	int i, ret = -ENOMEM;
> >>  
> >> +	if (len / PAGE_SIZE > totalram_pages())
> >> +		return ERR_PTR(-ENOMEM);
> >> +
> >
> >This is an antipattern imho. Check 7661809d493b ("mm: don't allow
> >oversized kvmalloc() calls") how kvmalloc has dealt with a similar
> 
> Hello Thank you for the information.
> 
> I tried to search the macro of INT_MAX.
> 
> include/vdso/limits.h
> #define INT_MAX         ((int)(~0U >> 1))
> 
> AFAIK the dma-buf system heap user can request that huge size more than 2GB.

Do you have any pointers? This all is unreclaimable memory, right? How
are those users constrained to not go overboard?

> So
> I think totalram_pages() is better than INT_MAX in this case.
> 
> >issue. totalram_pages doesn't really tell you anything about incorrect
> >users. You might be on a low memory system where the request size is
> >sane normally, it just doesn't fit into memory on that particular
> >machine.
> 
> Sorry maybe I'm not fully understand what you meant. User may requested
> a huge size like 3GB on 2GB ram device. But I think that should be rejected
> because it is bigger than the device ram size.

Even totalram_pages/10 can be just unfeasible amount of data to be
allocated without a major disruption. totalram_pages is no measure of
the memory availability.
If you want to have a ballpark estimation then si_mem_available might be
something you are looking for. But I thought the sole purpose of this
patch is to catch obviously buggy callers (like sign overflow lenght
etc) rather than any memory consumption sanity check.

-- 
Michal Hocko
SUSE Labs