[PATCH] virtio: make more robust in face of misbehaving devices

[PATCH] virtio: make more robust in face of misbehaving devices

[Ahmad Fatoum]

The virtio device drivers assume at a number of places that polling
indefinitely with virtqueue_get_buf will eventually succeed.

It was observed that sometimes after a warm reboot inside QEMU, barebox
finds itself inside an infinite poll loop.

This needs to be further analyzed, of course, but it's already and
improvement for barebox to recover from such situations by returning an
error code or at least printing an error message.

It's unlikely though that the system will be completely functional
after this, but it gives the user a fighting chance.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/block/virtio_blk.c      |  4 ++--
 drivers/hw_random/virtio-rng.c  |  4 ++--
 drivers/net/virtio.c            |  6 ++----
 drivers/serial/virtio_console.c |  4 ++--
 drivers/virtio/virtio_ring.c    | 15 +++++++++++++++
 include/linux/virtio_ring.h     | 22 ++++++++++++++++++++++
 6 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 11111f174142..f1ee76da9750 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -55,8 +55,8 @@ static int virtio_blk_do_req(struct virtio_blk_priv *priv, void *buffer,
 
 	virtqueue_kick(priv->vq);
 
-	while (!virtqueue_get_buf(priv->vq, NULL))
-		;
+	if (!virtqueue_get_buf_timeout(priv->vq, NULL, NSEC_PER_SEC))
+		return -ETIMEDOUT;
 
 	return status == VIRTIO_BLK_S_OK ? 0 : -EIO;
 }
diff --git a/drivers/hw_random/virtio-rng.c b/drivers/hw_random/virtio-rng.c
index 1ce321352935..3d144e4ab654 100644
--- a/drivers/hw_random/virtio-rng.c
+++ b/drivers/hw_random/virtio-rng.c
@@ -47,8 +47,8 @@ static int virtio_rng_read(struct hwrng *hwrng, void *data, size_t len, bool wai
 
 		virtqueue_kick(vi->rng_vq);
 
-		while (!virtqueue_get_buf(vi->rng_vq, &rsize))
-			;
+		if (!virtqueue_get_buf_timeout(vi->rng_vq, &rsize, NSEC_PER_SEC))
+			return -ETIMEDOUT;
 
 		memcpy(ptr, buf, rsize);
 		remaining -= rsize;
diff --git a/drivers/net/virtio.c b/drivers/net/virtio.c
index 73afbc7645bd..8d46acb99a25 100644
--- a/drivers/net/virtio.c
+++ b/drivers/net/virtio.c
@@ -92,10 +92,8 @@ static int virtio_net_send(struct eth_device *edev, void *packet, int length)
 
 	virtqueue_kick(priv->tx_vq);
 
-	while (1) {
-		if (virtqueue_get_buf(priv->tx_vq, NULL))
-			break;
-	}
+	if (!virtqueue_get_buf_timeout(priv->tx_vq, NULL, NSEC_PER_SEC))
+		return -ETIMEDOUT;
 
 	return 0;
 }
diff --git a/drivers/serial/virtio_console.c b/drivers/serial/virtio_console.c
index 09249aef7a14..ae36cad6e5bd 100644
--- a/drivers/serial/virtio_console.c
+++ b/drivers/serial/virtio_console.c
@@ -53,8 +53,8 @@ static void put_chars(struct virtio_console *virtcons, const char *buf, int coun
 		/* Tell Host to go! */
 		virtqueue_kick(out_vq);
 		/* Chill out until it's done with the buffer. */
-		while (!virtqueue_get_buf(out_vq, &len))
-			cpu_relax();
+		if (!virtqueue_get_buf_timeout(out_vq, &len, NSEC_PER_SEC))
+			dev_warn(virtcons->cdev.dev, "Timeout waiting for TX ack\n");
 	}
 }
 
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 29d7bb9ef662..284721aeff5b 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -13,6 +13,7 @@
 #include <linux/virtio_types.h>
 #include <linux/virtio.h>
 #include <linux/virtio_ring.h>
+#include <linux/ktime.h>
 #include <linux/bug.h>
 #include <dma.h>
 
@@ -276,6 +277,20 @@ void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len)
 	return ret;
 }
 
+void *virtqueue_get_buf_timeout(struct virtqueue *vq, unsigned int *len,
+				ktime_t timeout)
+{
+	ktime_t start = get_time_ns();
+
+	do {
+		void *ret = virtqueue_get_buf(vq, len);
+		if (ret)
+			return ret;
+	} while (!is_timeout(start, timeout));
+
+	return NULL;
+}
+
 static struct virtqueue *__vring_new_virtqueue(unsigned int index,
 					       struct vring vring,
 					       struct virtio_device *vdev)
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 04a2ad0cb1f9..7548504bc42b 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -11,6 +11,7 @@
 
 #include <linux/virtio_types.h>
 #include <linux/scatterlist.h>
+#include <linux/ktime.h>
 
 /* This marks a buffer as continuing via the next field */
 #define VRING_DESC_F_NEXT		1
@@ -257,6 +258,27 @@ void virtqueue_kick(struct virtqueue *vq);
  */
 void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len);
 
+/**
+ * virtqueue_get_buf_timeout - poll the next used buffer with timeout
+ *
+ * @vq:		the struct virtqueue we're talking about
+ * @len:	the length written into the buffer
+ * @timeout:	the timeout in nanoseconds
+ *
+ * If the device wrote data into the buffer, @len will be set to the
+ * amount written. This means you don't need to clear the buffer
+ * beforehand to ensure there's no data leakage in the case of short
+ * writes.
+ *
+ * Caller must ensure we don't call this with other virtqueue
+ * operations at the same time (except where noted).
+ *
+ * Returns NULL on timeout, or the "data" token
+ * handed to virtqueue_add_*().
+ */
+void *virtqueue_get_buf_timeout(struct virtqueue *vq, unsigned int *len,
+				ktime_t timeout);
+
 /**
  * vring_create_virtqueue - create a virtqueue for a virtio device
  *
-- 
2.39.5

Re: [PATCH] virtio: make more robust in face of misbehaving devices

[Sascha Hauer]

On Tue, 22 Apr 2025 09:56:58 +0200, Ahmad Fatoum wrote:
> The virtio device drivers assume at a number of places that polling
> indefinitely with virtqueue_get_buf will eventually succeed.
> 
> It was observed that sometimes after a warm reboot inside QEMU, barebox
> finds itself inside an infinite poll loop.
> 
> This needs to be further analyzed, of course, but it's already and
> improvement for barebox to recover from such situations by returning an
> error code or at least printing an error message.
> 
> [...]

Applied, thanks!

[1/1] virtio: make more robust in face of misbehaving devices
      https://git.pengutronix.de/cgit/barebox/commit/?id=1e84d6ffc4af (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>