[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250619-uffd-fixes-v3-2-a7274d3bd5e4@columbia.edu>
Date: Thu, 19 Jun 2025 21:24:24 -0400
From: Tal Zussman <tz2294@...umbia.edu>
To: Andrew Morton <akpm@...ux-foundation.org>, Peter Xu <peterx@...hat.com>,
"Jason A. Donenfeld" <Jason@...c4.com>,
David Hildenbrand <david@...hat.com>,
Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
Andrea Arcangeli <aarcange@...hat.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
linux-fsdevel@...r.kernel.org, Tal Zussman <tz2294@...umbia.edu>
Subject: [PATCH v3 2/4] userfaultfd: prevent unregistering VMAs through a
different userfaultfd
Currently, a VMA registered with a uffd can be unregistered through a
different uffd associated with the same mm_struct.
The existing behavior is slightly broken and may incorrectly reject
unregistering some VMAs due to the following check:
if (!vma_can_userfault(cur, cur->vm_flags, wp_async))
goto out_unlock;
where wp_async is derived from ctx, not from cur. For example, a
file-backed VMA registered with wp_async enabled and UFFD_WP mode cannot
be unregistered through a uffd that does not have wp_async enabled.
Rather than fix this and maintain this odd behavior, make unregistration
stricter by requiring VMAs to be unregistered through the same uffd they
were registered with. Additionally, reorder the BUG() checks to avoid
the aforementioned wp_async issue in them. Convert the existing check to
VM_WARN_ON_ONCE() as BUG_ON() is deprecated.
This change slightly modifies the ABI. It should not be backported to
-stable. It is expected that no one depends on this behavior, and no
such cases are known.
While at it, correct the comment for the no userfaultfd case. This seems to
be a copy-paste artifact from the analogous userfaultfd_register() check.
Fixes: 86039bd3b4e6 ("userfaultfd: add new syscall to provide memory externalization")
Acked-by: David Hildenbrand <david@...hat.com>
Signed-off-by: Tal Zussman <tz2294@...umbia.edu>
---
fs/userfaultfd.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 22f4bf956ba1..8e7fb2a7a6aa 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1467,6 +1467,14 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
BUG_ON(!!cur->vm_userfaultfd_ctx.ctx ^
!!(cur->vm_flags & __VM_UFFD_FLAGS));
+ /*
+ * Prevent unregistering through a different userfaultfd than
+ * the one used for registration.
+ */
+ if (cur->vm_userfaultfd_ctx.ctx &&
+ cur->vm_userfaultfd_ctx.ctx != ctx)
+ goto out_unlock;
+
/*
* Check not compatible vmas, not strictly required
* here as not compatible vmas cannot have an
@@ -1490,15 +1498,12 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
for_each_vma_range(vmi, vma, end) {
cond_resched();
- BUG_ON(!vma_can_userfault(vma, vma->vm_flags, wp_async));
-
- /*
- * Nothing to do: this vma is already registered into this
- * userfaultfd and with the right tracking mode too.
- */
+ /* VMA not registered with userfaultfd. */
if (!vma->vm_userfaultfd_ctx.ctx)
goto skip;
+ VM_WARN_ON_ONCE(vma->vm_userfaultfd_ctx.ctx != ctx);
+ VM_WARN_ON_ONCE(!vma_can_userfault(vma, vma->vm_flags, wp_async));
WARN_ON(!(vma->vm_flags & VM_MAYWRITE));
if (vma->vm_start > start)
--
2.39.5
Powered by blists - more mailing lists