All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: sashiko-reviews@lists.linux.dev
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH v2 4/4] KVM: guest_memfd: Drop superfluous WRITE_ONCE() when binding a memslot
Date: Wed, 2 Sep 2026 12:00:27 -0700	[thread overview]
Message-ID: <aphySzgAce0I1Cox@google.com> (raw)
In-Reply-To: <20260902183634.9E9711F000E9@smtp.kernel.org>

On Wed, Sep 02, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Modifying the guest_memfd xarray outside of filemap_invalidate_lock on the error path causes a Use-After-Free and permanent MMU invalidation lockup.
> --
> 
> commit c377e9963148427885842d70319259945c649a8a
> Author: Sean Christopherson <seanjc@google.com>
> 
> KVM: guest_memfd: Drop superfluous WRITE_ONCE() when binding a memslot
> 
> Drop the superfluous WRITE_ONCE() when setting a memslot's guest_memfd file
> during initial binding, as the memslot *must* be inactive and unreachable.
> The superfluous WRITE_ONCE() was added by commit 67b43038ce14 to
> maintain rough "parity" with the existing rcu_assign_pointer().
> 
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index 2c8d8735de5fe..323159c9b5f34 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -653,7 +653,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
> >  	 * kvm_gmem_bind() must occur on a new memslot.  Because the memslot
> >  	 * is not visible yet, kvm_gmem_get_pfn() is guaranteed to see the file.
> >  	 */
> > -	WRITE_ONCE(slot->gmem.file, file);
> > +	slot->gmem.file = file;
> >  	slot->gmem.pgoff = start;
> >  	if (kvm_gmem_supports_mmap(inode))
> >  		slot->flags |= KVM_MEMSLOT_GMEM_ONLY;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but modifying the xarray outside
> the filemap_invalidate_lock on the error path further down in this function
> appears to cause a use-after-free and a permanent MMU invalidation lockup.
> 
> If xa_store_range() encounters -ENOMEM during the store, it can abort and
> leave the slot partially inserted in the tree. kvm_gmem_bind() then explicitly
> drops filemap_invalidate_lock before executing its error path. 

LOL, I think I get the point...

      reply	other threads:[~2026-09-02 19:00 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-02 18:20 [PATCH v2 0/4] KVM: guest_memfd: Fix binding bugs Sean Christopherson
2026-09-02 18:20 ` [PATCH v2 1/4] KVM: guest_memfd: Gracefully handle xarray errors when binding a memslot Sean Christopherson
2026-09-02 18:32   ` sashiko-bot
2026-09-02 18:20 ` [PATCH v2 2/4] KVM: Use goto to handle errors during memslot preparation Sean Christopherson
2026-09-02 18:20 ` [PATCH v2 3/4] KVM: guest_memfd: Establish memslot<=>guest_memfd bindings *after* memslot is ready Sean Christopherson
2026-09-02 18:35   ` sashiko-bot
2026-09-02 18:20 ` [PATCH v2 4/4] KVM: guest_memfd: Drop superfluous WRITE_ONCE() when binding a memslot Sean Christopherson
2026-09-02 18:36   ` sashiko-bot
2026-09-02 19:00     ` Sean Christopherson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aphySzgAce0I1Cox@google.com \
    --to=seanjc@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.