[PATCH] virtio: fix use-after-free in unregister_virtio_device()

Karl Mehltretter <[email protected]>
Newsgroups dev.linux.lists.virtualization,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
device_unregister() is device_del() plus put_device(). When the caller
holds no extra reference, that drops the last one and runs the release
callback, which for several transports frees the memory the embedded
struct virtio_device sits in. unregister_virtio_device() then calls
virtio_debug_device_exit(), which reads dev->debugfs_dir out of the freed
object.

Affected transports are the ones whose release callback frees and whose
remove path takes no reference: virtio_mmio, virtio_vdpa, virtio_uml,
mlxbf-tmfifo and virtio_ccw. virtio_pci is unaffected because
virtio_pci_remove() brackets the call with get_device() and put_device().

Remove the debugfs entries before the device can go away. They are only
accessed through the protected debugfs interface, so
debugfs_remove_recursive() waits for in-progress file operations before
returning. Tearing them down while the device is still alive is therefore
safe.

Reproduced on User-Mode Linux with CONFIG_KASAN and CONFIG_VIRTIO_DEBUG
by unbinding a virtio-uml device:

  BUG: KASAN: slab-use-after-free in virtio_debug_device_exit+0x36/0x4d
  Read of size 8 at addr 00000000616e0b10 by task init/1
   __asan_report_load8_noabort
   virtio_debug_device_exit+0x36/0x4d
   unregister_virtio_device+0x48/0x75
   virtio_uml_remove
   platform_remove
   device_release_driver_internal
   unbind_store

  Freed by task 1:
   kfree
   virtio_uml_release_dev
   device_release
   kobject_put
   put_device
   device_unregister
   unregister_virtio_device+0x48/0x75
   virtio_uml_remove

With this applied, the report is gone and unbind is clean.

Fixes: 96a8326d69ff ("virtio: add debugfs infrastructure to allow to debug virtio features")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Karl Mehltretter <[email protected]>
---
 drivers/virtio/virtio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 75bb4ffe3b877..b6c9e927bef57 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -604,8 +604,8 @@ void unregister_virtio_device(struct virtio_device *dev)
 {
 	int index = dev->index; /* save for after device release */
 
-	device_unregister(&dev->dev);
 	virtio_debug_device_exit(dev);
+	device_unregister(&dev->dev);
 	ida_free(&virtio_index_ida, index);
 }
 EXPORT_SYMBOL_GPL(unregister_virtio_device);

base-commit: 4477a78374a57c3809b172ad30cceabda48c47c6
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.