Commit c2e90800 authored by Mark Rutland's avatar Mark Rutland Committed by Michael S. Tsirkin

virtio_mmio: fix devm cleanup

Recent rework of the virtio_mmio probe/remove paths balanced a
devm_ioremap() with an iounmap() rather than its devm variant. This ends
up corrupting the devm datastructures, and results in the following
boot-time splat on arm64 under QEMU 2.9.0:

[    3.450397] ------------[ cut here ]------------
[    3.453822] Trying to vfree() nonexistent vm area (00000000c05b4844)
[    3.460534] WARNING: CPU: 1 PID: 1 at mm/vmalloc.c:1525 __vunmap+0x1b8/0x220
[    3.475898] Kernel panic - not syncing: panic_on_warn set ...
[    3.475898]
[    3.493933] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.15.0-rc3 #1
[    3.513109] Hardware name: linux,dummy-virt (DT)
[    3.525382] Call trace:
[    3.531683]  dump_backtrace+0x0/0x368
[    3.543921]  show_stack+0x20/0x30
[    3.547767]  dump_stack+0x108/0x164
[    3.559584]  panic+0x25c/0x51c
[    3.569184]  __warn+0x29c/0x31c
[    3.576023]  report_bug+0x1d4/0x290
[    3.586069]  bug_handler.part.2+0x40/0x100
[    3.597820]  bug_handler+0x4c/0x88
[    3.608400]  brk_handler+0x11c/0x218
[    3.613430]  do_debug_exception+0xe8/0x318
[    3.627370]  el1_dbg+0x18/0x78
[    3.634037]  __vunmap+0x1b8/0x220
[    3.648747]  vunmap+0x6c/0xc0
[    3.653864]  __iounmap+0x44/0x58
[    3.659771]  devm_ioremap_release+0x34/0x68
[    3.672983]  release_nodes+0x404/0x880
[    3.683543]  devres_release_all+0x6c/0xe8
[    3.695692]  driver_probe_device+0x250/0x828
[    3.706187]  __driver_attach+0x190/0x210
[    3.717645]  bus_for_each_dev+0x14c/0x1f0
[    3.728633]  driver_attach+0x48/0x78
[    3.740249]  bus_add_driver+0x26c/0x5b8
[    3.752248]  driver_register+0x16c/0x398
[    3.757211]  __platform_driver_register+0xd8/0x128
[    3.770860]  virtio_mmio_init+0x1c/0x24
[    3.782671]  do_one_initcall+0xe0/0x398
[    3.791890]  kernel_init_freeable+0x594/0x660
[    3.798514]  kernel_init+0x18/0x190
[    3.810220]  ret_from_fork+0x10/0x18

To fix this, we can simply rip out the explicit cleanup that the devm
infrastructure will do for us when our probe function returns an error
code, or when our remove function returns.

We only need to ensure that we call put_device() if a call to
register_virtio_device() fails in the probe path.
Signed-off-by: default avatarMark Rutland <mark.rutland@arm.com>
Fixes: 7eb781b1 ("virtio_mmio: add cleanup for virtio_mmio_probe")
Fixes: 25f32223 ("virtio_mmio: add cleanup for virtio_mmio_remove")
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: weiping zhang <zhangweiping@didichuxing.com>
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
Reviewed-by: default avatarCornelia Huck <cohuck@redhat.com>
parent 5790eabc
...@@ -522,10 +522,8 @@ static int virtio_mmio_probe(struct platform_device *pdev) ...@@ -522,10 +522,8 @@ static int virtio_mmio_probe(struct platform_device *pdev)
return -EBUSY; return -EBUSY;
vm_dev = devm_kzalloc(&pdev->dev, sizeof(*vm_dev), GFP_KERNEL); vm_dev = devm_kzalloc(&pdev->dev, sizeof(*vm_dev), GFP_KERNEL);
if (!vm_dev) { if (!vm_dev)
rc = -ENOMEM; return -ENOMEM;
goto free_mem;
}
vm_dev->vdev.dev.parent = &pdev->dev; vm_dev->vdev.dev.parent = &pdev->dev;
vm_dev->vdev.dev.release = virtio_mmio_release_dev; vm_dev->vdev.dev.release = virtio_mmio_release_dev;
...@@ -535,17 +533,14 @@ static int virtio_mmio_probe(struct platform_device *pdev) ...@@ -535,17 +533,14 @@ static int virtio_mmio_probe(struct platform_device *pdev)
spin_lock_init(&vm_dev->lock); spin_lock_init(&vm_dev->lock);
vm_dev->base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem)); vm_dev->base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
if (vm_dev->base == NULL) { if (vm_dev->base == NULL)
rc = -EFAULT; return -EFAULT;
goto free_vmdev;
}
/* Check magic value */ /* Check magic value */
magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE); magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) { if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic); dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic);
rc = -ENODEV; return -ENODEV;
goto unmap;
} }
/* Check device version */ /* Check device version */
...@@ -553,8 +548,7 @@ static int virtio_mmio_probe(struct platform_device *pdev) ...@@ -553,8 +548,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
if (vm_dev->version < 1 || vm_dev->version > 2) { if (vm_dev->version < 1 || vm_dev->version > 2) {
dev_err(&pdev->dev, "Version %ld not supported!\n", dev_err(&pdev->dev, "Version %ld not supported!\n",
vm_dev->version); vm_dev->version);
rc = -ENXIO; return -ENXIO;
goto unmap;
} }
vm_dev->vdev.id.device = readl(vm_dev->base + VIRTIO_MMIO_DEVICE_ID); vm_dev->vdev.id.device = readl(vm_dev->base + VIRTIO_MMIO_DEVICE_ID);
...@@ -563,8 +557,7 @@ static int virtio_mmio_probe(struct platform_device *pdev) ...@@ -563,8 +557,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
* virtio-mmio device with an ID 0 is a (dummy) placeholder * virtio-mmio device with an ID 0 is a (dummy) placeholder
* with no function. End probing now with no error reported. * with no function. End probing now with no error reported.
*/ */
rc = -ENODEV; return -ENODEV;
goto unmap;
} }
vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID); vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID);
...@@ -590,33 +583,15 @@ static int virtio_mmio_probe(struct platform_device *pdev) ...@@ -590,33 +583,15 @@ static int virtio_mmio_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, vm_dev); platform_set_drvdata(pdev, vm_dev);
rc = register_virtio_device(&vm_dev->vdev); rc = register_virtio_device(&vm_dev->vdev);
if (rc) { if (rc)
iounmap(vm_dev->base);
devm_release_mem_region(&pdev->dev, mem->start,
resource_size(mem));
put_device(&vm_dev->vdev.dev); put_device(&vm_dev->vdev.dev);
}
return rc;
unmap:
iounmap(vm_dev->base);
free_mem:
devm_release_mem_region(&pdev->dev, mem->start,
resource_size(mem));
free_vmdev:
devm_kfree(&pdev->dev, vm_dev);
return rc; return rc;
} }
static int virtio_mmio_remove(struct platform_device *pdev) static int virtio_mmio_remove(struct platform_device *pdev)
{ {
struct virtio_mmio_device *vm_dev = platform_get_drvdata(pdev); struct virtio_mmio_device *vm_dev = platform_get_drvdata(pdev);
struct resource *mem;
iounmap(vm_dev->base);
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (mem)
devm_release_mem_region(&pdev->dev, mem->start,
resource_size(mem));
unregister_virtio_device(&vm_dev->vdev); unregister_virtio_device(&vm_dev->vdev);
return 0; return 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment