Re: [syzbot] [kvm?] KASAN: slab-use-after-free Write in kvm_gmem_release

[Sean Christopherson <seanjc@google.com>]

On Thu, Oct 23, 2025, syzbot wrote:
> syzbot has bisected this issue to:
> 
> commit d1e54dd08f163a9021433020d16a8f8f70ddc41c
> Author: Fuad Tabba <tabba@google.com>
> Date:   Tue Jul 29 22:54:40 2025 +0000
> 
>     KVM: x86: Enable KVM_GUEST_MEMFD for all 64-bit builds
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=12a663cd980000
> start commit:   43e9ad0c55a3 Merge tag 'scsi-fixes' of git://git.kernel.or..
> git tree:       upstream
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=11a663cd980000
> console output: https://syzkaller.appspot.com/x/log.txt?x=16a663cd980000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=67b63a24f3c26fca
> dashboard link: https://syzkaller.appspot.com/bug?extid=2479e53d0db9b32ae2aa
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=173ecd2f980000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14bc2be2580000
> 
> Reported-by: syzbot+2479e53d0db9b32ae2aa@syzkaller.appspotmail.com
> Fixes: d1e54dd08f16 ("KVM: x86: Enable KVM_GUEST_MEMFD for all 64-bit builds")

LOL, I was about to say that I suspected that this bug existed since guest_memfd
was first added, but that syzbot only now found the issue due to us enabling
KVM_GUEST_MEMFD broadly.  syzbot beat me to the punch.

#syz test

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 427c0acee9d7..3741ea23e330 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -708,23 +708,11 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
        return r;
 }
 
-void kvm_gmem_unbind(struct kvm_memory_slot *slot)
+static void __kvm_gmem_unbind(struct kvm_memory_slot *slot, struct gmem_file *f)
 {
        unsigned long start = slot->gmem.pgoff;
        unsigned long end = start + slot->npages;
-       struct gmem_file *f;
 
-       /*
-        * Nothing to do if the underlying file was already closed (or is being
-        * closed right now), kvm_gmem_release() invalidates all bindings.
-        */
-       CLASS(gmem_get_file, file)(slot);
-       if (!file)
-               return;
-
-       f = file->private_data;
-
-       filemap_invalidate_lock(file->f_mapping);
        xa_store_range(&f->bindings, start, end - 1, NULL, GFP_KERNEL);
 
        /*
@@ -732,6 +720,35 @@ void kvm_gmem_unbind(struct kvm_memory_slot *slot)
         * cannot see this memslot.
         */
        WRITE_ONCE(slot->gmem.file, NULL);
+}
+
+void kvm_gmem_unbind(struct kvm_memory_slot *slot)
+{
+       /*
+        * Nothing to do if the underlying file was _already_ closed, as
+        * kvm_gmem_release() invalidates and nullifies all bindings.
+        */
+       if (!slot->gmem.file)
+               return;
+
+       CLASS(gmem_get_file, file)(slot);
+
+       /*
+        * However, if the file is _being_ closed, then the bindings need to be
+        * removed as kvm_gmem_release() might not run until after the memslot
+        * is freed.  Note, modifying the bindings is safe even though the file
+        * is dying as kvm_gmem_release() nullifies slot->gmem.file under
+        * slots_lock, and only puts its reference to KVM after destroying all
+        * bindings.  I.e. reaching this point means kvm_gmem_release() can't
+        * concurrently destroy the bindings or free the gmem_file.
+        */
+       if (!file) {
+               __kvm_gmem_unbind(slot, slot->gmem.file->private_data);
+               return;
+       }
+
+       filemap_invalidate_lock(file->f_mapping);
+       __kvm_gmem_unbind(slot, file->private_data);
        filemap_invalidate_unlock(file->f_mapping);
 }