mm/khugepaged: fix vm_lock/i_mmap_rwsem inversion in retract_page_tables

Internal syscaller on linux-next reported a lock inversion cause by
vm_lock being taken after i_mmap_rwsem:

======================================================
WARNING: possible circular locking dependency detected
6.2.0-next-20230301-syzkaller #0 Not tainted
------------------------------------------------------
syz-executor115/5084 is trying to acquire lock:
ffff888078307a90 (&vma->vm_lock->lock){++++}-{3:3}, at: vma_start_write include/linux/mm.h:678 [inline]
ffff888078307a90 (&vma->vm_lock->lock){++++}-{3:3}, at: retract_page_tables mm/khugepaged.c:1826 [inline]
ffff888078307a90 (&vma->vm_lock->lock){++++}-{3:3}, at: collapse_file+0x4fa5/0x5980 mm/khugepaged.c:2204

but task is already holding lock:
ffff88801f93efa8 (&mapping->i_mmap_rwsem){++++}-{3:3}, at: i_mmap_lock_write include/linux/fs.h:468 [inline]
ffff88801f93efa8 (&mapping->i_mmap_rwsem){++++}-{3:3}, at: retract_page_tables mm/khugepaged.c:1745 [inline]
ffff88801f93efa8 (&mapping->i_mmap_rwsem){++++}-{3:3}, at: collapse_file+0x3da6/0x5980 mm/khugepaged.c:2204

retract_page_tables takes i_mmap_rwsem before exclusive mmap_lock, which
is inverse to normal order.  Deadlock is avoided by try-locking mmap_lock
and skipping on failure to obtain it.  Locking the VMA should use the same
locking pattern to avoid this lock inversion.

Link: https://lkml.kernel.org/r/20230303213250.3555716-1-surenb@google.com
Fixes: 44a83f2083bd ("mm/khugepaged: write-lock VMA while collapsing a huge page")
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reported-by: <syzbot+8955a9646d1a48b8be92@syzkaller.appspotmail.com>
Cc: Arjun Roy <arjunroy@google.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Greg Thelen <gthelen@google.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Jann Horn <jannh@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: kernel-team@android.com
Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
Cc: Lorenzo Stoakes <lstoakes@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Michel Lespinasse <michel@lespinasse.org>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Peter Oskolkov <posk@google.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Punit Agrawal <punit.agrawal@bytedance.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Shakeel Butt <shakeelb@google.com>
Cc: Song Liu <songliubraving@fb.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 3d5e8666892daf5edb3fdd5bfbb30590ece99a7a..8a981f0085f25940901dabc69e11d5a0a8143d66 100644 (file)
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -664,18 +664,23 @@ static inline void vma_end_read(struct vm_area_struct *vma)
        rcu_read_unlock();
 }
 
-static inline void vma_start_write(struct vm_area_struct *vma)
+static bool __is_vma_write_locked(struct vm_area_struct *vma, int *mm_lock_seq)
 {
-       int mm_lock_seq;
-
        mmap_assert_write_locked(vma->vm_mm);
 
        /*
         * current task is holding mmap_write_lock, both vma->vm_lock_seq and
         * mm->mm_lock_seq can't be concurrently modified.
         */
-       mm_lock_seq = READ_ONCE(vma->vm_mm->mm_lock_seq);
-       if (vma->vm_lock_seq == mm_lock_seq)
+       *mm_lock_seq = READ_ONCE(vma->vm_mm->mm_lock_seq);
+       return (vma->vm_lock_seq == *mm_lock_seq);
+}
+
+static inline void vma_start_write(struct vm_area_struct *vma)
+{
+       int mm_lock_seq;
+
+       if (__is_vma_write_locked(vma, &mm_lock_seq))
                return;
 
        down_write(&vma->lock);
@@ -683,14 +688,26 @@ static inline void vma_start_write(struct vm_area_struct *vma)
        up_write(&vma->lock);
 }
 
+static inline bool vma_try_start_write(struct vm_area_struct *vma)
+{
+       int mm_lock_seq;
+
+       if (__is_vma_write_locked(vma, &mm_lock_seq))
+               return true;
+
+       if (!down_write_trylock(&vma->vm_lock->lock))
+               return false;
+
+       vma->vm_lock_seq = mm_lock_seq;
+       up_write(&vma->vm_lock->lock);
+       return true;
+}
+
 static inline void vma_assert_write_locked(struct vm_area_struct *vma)
 {
-       mmap_assert_write_locked(vma->vm_mm);
-       /*
-        * current task is holding mmap_write_lock, both vma->vm_lock_seq and
-        * mm->mm_lock_seq can't be concurrently modified.
-        */
-       VM_BUG_ON_VMA(vma->vm_lock_seq != READ_ONCE(vma->vm_mm->mm_lock_seq), vma);
+       int mm_lock_seq;
+
+       VM_BUG_ON_VMA(!__is_vma_write_locked(vma, &mm_lock_seq), vma);
 }
 
 #else /* CONFIG_PER_VMA_LOCK */

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 21d683e2043008daa8d01bfa5ee1804d72b482e4..27956d44041347a75c2dc7820ba0a20ec85e20aa 100644 (file)
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1698,6 +1698,10 @@ static int retract_page_tables(struct address_space *mapping, pgoff_t pgoff,
                result = SCAN_PTE_MAPPED_HUGEPAGE;
                if ((cc->is_khugepaged || is_target) &&
                    mmap_write_trylock(mm)) {
+                       /* trylock for the same lock inversion as above */
+                       if (!vma_try_start_write(vma))
+                               goto unlock_next;
+
                        /*
                         * Re-check whether we have an ->anon_vma, because
                         * collapse_and_free_pmd() requires that either no
@@ -1726,7 +1730,6 @@ static int retract_page_tables(struct address_space *mapping, pgoff_t pgoff,
                                result = SCAN_PTE_UFFD_WP;
                                goto unlock_next;
                        }
-                       vma_start_write(vma);
                        collapse_and_free_pmd(mm, vma, addr, pmd);
                        if (!cc->is_khugepaged && is_target)
                                result = set_huge_pmd(vma, addr, pmd, hpage);