Re: [PATCH] drm/i915/selftests: Fix unlocked mm.stolen in reset selftest

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

Hi,

On Wed Aug 26, 2026 at 10:49 AM CEST, Krzysztof Karas wrote:
> Hi Sebastian,
>
> On 2026-08-21 at 15:40:44 +0200, Sebastian Brzezinka wrote:
>> __igt_reset_stolen() use via __drm_mm_interval_first(), but
>> does so without holding i915->mm.stolen_lock. Every writer of this
>> (i915_gem_stolen.c: drm_mm_insert_node_in_range(), drm_mm_reserve_node(),
>> drm_mm_remove_node()) takes stolen_lock, so the selftest's unlocked
>> reader can race.
>> 
>> This was observed as a NULL pointer dereference in
>> __drm_mm_interval_first() during igt_reset_device_stolen():
>> 
>> BUG: kernel NULL pointer dereference, address: 0000000000000010
>> ...
>> RIP: 0010:__drm_mm_interval_first+0x2c/0x90
>> ...
>> Call Trace:
>>   __igt_reset_stolen+0x565/0x6a0 [i915]
>>   igt_reset_device_stolen+0x1a/0x30 [i915]
>>   __i915_subtests+0xb8/0x250 [i915]
>> ...
>> 
>> Fixes: 3da3c5c1c982 ("drm/i915: Exclude low pages (128KiB) of stolen from use")
>> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16841
>> Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/selftest_reset.c | 20 +++++++++++++-------
>>  1 file changed, 13 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
>> index 2cfc23c58e90..eb6b2261329d 100644
>> --- a/drivers/gpu/drm/i915/gt/selftest_reset.c
>> +++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
>> @@ -30,6 +30,7 @@ __igt_reset_stolen(struct intel_gt *gt,
>>  	void *tmp;
>>  	u32 *crc;
>>  	int err;
>> +	bool unused;
> You should ensure the mutex is initialized:
>
> if (!drm_mm_initialized(&gt->i915->mm.stolen))
>     return 0;
I don't think it's necessary, and even if it were, it would be outside
the scope of this patch.

>
>>  
>>  	if (!drm_mm_node_allocated(&ggtt->error_capture))
>>  		return 0;
>> @@ -95,9 +96,12 @@ __igt_reset_stolen(struct intel_gt *gt,
>>  				      ggtt->error_capture.start,
>>  				      PAGE_SIZE);
>>  
>> -		if (!__drm_mm_interval_first(&gt->i915->mm.stolen,
>> -					     page << PAGE_SHIFT,
>> -					     ((page + 1) << PAGE_SHIFT) - 1))
>> +		mutex_lock(&gt->i915->mm.stolen_lock);
>> +		unused = !__drm_mm_interval_first(&gt->i915->mm.stolen,
>> +						  page << PAGE_SHIFT,
>> +						  ((page + 1) << PAGE_SHIFT) - 1);
>> +		mutex_unlock(&gt->i915->mm.stolen_lock);
>> +		if (unused)
>>  			memset_io(s, STACK_MAGIC, PAGE_SIZE);
>>  
>>  		in = (void __force *)s;
>> @@ -143,10 +147,12 @@ __igt_reset_stolen(struct intel_gt *gt,
>>  			in = tmp;
>>  		x = crc32_le(0, in, PAGE_SIZE);
>>  
>> -		if (x != crc[page] &&
>> -		    !__drm_mm_interval_first(&gt->i915->mm.stolen,
>> -					     page << PAGE_SHIFT,
>> -					     ((page + 1) << PAGE_SHIFT) - 1)) {
>> +		mutex_lock(&gt->i915->mm.stolen_lock);
>> +		unused = !__drm_mm_interval_first(&gt->i915->mm.stolen,
>> +						  page << PAGE_SHIFT,
>> +						  ((page + 1) << PAGE_SHIFT) - 1);
>> +		mutex_unlock(&gt->i915->mm.stolen_lock);
>> +		if (x != crc[page] && unused) {
>>  			pr_debug("unused stolen page %pa modified by GPU reset\n",
>>  				 &page);
>>  			if (count++ == 0)
>> -- 
>> 2.55.0
>> 





-- 
Best regards,
Sebastian