[PATCH] Revert "KVM: mmio: Fix use-after-free Read in kvm_vm_ioctl_unregister_coalesced_mmio"

[Michal Luczaj <mhal@rbox.co>]

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@rbox.co>
---
 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