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

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

[Sebastian Brzezinka]

_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]
...

Also cap num_pages to i915->dsm.usable_size, which already excludes the
hardware reserved region from drm_mm.stolen's range, so poison/verify
writes never touch reserved stolen pages.

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>
---
v1 -> v2:
- Fix always false !__drm_mm_interval_first() check (Andi)
- Extend stolen_lock over the poison/verify writes, not just
  the lookup (Andi)
- Move the lookup into stolen_page_unused() helper

v2 -> v3:
- Cap num_pages to i915->dsm.usable_size to avoid poisoning
  hardware reserved stolen pages (Shashiko)
---
 drivers/gpu/drm/i915/gt/selftest_reset.c | 26 ++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
index 2cfc23c58e90..811ddf991cb2 100644
--- a/drivers/gpu/drm/i915/gt/selftest_reset.c
+++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
@@ -14,6 +14,16 @@
 #include "selftests/igt_atomic.h"
 #include "selftests/igt_spinner.h"
 
+static bool stolen_page_unused(struct intel_gt *gt, resource_size_t page)
+{
+	lockdep_assert_held(&gt->i915->mm.stolen_lock);
+
+	return __drm_mm_interval_first(&gt->i915->mm.stolen,
+					page << PAGE_SHIFT,
+					((page + 1) << PAGE_SHIFT) - 1) ==
+	       &gt->i915->mm.stolen.head_node;
+}
+
 static int
 __igt_reset_stolen(struct intel_gt *gt,
 		   intel_engine_mask_t mask,
@@ -34,7 +44,8 @@ __igt_reset_stolen(struct intel_gt *gt,
 	if (!drm_mm_node_allocated(&ggtt->error_capture))
 		return 0;
 
-	num_pages = resource_size(dsm) >> PAGE_SHIFT;
+	num_pages = min_t(resource_size_t, resource_size(dsm),
+			  gt->i915->dsm.usable_size) >> PAGE_SHIFT;
 	if (!num_pages)
 		return 0;
 
@@ -95,10 +106,10 @@ __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);
+		if (stolen_page_unused(gt, page))
 			memset_io(s, STACK_MAGIC, PAGE_SIZE);
+		mutex_unlock(&gt->i915->mm.stolen_lock);
 
 		in = (void __force *)s;
 		if (i915_memcpy_from_wc(tmp, in, PAGE_SIZE))
@@ -143,16 +154,15 @@ __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);
+		if (x != crc[page] && stolen_page_unused(gt, page)) {
 			pr_debug("unused stolen page %pa modified by GPU reset\n",
 				 &page);
 			if (count++ == 0)
 				igt_hexdump(in, PAGE_SIZE);
 			max = page;
 		}
+		mutex_unlock(&gt->i915->mm.stolen_lock);
 
 		io_mapping_unmap(s);
 	}
-- 
2.55.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: TOCTOU race in unused stolen memory verification
--

commit c306e84112aa79cbd3009b777df94536525c83b4
Author: Sebastian Brzezinka <sebastian.brzezinka@intel.com>

drm/i915/selftests: Fix unlocked mm.stolen in reset selftest

This commit fixes an unlocked mm.stolen read by introducing the
stolen_page_unused() helper and wrapping accesses with stolen_lock. It also
caps num_pages to avoid the hardware-reserved region.

> diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
> index 2cfc23c58e909..811ddf991cb27 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_reset.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_reset.c

[ ... ]

> @@ -95,10 +106,10 @@ __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);
> +		if (stolen_page_unused(gt, page))
>  			memset_io(s, STACK_MAGIC, PAGE_SIZE);
> +		mutex_unlock(&gt->i915->mm.stolen_lock);
>  
>  		in = (void __force *)s;
>  		if (i915_memcpy_from_wc(tmp, in, PAGE_SIZE))
>  			in = tmp;
>  		crc[page] = crc32_le(0, in, PAGE_SIZE);

[Severity: Medium]
Since stolen_lock is dropped before computing the CRC, does this introduce a
race window where a concurrent thread can allocate the page and modify it
before the initial CRC is calculated?

[ ... ]

> @@ -143,16 +154,15 @@ __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);
> +		if (x != crc[page] && stolen_page_unused(gt, page)) {

[Severity: Medium]
Similarly, can a concurrent thread legitimately allocate, modify, and free the
page while the lock is dropped here?

If this happens, when stolen_lock is re-acquired, stolen_page_unused() will
return true, but the CRC will mismatch. Could this lead the test to falsely
report that the GPU reset clobbered unused stolen memory?

>  			pr_debug("unused stolen page %pa modified by GPU reset\n",
>  				 &page);
>  			if (count++ == 0)
>  				igt_hexdump(in, PAGE_SIZE);
>  			max = page;
>  		}
> +		mutex_unlock(&gt->i915->mm.stolen_lock);
>  
>  		io_mapping_unmap(s);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930113513.15169-1-sebastian.brzezinka@intel.com?part=1

