Re: [PATCH] drm/i915: Add sanity check for relocation entry pointer in execbuffer

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 16 Jun 2025, Sebastian Brzezinka <sebastian.brzezinka@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.brzezinka@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