On Sat, Nov 9, 2019 at 1:01 AM <cohens@xxxxxxxxxxxxxx> wrote: > > On 2019-11-08 03:34, Daniel Vetter wrote: > > On Thu, Nov 07, 2019 at 02:39:11PM -0500, Steve Cohen wrote: > >> Fuzzers used in Android compliance testing repeatedly call the > >> create blob IOCTL which eventually exhausts the system memory. > >> This series adds a hook which allows drivers to impose their own > >> limitations on the size and/or number of blobs created. > > > > Pretty sure this isn't just a problem for msm/dpu alone, why this very > > limited approach? > > > I'm not familiar enough with the blob requirements for other > vendor's drivers to impose any restrictions on them. The idea > was to provide the hook for vendors to implement their own > checks. Support for msm/mdp* drivers will be added in v2 if this > approach is acceptable. Yeah, but I don't think different limits (and then maybe different tunables for these different limits) on drivers isn't a great idea. Plus I thought this kind of stuff was supposed to be catched by the kernel memory cgroup controller. Does that not work? The blob stuff is maybe a bit too direct way to allocate kernel memory, but there's many others I've thought. This all just feels a lot like busywork to check an android compliance checkbox, without really digging into the underlying problem and fixing it for real. One approach that would work I think would be: - Extended the blob property type to have min/max size (we might need a range for some), set it for all current blob properties. To do that we'd need to create a new property creation function for blobs, which takes those limits. There's not a hole lot of them, so should be doable. - In the create blob function we walk all property (or book-keep that somewhere) and check the blob isn't too big. - We also validate the size when setting the property, at least some of that code from various places. I think this would actually improve the situation here, instead of whack-a-mole. The cheaper cope-out would be if we simply limit the total property size to something big enough, but not unlimited, like 16MB or something like that. > > Also, why are your fuzzers not also allocating enormous amounts of gem > > buffers, which will also exhaust memory eventually? > > Excellent question... This will likely come in a follow-up series. Would be good to know that, so we can solve this for real, not just a one-off for the compliance checkbox. Also really wondering why cgroups doesn't catch this. -Daniel > > > -Daniel > > > >> > >> Steve Cohen (3): > >> drm: add driver hook for create blob limitations > >> drm/msm: add support for createblob_check driver hook > >> drm/msm/dpu: check blob limitations during create blob ioctl > >> > >> drivers/gpu/drm/drm_property.c | 7 +++++++ > >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 ++++++++++++++ > >> drivers/gpu/drm/msm/msm_drv.c | 25 > >> +++++++++++++++++++++++++ > >> drivers/gpu/drm/msm/msm_kms.h | 1 + > >> include/drm/drm_drv.h | 9 +++++++++ > >> 5 files changed, 56 insertions(+) > >> > >> -- > >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora > >> Forum, > >> a Linux Foundation Collaborative Project > >> > >> _______________________________________________ > >> dri-devel mailing list > >> dri-devel@xxxxxxxxxxxxxxxxxxxxx > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
