From: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
To: Krzysztof Karas <krzysztof.karas@intel.com>,
intel-gfx@lists.freedesktop.org
Cc: Andi Shyti <andi.shyti@linux.intel.com>,
Sebastian Brzezinka <sebastian.brzezinka@intel.com>,
Krzysztof Niemiec <krzysztof.niemiec@intel.com>
Subject: Re: [RFC v5 1/2] drm/i915/selftests: Prevent userspace mapping invalidation
Date: Thu, 09 Apr 2026 14:09:22 +0200 [thread overview]
Message-ID: <6bf5ddf0441044f3ba94218efc891cf7e11d8a84.camel@linux.intel.com> (raw)
In-Reply-To: <20260409080825.2191535-2-krzysztof.karas@intel.com>
Hi Krzysztof,
On Thu, 2026-04-09 at 08:08 +0000, Krzysztof Karas wrote:
> Migration testing in i915 assumes current task's address space
> to allocate new userspace mapping and uses it without
> registering real user for that address space in mm_struct.
> On single NUMA node setups PCI probe executes in the same
> context as userspace process calling the test (i915_selftest
> from IGT), but when multiple nodes are available, the PCI code
> puts probe into a kernel workqueue. This switches execution in
> a kworker, which does not have its own address space in
> userspace and must borrow such memory from another process, so
> "current->active_mm" is unknown at the start of the test.
>
> It was observed that mm->mm_users would occasionally be 0
> or drop to 0 during the test due to short delay between
> scheduling and executing work in forked process, which reaped
> userspace mappings, further leading to failures upon reading
> from userland memory.
>
> Prevent this by adding a PID parameter to a trusted task, so its
> mm struct may be used if needed.
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14204
> Fixes: 34b1c1c71d37 ("i915/selftest/igt_mmap: let mmap tests run in kthread")
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
> v2 (Janusz):
> * Reword and shorten commit message to be more precise.
> * Reorder variable declarations to follow upside down christmas
> tree style.
>
> v3 (Andi):
> * Prevent PID and mm leaks.
> * Remove a flag and use mm pointer to determine whether to
> release references to the memory.
>
> v4:
> * Revert !current->mm check. (Janusz, Sebastian)
> * Drop refernce to mm sooner. (Janusz)
> * Ensure kthread_use_mm did its job. (Janusz)
>
> v5 (Janusz):
> * Remove missing PID warning.
>
> drivers/gpu/drm/i915/i915_selftest.h | 1 +
> .../gpu/drm/i915/selftests/i915_selftest.c | 43 +++++++++++++++++++
> 2 files changed, 44 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
> index 72922028f4ba..e29ca298e7eb 100644
> --- a/drivers/gpu/drm/i915/i915_selftest.h
> +++ b/drivers/gpu/drm/i915/i915_selftest.h
> @@ -35,6 +35,7 @@ struct i915_selftest {
> unsigned long timeout_jiffies;
> unsigned int timeout_ms;
> unsigned int random_seed;
> + unsigned int userspace_pid;
> char *filter;
> int mock;
> int live;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> index 8460f0a70d04..2c169148c053 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> @@ -186,6 +186,8 @@ static int __run_selftests(const char *name,
> unsigned int count,
> void *data)
> {
> + int u_pid_nr = i915_selftest.userspace_pid;
> + struct mm_struct *mm = NULL;
> int err = 0;
>
> while (!i915_selftest.random_seed)
> @@ -201,6 +203,42 @@ static int __run_selftests(const char *name,
> pr_info(DRIVER_NAME ": Performing %s selftests with st_random_seed=0x%x st_timeout=%u\n",
> name, i915_selftest.random_seed, i915_selftest.timeout_ms);
>
> + /**
> + * If we are running in a kthread on a multi NUMA system and the user passed
> + * a valid PID of a userspace task, then we may borrow its address space
> + * to prepare a safe environment for the mmap selftests.
> + */
> + if (!current->mm) {
Please extend this condition over u_pid_nr actually provided by a user
(not 0?), otherwise we will be trying to get current->mm of a process
with default i915_selftest.userspace_pid and submitting a warning if
not successful.
Thanks,
Janusz
> + struct pid *u_pid = find_get_pid(u_pid_nr);
> + struct task_struct *task;
> +
> + if (!u_pid) {
> + pr_warn("Could not find PID: %d\n", u_pid_nr);
> + goto run_tests;
> + }
> +
> + task = get_pid_task(u_pid, PIDTYPE_PID);
> + put_pid(u_pid);
> + if (!task) {
> + pr_warn("Could not find userspace task for PID: %d\n", u_pid_nr);
> + goto run_tests;
> + }
> +
> + mm = get_task_mm(task);
> + put_task_struct(task);
> + if (!mm) {
> + pr_warn("Could not find address space of task with PID: %d\n", u_pid_nr);
> + goto run_tests;
> + }
> +
> + kthread_use_mm(mm);
> + mmput_async(mm);
> + if (unlikely(!current->mm)) {
> + pr_warn("Could not set mm as current->mm\n");
> + }
> + }
> +
> +run_tests:
> /* Tests are listed in order in i915_*_selftests.h */
> for (; count--; st++) {
> if (!st->enabled)
> @@ -226,6 +264,9 @@ static int __run_selftests(const char *name,
> st->name, err))
> err = -1;
>
> + if (mm)
> + kthread_unuse_mm(mm);
> +
> return err;
> }
>
> @@ -507,6 +548,8 @@ void igt_hexdump(const void *buf, size_t len)
> module_param_named(st_random_seed, i915_selftest.random_seed, uint, 0400);
> module_param_named(st_timeout, i915_selftest.timeout_ms, uint, 0400);
> module_param_named(st_filter, i915_selftest.filter, charp, 0400);
> +module_param_named(st_userspace_pid, i915_selftest.userspace_pid, uint, 0400);
> +MODULE_PARM_DESC(st_userspace_pid, "For usage in tests that map userspace memory and require address space with controllable lifetime.");
>
> module_param_named_unsafe(mock_selftests, i915_selftest.mock, int, 0400);
> MODULE_PARM_DESC(mock_selftests, "Run selftests before loading, using mock hardware (0:disabled [default], 1:run tests then load driver, -1:run tests then leave dummy module)");
next prev parent reply other threads:[~2026-04-09 12:09 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-09 8:08 [RFC v5 0/2] drm/i915/selftests: Use safe userspace memory for mappings Krzysztof Karas
2026-04-09 8:08 ` [RFC v5 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
2026-04-09 12:09 ` Janusz Krzysztofik [this message]
2026-04-09 8:08 ` [RFC v5 2/2] drm/i915/selftests: Run vma tests only if current->mm is present Krzysztof Karas
2026-04-09 11:16 ` Andi Shyti
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6bf5ddf0441044f3ba94218efc891cf7e11d8a84.camel@linux.intel.com \
--to=janusz.krzysztofik@linux.intel.com \
--cc=andi.shyti@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=krzysztof.karas@intel.com \
--cc=krzysztof.niemiec@intel.com \
--cc=sebastian.brzezinka@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox