[PATCH 0/2] virtio: synchronize callbacks during device reset

[PATCH 0/2] virtio: synchronize callbacks during device reset

[Karl Mehltretter]

virtio_reset_device() documents that vq callbacks are not in progress
once it returns. virtio-pci delivers that by ending vp_reset() with
vp_synchronize_vectors(), virtio-mmio does not, and a driver that frees
state its callback uses then hits a use after free.

Patch 1 brings virtio-mmio in line with virtio-pci.

Patch 2 makes virtio_input reset before unregistering its input device
and stop its callback once teardown has begun.

Together these patches fix the use after free on virtio-pci and
virtio-mmio. Classic virtio-ccw and other transports still
do not provide the documented guarantee.

The use-after-free was reproduced under KASAN with the race window
artificially widened.

Callback synchronization was tested in an arm64 QEMU guest with a
virtio-input device over virtio-mmio and four vCPUs. The event callback
used a busy delay, not a sleep, and an in-progress counter tracked
overlap with unbind.

The driver was unbound and rebound 120 times under continuous input.
With patch 2 alone, virtio_reset_device() returned before the callback
finished in all 109 races, each in under 1 ms. With both patches,
vm_reset() waited for the callback in all 111 races, taking 24 to 86 ms.

Karl Mehltretter (2):
  virtio-mmio: synchronize callbacks during device reset
  virtio_input: stop callbacks before unregistering input device

 drivers/virtio/virtio_input.c | 8 ++++++--
 drivers/virtio/virtio_mmio.c  | 6 ++++++
 2 files changed, 12 insertions(+), 2 deletions(-)


base-commit: 3eb40771c00a8488fa6ed2cc1fe203477908bf38
-- 
2.53.0

[PATCH 1/2] virtio-mmio: synchronize callbacks during device reset

[Karl Mehltretter]

virtio_reset_device() promises that vq callbacks are not in progress
once it returns, but vm_reset() only writes 0 to the status register.
A callback that has already entered vring_interrupt() keeps running
while the driver tears down the state it uses.

Wait for a status read to return 0, so the device has stopped raising
interrupts, then synchronize_irq() as vp_reset() does with
vp_synchronize_vectors().

Fixes: edfd52e63672 ("virtio: Add platform bus driver for memory mapped virtio device")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 drivers/virtio/virtio_mmio.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 510b7c4efdff8..a2ca03dbe803d 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -55,6 +55,7 @@
 #define pr_fmt(fmt) "virtio-mmio: " fmt
 
 #include <linux/acpi.h>
+#include <linux/delay.h>
 #include <linux/dma-mapping.h>
 #include <linux/highmem.h>
 #include <linux/interrupt.h>
@@ -254,6 +255,11 @@ static void vm_reset(struct virtio_device *vdev)
 
 	/* 0 status means a reset. */
 	writel(0, vm_dev->base + VIRTIO_MMIO_STATUS);
+
+	/* Wait for reset completion before flushing callbacks. */
+	while (vm_get_status(vdev))
+		fsleep(1000);
+	synchronize_irq(platform_get_irq(vm_dev->pdev, 0));
 }
 
 
-- 
2.53.0

Re: [PATCH 1/2] virtio-mmio: synchronize callbacks during device reset

[Michael S. Tsirkin]

On Tue, Aug 18, 2026 at 06:04:32AM +0200, Karl Mehltretter wrote:
> virtio_reset_device() promises that vq callbacks are not in progress
> once it returns, but vm_reset() only writes 0 to the status register.
> A callback that has already entered vring_interrupt() keeps running
> while the driver tears down the state it uses.
> 
> Wait for a status read to return 0, so the device has stopped raising
> interrupts, then synchronize_irq() as vp_reset() does with
> vp_synchronize_vectors().
> 
> Fixes: edfd52e63672 ("virtio: Add platform bus driver for memory mapped virtio device")
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
>  drivers/virtio/virtio_mmio.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 510b7c4efdff8..a2ca03dbe803d 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -55,6 +55,7 @@
>  #define pr_fmt(fmt) "virtio-mmio: " fmt
>  
>  #include <linux/acpi.h>
> +#include <linux/delay.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/highmem.h>
>  #include <linux/interrupt.h>
> @@ -254,6 +255,11 @@ static void vm_reset(struct virtio_device *vdev)
>  
>  	/* 0 status means a reset. */
>  	writel(0, vm_dev->base + VIRTIO_MMIO_STATUS);
> +
> +	/* Wait for reset completion before flushing callbacks. */
> +	while (vm_get_status(vdev))
> +		fsleep(1000);

