linux-kernel - Re: [PATCH 1/6] KVM: guest_memfd: Add DEFAULT_SHARED flag, reject user page faults if not set

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: <aN_2JaorgERIkpW4@google.com>
Date: Fri, 3 Oct 2025 09:13:25 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Vishal Annapurve <vannapurve@...gle.com>
Cc: Ackerley Tng <ackerleytng@...gle.com>, David Hildenbrand <david@...hat.com>, 
	Patrick Roy <patrick.roy@...ux.dev>, Fuad Tabba <tabba@...gle.com>, 
	Paolo Bonzini <pbonzini@...hat.com>, Christian Borntraeger <borntraeger@...ux.ibm.com>, 
	Janosch Frank <frankja@...ux.ibm.com>, Claudio Imbrenda <imbrenda@...ux.ibm.com>, 
	kvm list <kvm@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	Nikita Kalyazin <kalyazin@...zon.co.uk>, Shivank Garg <shivankg@....com>
Subject: Re: [PATCH 1/6] KVM: guest_memfd: Add DEFAULT_SHARED flag, reject
 user page faults if not set

On Thu, Oct 02, 2025, Vishal Annapurve wrote:
> On Thu, Oct 2, 2025, 5:12 PM Sean Christopherson <seanjc@...gle.com> wrote:
> >
> > > >
> > > > If the _only_ user-visible asset that is added is a KVM_CREATE_GUEST_MEMFD flag,
> > > > a CAP is gross overkill.  Even if there are other assets that accompany the new
> > > > flag, there's no reason we couldn't say "this feature exist if XYZ flag is
> > > > supported".
> > > >
> > > > E.g. it's functionally no different than KVM_CAP_VM_TYPES reporting support for
> > > > KVM_X86_TDX_VM also effectively reporting support for a _huge_ number of things
> > > > far beyond being able to create a VM of type KVM_X86_TDX_VM.
> > > >
> > >
> > > What's your opinion about having KVM_CAP_GUEST_MEMFD_MMAP part of
> > > KVM_CAP_GUEST_MEMFD_CAPS i.e. having a KVM cap covering all features
> > > of guest_memfd?
> >
> > I'd much prefer to have both.  Describing flags for an ioctl via a bitmask that
> > doesn't *exactly* match the flags is asking for problems.  At best, it will be
> > confusing.  E.g. we'll probably end up with code like this:
> >
> >         gmem_caps = kvm_check_cap(KVM_CAP_GUEST_MEMFD_CAPS);
> >
> >         if (gmem_caps & KVM_CAP_GUEST_MEMFD_MMAP)
> >                 gmem_flags |= GUEST_MEMFD_FLAG_MMAP;
> >         if (gmem_caps & KVM_CAP_GUEST_MEMFD_INIT_SHARED)
> >                 gmem_flags |= KVM_CAP_GUEST_MEMFD_INIT_SHARED;
> >
> 
> No, I actually meant the userspace can just rely on the cap to assume
> right flags to be available (not necessarily the same flags as cap
> bits).
> 
> i.e. Userspace will do something like:
> gmem_caps = kvm_check_cap(KVM_CAP_GUEST_MEMFD_CAPS);
> 
> if (gmem_caps & KVM_CAP_GUEST_MEMFD_MMAP)
>         gmem_flags |= GUEST_MEMFD_FLAG_MMAP;
> if (gmem_caps & KVM_CAP_GUEST_MEMFD_HUGETLB)
>         gmem_flags |= GUEST_MEMFD_FLAG_HUGETLB | GUEST_MEMFD_FLAG_HUGETLB_2MB;

Yes, that's exactly what I said.  But I goofed when copy+pasted and failed to
do s/KVM_CAP_GUEST_MEMFD_INIT_SHARED/GUEST_MEMFD_FLAG_INIT_SHARED, which is the
type of bug that ideally just can't happen.

Side topic, I'm not at all convinced that this is what we want for KVM's uAPI:

	if (gmem_caps & KVM_CAP_GUEST_MEMFD_HUGETLB)                                  
		gmem_flags |= GUEST_MEMFD_FLAG_HUGETLB | GUEST_MEMFD_FLAG_HUGETLB_2MB;

See https://lore.kernel.org/all/aN_fJEZXo6wkcHOh@google.com.

> Userspace has to anyways assume flag values, userspace just needs to
> know if a particular feature is available.

I don't understand what you mean by "assume flag values".