_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 <[email protected]>
---
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
---
drivers/gpu/drm/i915/gt/selftest_reset.c | 23 ++++++++++++++++-------
1 file changed, 16 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..77af788314ff 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(>->i915->mm.stolen_lock);
+
+ return __drm_mm_interval_first(>->i915->mm.stolen,
+ page << PAGE_SHIFT,
+ ((page + 1) << PAGE_SHIFT) - 1) ==
+ >->i915->mm.stolen.head_node;
+}
+
static int
__igt_reset_stolen(struct intel_gt *gt,
intel_engine_mask_t mask,
@@ -95,10 +105,10 @@ __igt_reset_stolen(struct intel_gt *gt,
ggtt->error_capture.start,
PAGE_SIZE);
- if (!__drm_mm_interval_first(>->i915->mm.stolen,
- page << PAGE_SHIFT,
- ((page + 1) << PAGE_SHIFT) - 1))
+ mutex_lock(>->i915->mm.stolen_lock);
+ if (stolen_page_unused(gt, page))
memset_io(s, STACK_MAGIC, PAGE_SIZE);
+ mutex_unlock(>->i915->mm.stolen_lock);
in = (void __force *)s;
if (i915_memcpy_from_wc(tmp, in, PAGE_SIZE))
@@ -143,16 +153,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(>->i915->mm.stolen,
- page << PAGE_SHIFT,
- ((page + 1) << PAGE_SHIFT) - 1)) {
+ mutex_lock(>->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(>->i915->mm.stolen_lock);
io_mapping_unmap(s);
}
--
2.55.0