✗ i915.CI.BAT: failure for drm/i915/selftests: Fix unlocked mm.stolen in reset selftest (rev5)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 2538 bytes --]

== Series Details ==

Series: drm/i915/selftests: Fix unlocked mm.stolen in reset selftest (rev5)
URL   : https://patchwork.freedesktop.org/series/172573/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_19242 -> Patchwork_172573v5
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_172573v5 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_172573v5, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172573v5/index.html

Participating hosts (39 -> 38)
------------------------------

  Missing    (1): bat-dg2-13 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_172573v5:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live:
    - fi-glk-j4005:       [PASS][1] -> [DMESG-FAIL][2] +1 other test dmesg-fail
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19242/fi-glk-j4005/igt@i915_selftest@live.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172573v5/fi-glk-j4005/igt@i915_selftest@live.html
    - bat-apl-1:          [PASS][3] -> [DMESG-FAIL][4] +1 other test dmesg-fail
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19242/bat-apl-1/igt@i915_selftest@live.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172573v5/bat-apl-1/igt@i915_selftest@live.html

  * igt@i915_selftest@live@reset:
    - fi-bsw-nick:        [PASS][5] -> [DMESG-FAIL][6] +1 other test dmesg-fail
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19242/fi-bsw-nick/igt@i915_selftest@live@reset.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172573v5/fi-bsw-nick/igt@i915_selftest@live@reset.html

  


Build changes
-------------

  * Linux: CI_DRM_19242 -> Patchwork_172573v5

  CI-20190529: 20190529
  CI_DRM_19242: dde43fa4801498eefd0280da6c729ff5a178f866 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_9118: 26513be3e0f711ed835ec50d5cdcb723ef224105 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_172573v5: dde43fa4801498eefd0280da6c729ff5a178f866 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_172573v5/index.html

[-- Attachment #2: Type: text/html, Size: 3189 bytes --]

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

[Krzysztof Karas]

Hi Sebastian,

On 2026-09-30 at 13:35:13 +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]
> ...
> 
> Also cap num_pages to i915->dsm.usable_size, which already excludes the
> hardware reserved region from drm_mm.stolen's range, so poison/verify
> writes never touch reserved stolen pages.
> 
> 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>
> ---
> v1 -> v2:
> - Fix always false !__drm_mm_interval_first() check (Andi)
> - Extend stolen_lock over the poison/verify writes, not just
>   the lookup (Andi)
> - Move the lookup into stolen_page_unused() helper
> 
> v2 -> v3:
> - Cap num_pages to i915->dsm.usable_size to avoid poisoning
>   hardware reserved stolen pages (Shashiko)
> ---
>  drivers/gpu/drm/i915/gt/selftest_reset.c | 26 ++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
> index 2cfc23c58e90..811ddf991cb2 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_reset.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
> @@ -14,6 +14,16 @@
>  #include "selftests/igt_atomic.h"
>  #include "selftests/igt_spinner.h"
>  
> +static bool stolen_page_unused(struct intel_gt *gt, resource_size_t page)
> +{
> +	lockdep_assert_held(&gt->i915->mm.stolen_lock);
> +
> +	return __drm_mm_interval_first(&gt->i915->mm.stolen,
> +					page << PAGE_SHIFT,
> +					((page + 1) << PAGE_SHIFT) - 1) ==
> +	       &gt->i915->mm.stolen.head_node;
> +}
> +
>  static int
>  __igt_reset_stolen(struct intel_gt *gt,
>  		   intel_engine_mask_t mask,
> @@ -34,7 +44,8 @@ __igt_reset_stolen(struct intel_gt *gt,
>  	if (!drm_mm_node_allocated(&ggtt->error_capture))
>  		return 0;
>  
> -	num_pages = resource_size(dsm) >> PAGE_SHIFT;
> +	num_pages = min_t(resource_size_t, resource_size(dsm),
> +			  gt->i915->dsm.usable_size) >> PAGE_SHIFT;
>  	if (!num_pages)
>  		return 0;
>  
> @@ -95,10 +106,10 @@ __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);
> +		if (stolen_page_unused(gt, page))
>  			memset_io(s, STACK_MAGIC, PAGE_SIZE);
> +		mutex_unlock(&gt->i915->mm.stolen_lock);
>  
>  		in = (void __force *)s;
>  		if (i915_memcpy_from_wc(tmp, in, PAGE_SIZE))
> @@ -143,16 +154,15 @@ __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);
> +		if (x != crc[page] && stolen_page_unused(gt, page)) {
>  			pr_debug("unused stolen page %pa modified by GPU reset\n",
>  				 &page);
>  			if (count++ == 0)
>  				igt_hexdump(in, PAGE_SIZE);
>  			max = page;
>  		}
> +		mutex_unlock(&gt->i915->mm.stolen_lock);
>  
>  		io_mapping_unmap(s);
>  	}
> -- 
> 2.55.0
> 

Reviewed-by: Krzysztof Karas <krzysztof.karas@intel.com>

-- 
Best Regards,
Krzysztof