The commit states:
If kvm_io_bus_unregister_dev() return -ENOMEM, we already call
kvm_iodevice_destructor() inside this function to delete
'struct kvm_coalesced_mmio_dev *dev' from list and free the dev,
but kvm_iodevice_destructor() is called again, it will lead the
above issue.
Let's check the the return value of kvm_io_bus_unregister_dev(),
only call kvm_iodevice_destructor() if the return value is 0.
This is not entirely correct. kvm_io_bus_unregister_dev() never invokes
kvm_iodevice_destructor() on the device that was passed for
unregistering. Thus, calling kvm_iodevice_destructor() iff
kvm_io_bus_unregister_dev() returns 0 does not fix a use-after-free,
but introduces a memory leak.
It seems that the actual fix for the use-after-free(s) was
commit 5d3c4c79384a ("KVM: Stop looking for coalesced MMIO zones if the
bus is destroyed"), which made into 5.10.37 (mainline 5.13-rc1). Now,
syzkaller's report from the reverted commit indicates an earlier kernel
version 5.10.0, while the memory leak was introduced in 5.10.52
(5.14-rc2).
Currently, running
ioctl(vm, KVM_REGISTER_COALESCED_MMIO, &zone);
// fail the upcoming kmalloc() in kvm_io_bus_unregister_dev()
ioctl(vm, KVM_UNREGISTER_COALESCED_MMIO, &zone);
results in
[ 200.212348] kvm: failed to shrink bus, removing it completely
unreferenced object 0xffff88810e7fa300 (size 64):
comm "a.out", pid 972, jiffies 4294867275 (age 20.499s)
hex dump (first 32 bytes):
58 e3 fd 00 00 c9 ff ff 58 e3 fd 00 00 c9 ff ff X.......X.......
c0 66 65 c0 ff ff ff ff 00 40 fd 00 00 c9 ff ff .fe......@......
backtrace:
[<00000000a9f977ff>] kmalloc_trace+0x26/0x60
[<0000000072e1256d>] kvm_vm_ioctl_register_coalesced_mmio+0x8b/0x420 [kvm]
[<00000000cc4b12dc>] kvm_vm_ioctl+0x1415/0x2050 [kvm]
[<000000004e08022f>] __x64_sys_ioctl+0x126/0x190
[<0000000044a4fad3>] do_syscall_64+0x55/0x80
[<00000000d7073b12>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
This reverts commit 23fa2e46a5556f787ce2ea1a315d3ab93cced204.
Signed-off-by: Michal Luczaj <mhal@xxxxxxx>
---
virt/kvm/coalesced_mmio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
index 0be80c213f7f..f08f5e82460b 100644
--- a/virt/kvm/coalesced_mmio.c
+++ b/virt/kvm/coalesced_mmio.c
@@ -186,6 +186,7 @@ int kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm,
coalesced_mmio_in_range(dev, zone->addr, zone->size)) {
r = kvm_io_bus_unregister_dev(kvm,
zone->pio ? KVM_PIO_BUS : KVM_MMIO_BUS, &dev->dev);
+ kvm_iodevice_destructor(&dev->dev);
/*
* On failure, unregister destroys all devices on the
@@ -195,7 +196,6 @@ int kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm,
*/
if (r)
break;
- kvm_iodevice_destructor(&dev->dev);
}
}
--
2.39.0
