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

[Sebastian Brzezinka <sebastian.brzezinka@intel.com>]

Hi Tvrtko,

On Tue Jun 17, 2025 at 11:39 AM UTC, Tvrtko Ursulin wrote:
>
> On 16/06/2025 15:22, Sebastian Brzezinka 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.
>> 
>> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11713
>> 
>> 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 */
>> +	if (!reloc || !access_ok(reloc, sizeof(*reloc))) {
>
> It doesn't look reloc is an user pointer - otherwise there wouldn't be 
> simply dereferenced just below. So something looks dodgy here, you 
> probably want to dig around a bit to figure out what is really going on.

Yes, you're right, it's indeed possible to pass both kernel and userspace
pointers. I overlooked that initially. I've corrected the issue on the
IGT side, so non-canonical pointers are no longer being sent.
Additionally, I attempted to improve the handling here, though 
it still needs more work to be fully robust. Thanks for the review!

-- 
Best regards,
Sebastian