Hi Krzysztof, > Initialize eb->vma[].vma pointers to NULL when the eb structure is first > set up. > > During the execution of eb_lookup_vmas(), the eb->vma array is > successively filled up with struct eb_vma objects. This process includes > calling eb_add_vma(), which might fail; however, even in the event of > failure, eb->vma[i].vma is set for the currently processed buffer. > > If eb_add_vma() fails, eb_lookup_vmas() returns with an error, which > prompts a call to eb_release_vmas() to clean up the mess. Since > eb_lookup_vmas() might fail during processing any (possibly not first) > buffer, eb_release_vmas() checks whether a buffer's vma is NULL to know > at what point did the lookup function fail. > > In eb_lookup_vmas(), eb->vma[i].vma is set to NULL if either the helper > function eb_lookup_vma() or eb_validate_vma() fails. eb->vma[i+1].vma is > set to NULL in case i915_gem_object_userptr_submit_init() fails; the > current one needs to be cleaned up by eb_release_vmas() at this point, > so the next one is set. If eb_add_vma() fails, neither the current nor > the next vma is nullified, which is a source of a NULL deref bug > described in [1]. > > When entering eb_lookup_vmas(), the vma pointers are set to the slab > poison value, instead of NULL. This doesn't matter for the actual > lookup, since it gets overwritten anyway, however the eb_release_vmas() > function only recognizes NULL as the stopping value, hence the pointers > are being nullified as they go in case of intermediate failure. This > patch changes the approach to filling them all with NULL at the start > instead, rather than handling that manually during failure. > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15062 > Fixes: 544460c33821 ("drm/i915: Multi-BB execbuf") > Reported-by: Gangmin Kim <[email protected]> > Cc: <[email protected]> # 5.16.x > Signed-off-by: Krzysztof Niemiec <[email protected]> > ---
LGTM: Reviewed-by: Krzysztof Karas <[email protected]> -- Best Regards, Krzysztof
