[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251024232459.8716-1-hdanton@sina.com>
Date: Sat, 25 Oct 2025 07:24:57 +0800
From: Hillf Danton <hdanton@...a.com>
To: syzbot <syzbot+2479e53d0db9b32ae2aa@...kaller.appspotmail.com>
Cc: Sean Christopherson <seanjc@...gle.com>,
kvm@...r.kernel.org,
linux-kernel@...r.kernel.org,
syzkaller-bugs@...glegroups.com,
tabba@...gle.com,
xiaoyao.li@...el.com
Subject: Re: [syzbot] [kvm?] KASAN: slab-use-after-free Write in kvm_gmem_release
On Thu, Oct 23, 2025, syzbot wrote:
> syzbot has bisected this issue to:
>
> commit d1e54dd08f163a9021433020d16a8f8f70ddc41c
> Author: Fuad Tabba <tabba@...gle.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
Test Sean's fix.
#syz test
--- x/virt/kvm/guest_memfd.c
+++ y/virt/kvm/guest_memfd.c
@@ -708,23 +708,11 @@ err:
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,7 +720,36 @@ void kvm_gmem_unbind(struct kvm_memory_s
* cannot see this memslot.
*/
WRITE_ONCE(slot->gmem.file, NULL);
- filemap_invalidate_unlock(file->f_mapping);
+}
+
+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);
}
/* Returns a locked folio on success. */
--
Powered by blists - more mailing lists