Forwarded: [PATCH] mm/madvise: prefer VMA lock for MADV_REMOVE

Forwarded: [PATCH] mm/madvise: prefer VMA lock for MADV_REMOVE

[syzbot]

For archival purposes, forwarding an incoming command email to
syzbot@lists.linux.dev.

***

Subject: [PATCH] mm/madvise: prefer VMA lock for MADV_REMOVE
Author: wang.yaxin@zte.com.cn

From: Jiang Kun <jiang.kun2@zte.com.cn>

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-unstable

MADV_REMOVE currently runs under the process-wide mmap_read_lock() and
temporarily drops and reacquires it around filesystem hole punching. For
single-VMA, local-mm, non-UFFD-armed ranges we can validate the range under
the per-VMA read lock.

Calling into the filesystem (vfs_fallocate()) while holding a VMA lock may
create lock ordering issues involving inode locks, mmap_lock and vm_lock.
To avoid that, drop the VMA lock before invoking vfs_fallocate(), while
pinning the file with get_file(). The mmap_lock fallback path retains the
existing userfaultfd coordination and lock drop/reacquire behaviour.

Signed-off-by: Jiang Kun <jiang.kun2@zte.com.cn>
Signed-off-by: Yaxin Wang <wang.yaxin@zte.com.cn>
---
 mm/madvise.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 6bf7009fa5ce..7e71227a86d5 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1015,7 +1015,8 @@ static long madvise_remove(struct madvise_behavior *madv_behavior)
 	unsigned long start = madv_behavior->range.start;
 	unsigned long end = madv_behavior->range.end;

-	mark_mmap_lock_dropped(madv_behavior);
+	if (madv_behavior->lock_mode == MADVISE_MMAP_READ_LOCK)
+		mark_mmap_lock_dropped(madv_behavior);

 	if (vma->vm_flags & VM_LOCKED)
 		return -EINVAL;
@@ -1039,15 +1040,21 @@ static long madvise_remove(struct madvise_behavior *madv_behavior)
 	 * mmap_lock.
 	 */
 	get_file(f);
-	if (userfaultfd_remove(vma, start, end)) {
-		/* mmap_lock was not released by userfaultfd_remove() */
-		mmap_read_unlock(mm);
+	if (madv_behavior->lock_mode == MADVISE_VMA_READ_LOCK) {
+		vma_end_read(vma);
+		madv_behavior->vma = NULL;
+	} else {
+		if (userfaultfd_remove(vma, start, end)) {
+			/* mmap_lock was not released by userfaultfd_remove() */
+			mmap_read_unlock(mm);
+		}
 	}
 	error = vfs_fallocate(f,
 				FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 				offset, end - start);
 	fput(f);
-	mmap_read_lock(mm);
+	if (madv_behavior->lock_mode == MADVISE_MMAP_READ_LOCK)
+		mmap_read_lock(mm);
 	return error;
 }

@@ -1685,7 +1692,8 @@ int madvise_walk_vmas(struct madvise_behavior *madv_behavior)
 	if (madv_behavior->lock_mode == MADVISE_VMA_READ_LOCK &&
 	    try_vma_read_lock(madv_behavior)) {
 		error = madvise_vma_behavior(madv_behavior);
-		vma_end_read(madv_behavior->vma);
+		if (madv_behavior->vma)
+			vma_end_read(madv_behavior->vma);
 		return error;
 	}

@@ -1754,7 +1762,6 @@ static enum madvise_lock_mode get_lock_mode(struct madvise_behavior *madv_behavi
 		return MADVISE_NO_LOCK;

 	switch (madv_behavior->behavior) {
-	case MADV_REMOVE:
 	case MADV_WILLNEED:
 	case MADV_COLD:
 	case MADV_PAGEOUT:
@@ -1762,6 +1769,7 @@ static enum madvise_lock_mode get_lock_mode(struct madvise_behavior *madv_behavi
 	case MADV_POPULATE_WRITE:
 	case MADV_COLLAPSE:
 		return MADVISE_MMAP_READ_LOCK;
+	case MADV_REMOVE:
 	case MADV_GUARD_INSTALL:
 	case MADV_GUARD_REMOVE:
 	case MADV_DONTNEED:
-- 
2.43.5