[PATCH] userfaultfd: fix lock leak in mfill_get_vma()

[PATCH] userfaultfd: fix lock leak in mfill_get_vma()

[Andrei Vagin]

In mfill_get_vma(), both the per-VMA lock (via uffd_mfill_lock()) and
the map_changing_lock are acquired. However, state->vma was only
assigned at the very end of the function. If any validation checks
failed after lock acquisition but before this assignment, the function
would jump to the out_unlock label and call mfill_put_vma(state).

Since mfill_put_vma() only performs cleanup if state->vma is non-NULL,
these error paths would leak both the per-VMA lock and the
map_changing_lock.

Fixes: akpm:userfaultfd-retry-copying-with-locks-dropped-in-mfill_atomic_pte_copy.patch
Reported-by: syzbot+cf5ad2009a9bae03cb23@syzkaller.appspotmail.com
Signed-off-by: Andrei Vagin <avagin@google.com>
---
 mm/userfaultfd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 9ffc80d0a51b..04f9e21fecf1 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -224,6 +224,7 @@ static int mfill_get_vma(struct mfill_state *state)
 	 * request the user to retry later
 	 */
 	down_read(&ctx->map_changing_lock);
+	state->vma = dst_vma;
 	err = -EAGAIN;
 	if (atomic_read(&ctx->mmap_changing))
 		goto out_unlock;
@@ -246,7 +247,7 @@ static int mfill_get_vma(struct mfill_state *state)
 		goto out_unlock;
 
 	if (is_vm_hugetlb_page(dst_vma))
-		goto out;
+		return 0;
 
 	ops = vma_uffd_ops(dst_vma);
 	if (!ops)
@@ -256,8 +257,6 @@ static int mfill_get_vma(struct mfill_state *state)
 	    !ops->get_folio_noalloc)
 		goto out_unlock;
 
-out:
-	state->vma = dst_vma;
 	return 0;
 
 out_unlock:
-- 
2.53.0.851.ga537e3e6e9-goog