Re: [PATCH v4 13/15] KVM: guest_memfd: implement userfaultfd operations

[Mike Rapoport <rppt@kernel.org>]

Hi Sean,

On Thu, Apr 02, 2026 at 03:05:07PM -0700, Sean Christopherson wrote:
> On Thu, Apr 02, 2026, Mike Rapoport wrote:
>
> > +#ifdef CONFIG_USERFAULTFD
> > +static bool kvm_gmem_can_userfault(struct vm_area_struct *vma, vm_flags_t vm_flags)
> > +{
> > +	struct inode *inode = file_inode(vma->vm_file);
> > +
> > +	/*
> > +	 * Only support userfaultfd for guest_memfd with INIT_SHARED flag.
> > +	 * This ensures the memory can be mapped to userspace.
> > +	 */
> > +	if (!(GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_INIT_SHARED))
> > +		return false;
> 
> I'm not comfortable with this change.  It works for now, but it's going to be
> wildly wrong when in-place conversion comes along.  While I agree with the "Let's
> solve each problem in it's time :)"[*], the time for in-place conversion is now.
> In-place conversion isn't landing this cycle or next, but it's been in development
> for longer than UFFD support, and I'm not willing to punt solvable problems to
> that series, because it's plenty fat as is.

I'm not against solving it as a part of uffd support, but since we are very
close to the merge window, for now I asked Andrew to drop drop guest_memfd
patches from the set and only move forward with the refactoring of uffd and
shmem that has value on its own.
 
> Happily, IIUC, this is an easy problem to solve, and will have a nice side effect
> for the common UFFD code.
> 
> My objection to an early, global "can_userfault()" check is that it's guaranteed
> to cause TOCTOU issues.  E.g. for VM_UFFD_MISSING and VM_UFFD_MINOR, the check on
> whether or not a given address can be faulted in needs to happen in __do_userfault(),
> not broadly when VM_UFFD_MINOR is added to a VMA.  Conceptually, that also better
> aligns the code with the "normal" user fault path in kvm_gmem_fault_user_mapping().
>
> diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> index 6f33307c2780..8a2d0625ffa3 100644
> --- a/include/linux/userfaultfd_k.h
> +++ b/include/linux/userfaultfd_k.h
> @@ -82,8 +82,8 @@ extern vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason);
>  
>  /* VMA userfaultfd operations */
>  struct vm_uffd_ops {
> -       /* Checks if a VMA can support userfaultfd */
> -       bool (*can_userfault)(struct vm_area_struct *vma, vm_flags_t vm_flags);
> +       /* What UFFD flags/modes are supported. */
> +       const vm_flags_t supported_uffd_flags;

VMA maintainers really didn't like a fields flag in vm_uffd_ops when it was
proposed earlier, but an indirect call may work.

>         /*
>          * Called to resolve UFFDIO_CONTINUE request.
>          * Should return the folio found at pgoff in the VMA's pagecache if it
> 
> with usage like:
> 
> static const struct vm_uffd_ops shmem_uffd_ops = {
> 	.supported_uffd_flags	= __VM_UFFD_FLAGS,
> 	.get_folio_noalloc	= shmem_get_folio_noalloc,
> 	.alloc_folio		= shmem_mfill_folio_alloc,
> 	.filemap_add		= shmem_mfill_filemap_add,
> 	.filemap_remove		= shmem_mfill_filemap_remove,
> };
> 
> All in all, somelike like so (completely untested):
> 
> ---
>  include/linux/userfaultfd_k.h |  4 +-
>  mm/filemap.c                  |  1 +
>  mm/hugetlb.c                  |  8 +---
>  mm/shmem.c                    |  7 +--
>  mm/userfaultfd.c              |  6 +--
>  virt/kvm/guest_memfd.c        | 80 ++++++++++++++++++++++++++++++++++-
>  6 files changed, 87 insertions(+), 19 deletions(-)

Let's revisit after -rc1 :)

-- 
Sincerely yours,
Mike.