lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 14 Jan 2022 08:16:30 +0100 From: Christian König <christian.koenig@....com> To: John Stultz <john.stultz@...aro.org> Cc: "Ruhl, Michael J" <michael.j.ruhl@...el.com>, "guangming.cao@...iatek.com" <guangming.cao@...iatek.com>, "sumit.semwal@...aro.org" <sumit.semwal@...aro.org>, "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, "wsd_upstream@...iatek.com" <wsd_upstream@...iatek.com>, "libo.kang@...iatek.com" <libo.kang@...iatek.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>, "yf.wang@...iatek.com" <yf.wang@...iatek.com>, "linaro-mm-sig@...ts.linaro.org" <linaro-mm-sig@...ts.linaro.org>, "linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>, "lmark@...eaurora.org" <lmark@...eaurora.org>, "benjamin.gaignard@...aro.org" <benjamin.gaignard@...aro.org>, "bo.song@...iatek.com" <bo.song@...iatek.com>, "matthias.bgg@...il.com" <matthias.bgg@...il.com>, "labbott@...hat.com" <labbott@...hat.com>, "mingyuan.ma@...iatek.com" <mingyuan.ma@...iatek.com>, "jianjiao.zeng@...iatek.com" <jianjiao.zeng@...iatek.com>, "linux-media@...r.kernel.org" <linux-media@...r.kernel.org> Subject: Re: [PATCH v3] dma-buf: dma-heap: Add a size check for allocation Am 14.01.22 um 00:26 schrieb John Stultz: > On Thu, Jan 13, 2022 at 5:05 AM Christian König > <christian.koenig@....com> wrote: >> Am 13.01.22 um 14:00 schrieb Ruhl, Michael J: >>>> -----Original Message----- >>>> From: dri-devel <dri-devel-bounces@...ts.freedesktop.org> On Behalf Of >>>> Ruhl, Michael J >>>>> -----Original Message----- >>>>> From: dri-devel <dri-devel-bounces@...ts.freedesktop.org> On Behalf Of >>>>> guangming.cao@...iatek.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
Powered by blists - more mailing lists