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 for Android: free password hash cracker in your pocket

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

Message-ID: <aNshILzpjAS-bUL5@google.com>
Date: Mon, 29 Sep 2025 17:15:28 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Ackerley Tng <ackerleytng@...gle.com>
Cc: 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@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Nikita Kalyazin <kalyazin@...zon.co.uk>, shivankg@....com
Subject: Re: [PATCH 1/6] KVM: guest_memfd: Add DEFAULT_SHARED flag, reject
 user page faults if not set

On Mon, Sep 29, 2025, Sean Christopherson wrote:
> On Mon, Sep 29, 2025, Ackerley Tng wrote:
> > David Hildenbrand <david@...hat.com> writes:
> > 
> > >                           GUEST_MEMFD_FLAG_DEFAULT_SHARED;
> > >>>>
> > >>>> At least for now, GUEST_MEMFD_FLAG_DEFAULT_SHARED and
> > >>>> GUEST_MEMFD_FLAG_MMAP don't make sense without each other. Is it worth
> > >>>> checking for that, at least until we have in-place conversion? Having
> > >>>> only GUEST_MEMFD_FLAG_DEFAULT_SHARED set, but GUEST_MEMFD_FLAG_MMAP,
> > >>>> isn't a useful combination.
> > >>>>
> > >>>
> > >>> I think it's okay to have the two flags be orthogonal from the start.
> > >> 
> > >> I think I dimly remember someone at one of the guest_memfd syncs
> > >> bringing up a usecase for having a VMA even if all memory is private,
> > >> not for faulting anything in, but to do madvise or something? Maybe it
> > >> was the NUMA stuff? (+Shivank)
> > >
> > > Yes, that should be it. But we're never faulting in these pages, we only 
> > > need the VMA (for the time being, until there is the in-place conversion).
> > >
> > 
> > Yup, Sean's patch disables faulting if GUEST_MEMFD_FLAG_DEFAULT_SHARED
> > is not set, but mmap() is always enabled so madvise() still works.
> 
> Hah!  I totally intended that :-D
> 
> > Requiring GUEST_MEMFD_FLAG_DEFAULT_SHARED to be set together with
> > GUEST_MEMFD_FLAG_MMAP would still allow madvise() to work since
> > GUEST_MEMFD_FLAG_DEFAULT_SHARED only gates faulting.
> > 
> > To clarify, I'm still for making GUEST_MEMFD_FLAG_DEFAULT_SHARED
> > orthogonal to GUEST_MEMFD_FLAG_MMAP with no additional checks on top of
> > whatever's in this patch. :)

Oh!  This got me looking at kvm_arch_supports_gmem_mmap() and thus
KVM_CAP_GUEST_MEMFD_MMAP.  Two things:

 1. We should change KVM_CAP_GUEST_MEMFD_MMAP into KVM_CAP_GUEST_MEMFD_FLAGS so
    that we don't need to add a capability every time a new flag comes along,
    and so that userspace can gather all flags in a single ioctl.  If gmem ever
    supports more than 32 flags, we'll need KVM_CAP_GUEST_MEMFD_FLAGS2, but
    that's a non-issue relatively speaking.

 2. We should allow mmap() for x86 CoCo VMs right away.  As evidenced by this
    series, mmap() on private memory is totally fine.  It's not useful until the
    NUMA and/or in-place conversion support comes along, but's not dangerous in
    any way.  The actual restriction is on initializing memory to be shared,
    because allowing memory to be shared from gmem's perspective while it's
    private from the VM's perspective would be all kinds of broken.


E.g. with a s/kvm_arch_supports_gmem_mmap/kvm_arch_supports_gmem_init_shared:

	case KVM_CAP_GUEST_MEMFD_FLAGS:
		if (!kvm || kvm_arch_supports_init_shared(kvm))
			return GUEST_MEMFD_FLAG_MMAP |
			       GUEST_MEMFD_FLAG_INIT_SHARED;

		return GUEST_MEMFD_FLAG_MMAP;

#2 is also a good reason to add INIT_SHARED straightaway.  Without INIT_SHARED,
we'd have to INIT_PRIVATE to make the NUMA support useful for x86 CoCo VMs, i.e.
it's not just in-place conversion that's affected, IIUC.

I'll add this in v2.