On Mon, 16 Jun 2025, Sebastian Brzezinka <sebastian.brzezi...@intel.com> wrote: > This patch adds a defensive check in `eb_relocate_entry()` to validate > the relocation entry pointer before dereferencing it. It ensures the > pointer is non-NULL and accessible from userspace using `access_ok()`. > > This prevents potential kernel crashes caused by invalid or non-canonical > pointers passed from userspace. > > If the pointer is invalid, an error is logged and the > function returns -EFAULT. > > The failure was observed on a Tiger Lake system while running the IGT > test `igt@gem_exec_big@single`. An appropriate patch has also been > submitted to fix the issue on the IGT side.
I don't know if the patch at hand is the right thing to do (I mean I don't know that it *isn't* either), but some comments nonetheless. > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11713 > Superfluous newline. Please keep the trailer lines together. > Signed-off-by: Sebastian Brzezinka <sebastian.brzezi...@intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index ca7e9216934a..8056dea0e656 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -1427,6 +1427,12 @@ eb_relocate_entry(struct i915_execbuffer *eb, > struct eb_vma *target; > int err; > > + /* Sanity check for non-canonical or NULL pointer */ Is this comment helpful to the reader? > + if (!reloc || !access_ok(reloc, sizeof(*reloc))) { > + DRM_ERROR("Invalid relocation entry pointer: %p\n", reloc); drm_err() please. > + return -EFAULT; > + } > + > /* we've already hold a reference to all valid objects */ > target = eb_get_vma(eb, reloc->target_handle); > if (unlikely(!target)) -- Jani Nikula, Intel
