| 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
| ||
|
Message-ID: <diqzbjmiekrn.fsf@google.com>
Date: Tue, 07 Oct 2025 09:31:24 -0700
From: Ackerley Tng <ackerleytng@...gle.com>
To: Sean Christopherson <seanjc@...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>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
David Hildenbrand <david@...hat.com>, Fuad Tabba <tabba@...gle.com>
Subject: Re: [PATCH v2 03/13] KVM: guest_memfd: Invalidate SHARED GPAs if gmem
supports INIT_SHARED
Sean Christopherson <seanjc@...gle.com> writes:
> When invalidating gmem ranges, e.g. in response to PUNCH_HOLE, process all
> possible range types (PRIVATE vs. SHARED) for the gmem instance. Since
> since guest_memfd doesn't yet support in-place conversions, simply pivot
> on INIT_SHARED as a gmem instance can currently only have private or shared
> memory, not both.
>
> Failure to mark shared GPAs for invalidation is benign in the current code
> base, as only x86's TDX consumes KVM_FILTER_{PRIVATE,SHARED}, and TDX
> doesn't yet support INIT_SHARED with guest_memfd.
This is the correct fix, and I agree it is not a problem in current code
since before this patch series and the introduction of INIT_SHARED,
mmap() was only supported by non-CoCo, which doesn't interpret
KVM_FILTER_{PRIVATE,SHARED} anyway.
Had something similar/related here [1]
[1] https://lore.kernel.org/all/d3832fd95a03aad562705872cbda5b3d248ca321.1747264138.git.ackerleytng@google.com/
Reviewed-by: Ackerley Tng <ackerleytng@...gle.com>
> However, invalidating
> only private GPAs is conceptually wrong and a lurking bug, e.g. could
> result in missed invalidations if ARM starts filtering invalidations based
> on attributes.
>
> Fixes: 3d3a04fad25a ("KVM: Allow and advertise support for host mmap() on guest_memfd files")
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
> virt/kvm/guest_memfd.c | 64 +++++++++++++++++++++++++++++-------------
> 1 file changed, 44 insertions(+), 20 deletions(-)
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index cf3afba23a6b..e10d2c71e78c 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -102,8 +102,17 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
> return filemap_grab_folio(inode->i_mapping, index);
> }
>
> -static void kvm_gmem_invalidate_begin(struct kvm_gmem *gmem, pgoff_t start,
> - pgoff_t end)
> +static enum kvm_gfn_range_filter kvm_gmem_get_invalidate_filter(struct inode *inode)
> +{
> + if ((u64)inode->i_private & GUEST_MEMFD_FLAG_INIT_SHARED)
> + return KVM_FILTER_SHARED;
> +
> + return KVM_FILTER_PRIVATE;
> +}
> +
> +static void __kvm_gmem_invalidate_begin(struct kvm_gmem *gmem, pgoff_t start,
> + pgoff_t end,
> + enum kvm_gfn_range_filter attr_filter)
> {
> bool flush = false, found_memslot = false;
> struct kvm_memory_slot *slot;
> @@ -118,8 +127,7 @@ static void kvm_gmem_invalidate_begin(struct kvm_gmem *gmem, pgoff_t start,
> .end = slot->base_gfn + min(pgoff + slot->npages, end) - pgoff,
> .slot = slot,
> .may_block = true,
> - /* guest memfd is relevant to only private mappings. */
> - .attr_filter = KVM_FILTER_PRIVATE,
> + .attr_filter = attr_filter,
> };
>
> if (!found_memslot) {
> @@ -139,8 +147,21 @@ static void kvm_gmem_invalidate_begin(struct kvm_gmem *gmem, pgoff_t start,
> KVM_MMU_UNLOCK(kvm);
> }
>
> -static void kvm_gmem_invalidate_end(struct kvm_gmem *gmem, pgoff_t start,
> - pgoff_t end)
> +static void kvm_gmem_invalidate_begin(struct inode *inode, pgoff_t start,
> + pgoff_t end)
> +{
> + struct list_head *gmem_list = &inode->i_mapping->i_private_list;
> + enum kvm_gfn_range_filter attr_filter;
> + struct kvm_gmem *gmem;
> +
> + attr_filter = kvm_gmem_get_invalidate_filter(inode);
> +
> + list_for_each_entry(gmem, gmem_list, entry)
> + __kvm_gmem_invalidate_begin(gmem, start, end, attr_filter);
> +}
> +
> +static void __kvm_gmem_invalidate_end(struct kvm_gmem *gmem, pgoff_t start,
> + pgoff_t end)
> {
> struct kvm *kvm = gmem->kvm;
>
> @@ -151,12 +172,20 @@ static void kvm_gmem_invalidate_end(struct kvm_gmem *gmem, pgoff_t start,
> }
> }
>
> +static void kvm_gmem_invalidate_end(struct inode *inode, pgoff_t start,
> + pgoff_t end)
> +{
> + struct list_head *gmem_list = &inode->i_mapping->i_private_list;
> + struct kvm_gmem *gmem;
> +
> + list_for_each_entry(gmem, gmem_list, entry)
> + __kvm_gmem_invalidate_end(gmem, start, end);
> +}
> +
> static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
> {
> - struct list_head *gmem_list = &inode->i_mapping->i_private_list;
> pgoff_t start = offset >> PAGE_SHIFT;
> pgoff_t end = (offset + len) >> PAGE_SHIFT;
> - struct kvm_gmem *gmem;
>
> /*
> * Bindings must be stable across invalidation to ensure the start+end
> @@ -164,13 +193,11 @@ static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
> */
> filemap_invalidate_lock(inode->i_mapping);
>
> - list_for_each_entry(gmem, gmem_list, entry)
> - kvm_gmem_invalidate_begin(gmem, start, end);
> + kvm_gmem_invalidate_begin(inode, start, end);
>
> truncate_inode_pages_range(inode->i_mapping, offset, offset + len - 1);
>
> - list_for_each_entry(gmem, gmem_list, entry)
> - kvm_gmem_invalidate_end(gmem, start, end);
> + kvm_gmem_invalidate_end(inode, start, end);
>
> filemap_invalidate_unlock(inode->i_mapping);
>
> @@ -280,8 +307,9 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
> * Zap all SPTEs pointed at by this file. Do not free the backing
> * memory, as its lifetime is associated with the inode, not the file.
> */
> - kvm_gmem_invalidate_begin(gmem, 0, -1ul);
> - kvm_gmem_invalidate_end(gmem, 0, -1ul);
> + __kvm_gmem_invalidate_begin(gmem, 0, -1ul,
> + kvm_gmem_get_invalidate_filter(inode));
> + __kvm_gmem_invalidate_end(gmem, 0, -1ul);
>
> list_del(&gmem->entry);
>
> @@ -403,8 +431,6 @@ static int kvm_gmem_migrate_folio(struct address_space *mapping,
>
> static int kvm_gmem_error_folio(struct address_space *mapping, struct folio *folio)
> {
> - struct list_head *gmem_list = &mapping->i_private_list;
> - struct kvm_gmem *gmem;
> pgoff_t start, end;
>
> filemap_invalidate_lock_shared(mapping);
> @@ -412,8 +438,7 @@ static int kvm_gmem_error_folio(struct address_space *mapping, struct folio *fol
> start = folio->index;
> end = start + folio_nr_pages(folio);
>
> - list_for_each_entry(gmem, gmem_list, entry)
> - kvm_gmem_invalidate_begin(gmem, start, end);
> + kvm_gmem_invalidate_begin(mapping->host, start, end);
>
> /*
> * Do not truncate the range, what action is taken in response to the
> @@ -424,8 +449,7 @@ static int kvm_gmem_error_folio(struct address_space *mapping, struct folio *fol
> * error to userspace.
> */
>
> - list_for_each_entry(gmem, gmem_list, entry)
> - kvm_gmem_invalidate_end(gmem, start, end);
> + kvm_gmem_invalidate_end(mapping->host, start, end);
>
> filemap_invalidate_unlock_shared(mapping);
>
> --
> 2.51.0.618.g983fd99d29-goog
Powered by blists - more mailing lists