Hi Janusz,

> Hi Krzysztof,
> 
> On Monday, 19 January 2026 11:16:02 CET Krzysztof Karas wrote:
> > IGT mmap testing in i915 uses current task's address space to
> > allocate new userspace mapping, without registering real user
> > for that address space in mm_struct.
> > 
> > It was observed that mm->mm_users would occasionally drop to 0
> > during tests, which reaped userspace mappings, further leading
> > to failures upon reading from userland memory.
> > 
> > Prevent this by artificially increasing mm_users counter for the
> > duration of the test.
> > 
> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14204
> > Signed-off-by: Krzysztof Karas <[email protected]>
> > ---
> > During testing I also found out that this problem affects
> > another function, __igt_mmap(), which also utilizes userspace
> > VMAs.
> > 
> > v2:
> >  * use mmget/mmput() (Jani);
> >  * include __igt_mmap() in the scope;
> >  * change comments and commit message;
> > 
> >  .../drm/i915/gem/selftests/i915_gem_mman.c    | 24 +++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c 
> > b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> > index 0d250d57496a..82ab090f66c8 100644
> > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> > @@ -916,6 +916,13 @@ static int __igt_mmap(struct drm_i915_private *i915,
> >     if (err)
> >             return err;
> >  
> > +   /*
> > +    * Get a reference to tasks's mm_struct to artificially increase 
> > mm_users
> > +    * and ensure the kernel does not try to clean up the userspace mappings
> > +    * of the current task during the test.
> > +    */
> > +   mmget_not_zero(current->mm);
> 
> What happens if that fails?
This cannot really fail, it may return false, if no other
references are currently held, which has its own implication
that I overlooked:
if mmget_not_zero() returns false, then we probably should not
call mmput().

On the other hand, I observed that the issue does not occur if
mm_users is 0 since the beginning. The problem only arises when
we go from mm_users == 1 to mm_users == 0.

However, I think it would be better to use unconditional mmget()
to better show our intentions and ensure no tampering with
userspace takes place (if the test works with mm_users == 0 and
with mm_users > 0, as long as the condition remains unchanged
throughout the test, then we could force the latter to ensure
stable userspace state accross every test run).

This was a bit lenghty, but does it answer your question?

-- 
Best Regards,
Krzysztof

Reply via email to