this is only required for transport version 3.


> +	synchronize_irq(platform_get_irq(vm_dev->pdev, 0));
>  }
>  
>  
> -- 
> 2.53.0

Re: [PATCH 1/2] virtio-mmio: synchronize callbacks during device reset

[Michael S. Tsirkin]

On Tue, Aug 18, 2026 at 06:04:32AM +0200, Karl Mehltretter wrote:
> virtio_reset_device() promises that vq callbacks are not in progress
> once it returns, but vm_reset() only writes 0 to the status register.
> A callback that has already entered vring_interrupt() keeps running
> while the driver tears down the state it uses.
> 
> Wait for a status read to return 0, so the device has stopped raising
> interrupts, then synchronize_irq() as vp_reset() does with
> vp_synchronize_vectors().
> 
> Fixes: edfd52e63672 ("virtio: Add platform bus driver for memory mapped virtio device")
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>


I htink we should  instead drop 

vp_synchronize_vectors

from vp_reset

and instead call virtio_synchronize_cbs from virtio_reset_device.



> ---
>  drivers/virtio/virtio_mmio.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 510b7c4efdff8..a2ca03dbe803d 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -55,6 +55,7 @@
>  #define pr_fmt(fmt) "virtio-mmio: " fmt
>  
>  #include <linux/acpi.h>
> +#include <linux/delay.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/highmem.h>
>  #include <linux/interrupt.h>
> @@ -254,6 +255,11 @@ static void vm_reset(struct virtio_device *vdev)
>  
>  	/* 0 status means a reset. */
>  	writel(0, vm_dev->base + VIRTIO_MMIO_STATUS);
> +
> +	/* Wait for reset completion before flushing callbacks. */
> +	while (vm_get_status(vdev))
> +		fsleep(1000);


only needed for v3 and up

> +	synchronize_irq(platform_get_irq(vm_dev->pdev, 0));


>  }
>  
>  
> -- 
> 2.53.0

[PATCH 2/2] virtio_input: stop callbacks before unregistering input device

[Karl Mehltretter]

virtinput_remove() unregisters the input device, which can free it,
before resetting the virtio device that can still deliver events.

virtinput_recv_events() drops vi->lock around input_event() and samples
vi->ready only on loop entry. A callback that has passed that check can
therefore use the freed device, requeue a buffer, and kick the queue
after reset has begun.

A KASAN run with the race window widened reproduced the use-after-free.

Reset the device first, as virtinput_freeze() already does. Recheck
vi->ready after reacquiring the lock so the callback does not requeue or
kick once teardown starts.

Fixes: 271c865161c5 ("Add virtio-input driver.")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 drivers/virtio/virtio_input.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
index deec24e8e6828..12ff33b178ea3 100644
--- a/drivers/virtio/virtio_input.c
+++ b/drivers/virtio/virtio_input.c
@@ -49,9 +49,12 @@ static void virtinput_recv_events(struct virtqueue *vq)
 				    le16_to_cpu(event->code),
 				    le32_to_cpu(event->value));
 			spin_lock_irqsave(&vi->lock, flags);
+			if (!vi->ready)
+				break;
 			virtinput_queue_evtbuf(vi, event);
 		}
-		virtqueue_kick(vq);
+		if (vi->ready)
+			virtqueue_kick(vq);
 	}
 	spin_unlock_irqrestore(&vi->lock, flags);
 }
@@ -350,8 +353,9 @@ static void virtinput_remove(struct virtio_device *vdev)
 	vi->ready = false;
 	spin_unlock_irqrestore(&vi->lock, flags);
 
-	input_unregister_device(vi->idev);
+	/* Stop callbacks before unregistering the input device. */
 	virtio_reset_device(vdev);
