linux-kernel - Re: [PATCH v3 4/6] KVM: guest_memfd: add support for userfaultfd minor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dd851cab-eb22-40ae-b926-6f0eb1567299@amazon.com>
Date: Wed, 11 Jun 2025 13:09:36 +0100
From: Nikita Kalyazin <kalyazin@...zon.com>
To: Peter Xu <peterx@...hat.com>
CC: <akpm@...ux-foundation.org>, <pbonzini@...hat.com>, <shuah@...nel.org>,
	<viro@...iv.linux.org.uk>, <brauner@...nel.org>, <muchun.song@...ux.dev>,
	<hughd@...gle.com>, <kvm@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
	<linux-fsdevel@...r.kernel.org>, <jack@...e.cz>,
	<lorenzo.stoakes@...cle.com>, <Liam.Howlett@...cle.com>, <jannh@...gle.com>,
	<ryan.roberts@....com>, <david@...hat.com>, <jthoughton@...gle.com>,
	<graf@...zon.de>, <jgowans@...zon.com>, <roypat@...zon.co.uk>,
	<derekmn@...zon.com>, <nsaenz@...zon.es>, <xmarcalx@...zon.com>
Subject: Re: [PATCH v3 4/6] KVM: guest_memfd: add support for userfaultfd
 minor



On 10/06/2025 23:25, Peter Xu wrote:
> On Fri, Apr 04, 2025 at 03:43:50PM +0000, Nikita Kalyazin wrote:
>> Add support for sending a pagefault event if userfaultfd is registered.
>> Only page minor event is currently supported.
>>
>> Signed-off-by: Nikita Kalyazin <kalyazin@...zon.com>
>> ---
>>   virt/kvm/guest_memfd.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>> index fbf89e643add..096d89e7282d 100644
>> --- a/virt/kvm/guest_memfd.c
>> +++ b/virt/kvm/guest_memfd.c
>> @@ -4,6 +4,9 @@
>>   #include <linux/kvm_host.h>
>>   #include <linux/pagemap.h>
>>   #include <linux/anon_inodes.h>
>> +#ifdef CONFIG_KVM_PRIVATE_MEM
>> +#include <linux/userfaultfd_k.h>
>> +#endif /* CONFIG_KVM_PRIVATE_MEM */
>>
>>   #include "kvm_mm.h"
>>
>> @@ -380,6 +383,13 @@ static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
>>                kvm_gmem_mark_prepared(folio);
>>        }
>>
>> +     if (userfaultfd_minor(vmf->vma) &&
>> +         !(vmf->flags & FAULT_FLAG_USERFAULT_CONTINUE)) {
>> +             folio_unlock(folio);
>> +             filemap_invalidate_unlock_shared(inode->i_mapping);
>> +             return handle_userfault(vmf, VM_UFFD_MINOR);
>> +     }
>> +
> 
> Hmm, does guest-memfd (when with your current approach) at least needs to
> define the new can_userfault() hook?
> 
> Meanwhile, we have some hard-coded lines so far, like:
> 
> mfill_atomic():
>          if (!vma_is_shmem(dst_vma) &&
>              uffd_flags_mode_is(flags, MFILL_ATOMIC_CONTINUE))
>                  goto out_unlock;
> 
> I thought it would fail guest-memfd already on a CONTINUE request, and it
> doesn't seem to be touched yet in this series.
> 
> I'm not yet sure how the test worked out without hitting things like it.
> Highly likely I missed something.  Some explanations would be welcomed..

Yes, I realised that I'd failed to post this part soon after I sent the 
series, but I refrained from sending a new version because the upstream 
consensus was to review/merge the mmap support in guest_memfd [1] before 
continuing to build on top of it.  This is the missed part I planned to 
include in the next version.  Sorry for the confusion.

diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
index 64551e8a55fb..080437fa7eab 100644
--- a/include/linux/userfaultfd_k.h
+++ b/include/linux/userfaultfd_k.h
@@ -221,8 +221,10 @@ static inline bool vma_can_userfault(struct 
vm_area_struct *vma,
  	if (vm_flags & VM_DROPPABLE)
  		return false;

-	if (!vma->vm_ops->can_userfault ||
-	    !vma->vm_ops->can_userfault(vma, VM_UFFD_MINOR))
+	if ((vm_flags & VM_UFFD_MINOR) &&
+	     (!vma->vm_ops ||
+	      !vma->vm_ops->can_userfault ||
+	      !vma->vm_ops->can_userfault(vma, VM_UFFD_MINOR)))
  		return false;

  	/*
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 0aa82c968e16..638360a78561 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -788,7 +788,9 @@ static __always_inline ssize_t mfill_atomic(struct 
userfaultfd_ctx *ctx,
  		return  mfill_atomic_hugetlb(ctx, dst_vma, dst_start,
  					     src_start, len, flags);

-	can_userfault = dst_vma->vm_ops->can_userfault &&
+	can_userfault =
+	    dst_vma->vm_ops &&
+	    dst_vma->vm_ops->can_userfault &&
  	    dst_vma->vm_ops->can_userfault(dst_vma, __VM_UFFD_FLAGS);

  	if (!vma_is_anonymous(dst_vma) && !can_userfault)
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 91ee5dd91c31..202b12dc4b6f 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -420,8 +420,15 @@ static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
  	return ret;
  }

+static bool kvm_gmem_can_userfault(struct vm_area_struct *vma,
+				   unsigned long vm_flags)
+{
+	return vm_flags & VM_UFFD_MINOR;
+}
+
  static const struct vm_operations_struct kvm_gmem_vm_ops = {
-	.fault = kvm_gmem_fault,
+	.fault         = kvm_gmem_fault,
+	.can_userfault = kvm_gmem_can_userfault,
  };

  static int kvm_gmem_mmap(struct file *file, struct vm_area_struct *vma)


[1] https://lore.kernel.org/kvm/20250605153800.557144-1-tabba@google.com/

> 
> Thanks,
> 
>>        vmf->page = folio_file_page(folio, vmf->pgoff);
>>
>>   out_folio:
>> --
>> 2.47.1
>>
> 
> --
> Peter Xu
>