linux-kernel - Re: [RFC PATCH 3/4] userfaultfd, guest_memfd: support userfault minor mode in guest

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] [day] [month] [year] [list]

Message-ID: <aSBUCcjmZ5PM61ir@kernel.org>
Date: Fri, 21 Nov 2025 13:59:05 +0200
From: Mike Rapoport <rppt@...nel.org>
To: "David Hildenbrand (Red Hat)" <david@...nel.org>
Cc: linux-mm@...ck.org, Andrea Arcangeli <aarcange@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Baolin Wang <baolin.wang@...ux.alibaba.com>,
	Hugh Dickins <hughd@...gle.com>,
	"Liam R. Howlett" <Liam.Howlett@...cle.com>,
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
	Michal Hocko <mhocko@...e.com>,
	Nikita Kalyazin <kalyazin@...zon.com>,
	Paolo Bonzini <pbonzini@...hat.com>, Peter Xu <peterx@...hat.com>,
	Sean Christopherson <seanjc@...gle.com>,
	Shuah Khan <shuah@...nel.org>,
	Suren Baghdasaryan <surenb@...gle.com>,
	Vlastimil Babka <vbabka@...e.cz>, linux-kernel@...r.kernel.org,
	kvm@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [RFC PATCH 3/4] userfaultfd, guest_memfd: support userfault
 minor mode in guest_memfd

On Tue, Nov 18, 2025 at 05:41:13PM +0100, David Hildenbrand (Red Hat) wrote:
> On 17.11.25 12:46, Mike Rapoport wrote:
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index fbca8c0972da..5e3c63307fdf 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -4,6 +4,7 @@
> >   #include <linux/kvm_host.h>
> >   #include <linux/pagemap.h>
> >   #include <linux/anon_inodes.h>
> > +#include <linux/userfaultfd_k.h>
> >   #include "kvm_mm.h"
> > @@ -369,6 +370,12 @@ static vm_fault_t kvm_gmem_fault_user_mapping(struct vm_fault *vmf)
> >   		return vmf_error(err);
> >   	}
> > +	if (userfaultfd_minor(vmf->vma)) {
> > +		folio_unlock(folio);
> > +		folio_put(folio);
> > +		return handle_userfault(vmf, VM_UFFD_MINOR);
> > +	}
> 
> Staring at things like VM_FAULT_NEEDDSYNC, I'm wondering whether we could have a
> new return value from ->fault that would indicate that
> handle_userfault(vmf, VM_UFFD_MINOR) should be called.
> 
> Maybe some VM_FAULT_UFFD_MINOR or simply VM_FAULT_USERFAULTFD and we
> can just derive that it is VM_UFFD_MINOR.

_UFFD_MINOR sounds better, maybe we'll want something for missing later on. 
 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 4f66a3206a63c..2cf17da880f0e 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -1601,6 +1601,8 @@ typedef __bitwise unsigned int vm_fault_t;
>   *                             fsync() to complete (for synchronous page faults
>   *                             in DAX)
>   * @VM_FAULT_COMPLETED:                ->fault completed, meanwhile mmap lock released
> + * @VM_FAULT_USERFAULTFD:      ->fault did not modify page tables and needs
> + *                             handle_userfault() to complete
>   * @VM_FAULT_HINDEX_MASK:      mask HINDEX value
>   *
>   */
> @@ -1618,6 +1620,7 @@ enum vm_fault_reason {
>         VM_FAULT_DONE_COW       = (__force vm_fault_t)0x001000,
>         VM_FAULT_NEEDDSYNC      = (__force vm_fault_t)0x002000,
>         VM_FAULT_COMPLETED      = (__force vm_fault_t)0x004000,
> +       VM_FAULT_USERFAULTFD    = (__force vm_fault_t)0x006000,
>         VM_FAULT_HINDEX_MASK    = (__force vm_fault_t)0x0f0000,
>  };
> @@ -1642,6 +1645,7 @@ enum vm_fault_reason {
>         { VM_FAULT_FALLBACK,            "FALLBACK" },   \
>         { VM_FAULT_DONE_COW,            "DONE_COW" },   \
>         { VM_FAULT_NEEDDSYNC,           "NEEDDSYNC" },  \
> +       { VM_FAULT_USERFAULTFD,         "USERFAULTFD" },\
>         { VM_FAULT_COMPLETED,           "COMPLETED" }
>  struct vm_special_mapping {
> 
> 
> IIUC, we have exactly two invocations of ->fault(vmf) in memory.c where
> we would have to handle it IIUC. And the return value would never leave
> the core.

I've found only one :/
But nevertheless, I like the idea to return VM_FAULT_UFFD_MINOR from
->fault() and then call handle_userfault() from __do_fault().
 
> That way, we wouldn't have to export handle_userfault().
> 
> Just a thought ...
> 
> -- 
> Cheers
> 
> David

-- 
Sincerely yours,
Mike.