Re: [PATCH 2/3] userfaultfd: prevent unregistering VMAs through a different userfaultfd

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04.06.25 00:14, Tal Zussman wrote:
Currently, a VMA registered with a uffd can be unregistered through a
different uffd asssociated with the same mm_struct.

Change this behavior to be stricter by requiring VMAs to be unregistered
through the same uffd they were registered with.

While at it, correct the comment for the no userfaultfd case. This seems
to be a copy-paste artifact from the analagous userfaultfd_register()
check.

I consider it a BUG that should be fixed. Hoping Peter can share his opinion.


Fixes: 86039bd3b4e6 ("userfaultfd: add new syscall to provide memory externalization")
Signed-off-by: Tal Zussman <tz2294@xxxxxxxxxxxx>
---
  fs/userfaultfd.c | 15 +++++++++++++--
  1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 22f4bf956ba1..9289e30b24c4 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1477,6 +1477,16 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
  		if (!vma_can_userfault(cur, cur->vm_flags, wp_async))
  			goto out_unlock;
+ /*
+		 * Check that this vma isn't already owned by a different
+		 * userfaultfd. This provides for more strict behavior by
+		 * preventing a VMA registered with a userfaultfd from being
+		 * unregistered through a different userfaultfd.
+		 */
+		if (cur->vm_userfaultfd_ctx.ctx &&
+		    cur->vm_userfaultfd_ctx.ctx != ctx)
+			goto out_unlock;

So we allow !cur->vm_userfaultfd_ctx.ctx to allow unregistering when there was nothing registered.

A bit weird to set "found = true" in that case. Maybe it's fine, just raising it ...

+
  		found = true;
  	} for_each_vma_range(vmi, cur, end);
  	BUG_ON(!found);
@@ -1491,10 +1501,11 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
  		cond_resched();
BUG_ON(!vma_can_userfault(vma, vma->vm_flags, wp_async));
+		BUG_ON(vma->vm_userfaultfd_ctx.ctx &&
+		       vma->vm_userfaultfd_ctx.ctx != ctx);

No new BUG_ON please. VM_WARN_ON_ONCE() if we really care. After all, we checked this above ...

  		/*
-		 * Nothing to do: this vma is already registered into this
-		 * userfaultfd and with the right tracking mode too.
+		 * Nothing to do: this vma is not registered with userfaultfd.
  		 */
  		if (!vma->vm_userfaultfd_ctx.ctx)
  			goto skip;



--
Cheers,

David / dhildenb





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux