Re: [PATCH v2] KVM: guest_memfd: Elaborate on how release() vs. get_pfn() is safe against UAF

[Yan Zhao]

On Tue, Sep 22, 2026 at 07:22:36AM -0700, Sean Christopherson wrote:
> And the whole thing as a diff:
LGTM.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 9a112daa5df9..483998f0df58 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -312,7 +312,7 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
>  	 * synchronize_srcu_expedited() in kvm_swap_active_memslots() to ensure
>  	 * kvm_gmem_get_pfn() (which runs with kvm->srcu held for read) can't
>  	 * grab a reference to slot->gmem.file even if the struct file object
> -	 * is reallocated.
> +	 * is reallocated (for use as a different file).
>  	 *
>  	 * file_ref_put() provides a full barrier, and __get_file_rcu() the
>  	 * matching acquire barrier, to ensure that kvm_gmem_get_file() (via
> @@ -327,7 +327,7 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
>  	 *
>  	 *  CPU0				CPU1
>  	 *  kvm_gmem_get_pfn()
> -	 *    f = X (from slot->gmem.file)
> +	 *    slot->gmem.file == A
>  	 *					kvm_gmem_release())
>  	 *					  slot->gmem.file = NULL
>  	 *
> @@ -336,18 +336,19 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
>  	 *
>  	 *					kvm_set_memory_region()
>  	 *					  slot created
> -	 *					  slot->gmem.file = f (alloc the same object)
> +	 *					  slot->gmem.file = B (alloc the same object)
>  	 *
>  	 *  get_file_active()
> -	 *    file = f
> -	 *    file_reloaded = f
> +	 *    file = B
> +	 *    file_reloaded = B
>  	 *
>  	 * <KVM does weird things with an old memslot+file>
>  	 *
>  	 * Obviously KVM would be broken in many places if the synchronization
>  	 * were omitted, but it's important to note that get_file_active() does
> -	 * NOT guarantee a reference to the correct file was obtained, only
> -	 * that the file doesn't point at a reallocated object.
> +	 * NOT guarantee a reference to the "original" file was obtained, only
> +	 * that it grabbed a reference for the returned file, i.e. didn't grab
> +	 * a reference for file A, but then returned a pointer to file B.
>  	 */
>  	xa_for_each(&f->bindings, index, slot)
>  		WRITE_ONCE(slot->gmem.file, NULL);