Skip to content

Commit 0bb4d9c

Browse files
committed
KVM: guest_memfd: Define a CLASS to get+put guest_memfd file from a memslot
Add a CLASS to handle getting and putting a guest_memfd file given a memslot to reduce the amount of related boilerplate, and more importantly to minimize the chances of forgetting to put the file (thankfully the bug that prompted this didn't escape initial testing). Define a CLASS instead of using __free(fput) as _free() comes with subtle caveats related to FILO ordering (objects are freed in the order in which they are declared), and the recommended solution/workaround (declare file pointers exactly when they are initialized) is visually jarring relative to KVM's (and the kernel's) overall strict adherence to not mixing declarations and code. E.g. the use in kvm_gmem_populate() would be: slot = gfn_to_memslot(kvm, start_gfn); if (!kvm_slot_has_gmem(slot)) return -EINVAL; struct file *file __free(fput) = kvm_gmem_get_file(slot; if (!file) return -EFAULT; filemap_invalidate_lock(file->f_mapping); Note, using CLASS() still declares variables in the middle of code, but the syntactic sugar obfuscates the declaration, i.e. hides the anomaly to a large extent. No functional change intended. Link: https://lore.kernel.org/r/20251007222356.348349-1-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent e66438b commit 0bb4d9c

File tree

1 file changed

+8
-14
lines changed

1 file changed

+8
-14
lines changed

virt/kvm/guest_memfd.c

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,9 @@ static inline struct file *kvm_gmem_get_file(struct kvm_memory_slot *slot)
385385
return get_file_active(&slot->gmem.file);
386386
}
387387

388+
DEFINE_CLASS(gmem_get_file, struct file *, if (_T) fput(_T),
389+
kvm_gmem_get_file(slot), struct kvm_memory_slot *slot);
390+
388391
static bool kvm_gmem_supports_mmap(struct inode *inode)
389392
{
390393
return GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_MMAP;
@@ -710,13 +713,12 @@ void kvm_gmem_unbind(struct kvm_memory_slot *slot)
710713
unsigned long start = slot->gmem.pgoff;
711714
unsigned long end = start + slot->npages;
712715
struct gmem_file *f;
713-
struct file *file;
714716

715717
/*
716718
* Nothing to do if the underlying file was already closed (or is being
717719
* closed right now), kvm_gmem_release() invalidates all bindings.
718720
*/
719-
file = kvm_gmem_get_file(slot);
721+
CLASS(gmem_get_file, file)(slot);
720722
if (!file)
721723
return;
722724

@@ -731,8 +733,6 @@ void kvm_gmem_unbind(struct kvm_memory_slot *slot)
731733
*/
732734
WRITE_ONCE(slot->gmem.file, NULL);
733735
filemap_invalidate_unlock(file->f_mapping);
734-
735-
fput(file);
736736
}
737737

738738
/* Returns a locked folio on success. */
@@ -778,19 +778,17 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
778778
int *max_order)
779779
{
780780
pgoff_t index = kvm_gmem_get_index(slot, gfn);
781-
struct file *file = kvm_gmem_get_file(slot);
782781
struct folio *folio;
783782
bool is_prepared = false;
784783
int r = 0;
785784

785+
CLASS(gmem_get_file, file)(slot);
786786
if (!file)
787787
return -EFAULT;
788788

789789
folio = __kvm_gmem_get_pfn(file, slot, index, pfn, &is_prepared, max_order);
790-
if (IS_ERR(folio)) {
791-
r = PTR_ERR(folio);
792-
goto out;
793-
}
790+
if (IS_ERR(folio))
791+
return PTR_ERR(folio);
794792

795793
if (!is_prepared)
796794
r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio);
@@ -802,8 +800,6 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
802800
else
803801
folio_put(folio);
804802

805-
out:
806-
fput(file);
807803
return r;
808804
}
809805
EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_get_pfn);
@@ -812,7 +808,6 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_get_pfn);
812808
long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long npages,
813809
kvm_gmem_populate_cb post_populate, void *opaque)
814810
{
815-
struct file *file;
816811
struct kvm_memory_slot *slot;
817812
void __user *p;
818813

@@ -828,7 +823,7 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
828823
if (!kvm_slot_has_gmem(slot))
829824
return -EINVAL;
830825

831-
file = kvm_gmem_get_file(slot);
826+
CLASS(gmem_get_file, file)(slot);
832827
if (!file)
833828
return -EFAULT;
834829

@@ -886,7 +881,6 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
886881

887882
filemap_invalidate_unlock(file->f_mapping);
888883

889-
fput(file);
890884
return ret && !i ? ret : i;
891885
}
892886
EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_populate);

0 commit comments

Comments
 (0)