+	input_unregister_device(vi->idev);
 	while ((buf = virtqueue_detach_unused_buf(vi->sts)) != NULL)
 		kfree(buf);
 	vdev->config->del_vqs(vdev);
-- 
2.53.0

Re: [PATCH 0/2] virtio: synchronize callbacks during device reset

[Michael S. Tsirkin]

On Tue, Aug 18, 2026 at 06:04:31AM +0200, Karl Mehltretter wrote:
> virtio_reset_device() documents that vq callbacks are not in progress
> once it returns. virtio-pci delivers that by ending vp_reset() with
> vp_synchronize_vectors(), virtio-mmio does not, and a driver that frees
> state its callback uses then hits a use after free.
> 
> Patch 1 brings virtio-mmio in line with virtio-pci.
> 
> Patch 2 makes virtio_input reset before unregistering its input device
> and stop its callback once teardown has begun.
> 
> Together these patches fix the use after free on virtio-pci

pci has synchronize on reset already

> and
> virtio-mmio. Classic virtio-ccw and other transports still
> do not provide the documented guarantee.
> 
> The use-after-free was reproduced under KASAN with the race window
> artificially widened.
> 
> Callback synchronization was tested in an arm64 QEMU guest with a
> virtio-input device over virtio-mmio and four vCPUs. The event callback
> used a busy delay, not a sleep, and an in-progress counter tracked
> overlap with unbind.
> 
> The driver was unbound and rebound 120 times under continuous input.
> With patch 2 alone, virtio_reset_device() returned before the callback
> finished in all 109 races, each in under 1 ms. With both patches,
> vm_reset() waited for the callback in all 111 races, taking 24 to 86 ms.
> 
> Karl Mehltretter (2):
>   virtio-mmio: synchronize callbacks during device reset
>   virtio_input: stop callbacks before unregistering input device
> 
>  drivers/virtio/virtio_input.c | 8 ++++++--
>  drivers/virtio/virtio_mmio.c  | 6 ++++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> 
> base-commit: 3eb40771c00a8488fa6ed2cc1fe203477908bf38
> -- 
> 2.53.0

Re: [PATCH 0/2] virtio: synchronize callbacks during device reset

[Michael S. Tsirkin]

On Tue, Aug 18, 2026 at 06:04:31AM +0200, Karl Mehltretter wrote:
> virtio_reset_device() documents that vq callbacks are not in progress
> once it returns. virtio-pci delivers that by ending vp_reset() with
> vp_synchronize_vectors(), virtio-mmio does not, and a driver that frees
> state its callback uses then hits a use after free.
> 
> Patch 1 brings virtio-mmio in line with virtio-pci.
> 
> Patch 2 makes virtio_input reset before unregistering its input device
> and stop its callback once teardown has begun.
> 
> Together these patches fix the use after free on virtio-pci and
> virtio-mmio. Classic virtio-ccw and other transports still
> do not provide the documented guarantee.


so with my proposal virtio-ccw will be fixed automagically.

other transports will need synchronize_cbs callback - pls add that as patch 3.


> The use-after-free was reproduced under KASAN with the race window
> artificially widened.
> 
> Callback synchronization was tested in an arm64 QEMU guest with a
> virtio-input device over virtio-mmio and four vCPUs. The event callback
> used a busy delay, not a sleep, and an in-progress counter tracked
> overlap with unbind.
> 
> The driver was unbound and rebound 120 times under continuous input.
> With patch 2 alone, virtio_reset_device() returned before the callback
> finished in all 109 races, each in under 1 ms. With both patches,
> vm_reset() waited for the callback in all 111 races, taking 24 to 86 ms.
> 
> Karl Mehltretter (2):
>   virtio-mmio: synchronize callbacks during device reset
>   virtio_input: stop callbacks before unregistering input device
> 
>  drivers/virtio/virtio_input.c | 8 ++++++--
>  drivers/virtio/virtio_mmio.c  | 6 ++++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> 
> base-commit: 3eb40771c00a8488fa6ed2cc1fe203477908bf38
> -- 
> 2.53.0