[PATCH] virtio_console: guard read-side virtqueue access during unplug

[Yaohan Li]

The virtio-console write path has an unplug fence, but the read path
can still access port->in_vq after device removal has destroyed its
virtqueues. port_has_data() and fill_readbuf() take inbuf_lock without
checking whether the owning port device has gone away.

Clear portdev while holding inbuf_lock during unplug, and check it
under that lock before reading from or re-adding a buffer to the
input virtqueue. Discard an input buffer instead of requeuing it if
the port was unplugged during a userspace copy.

A KASAN-enabled QEMU virtio-serial test reproduced the stale
virtqueue access when read raced with device unbind. The same
injection-assisted sequence did not report a use-after-free with
this change applied.

Fixes: 31610434bc352 ("Virtio console driver")
Assisted-by: ZCode:GLM-4
Signed-off-by: Yaohan Li <shunsheng1...@gmail.com>
---
 drivers/char/virtio_console.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 019bcae81af5c..e97e0daa34395 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -541,10 +541,16 @@ static bool port_has_data(struct port *port)
 
        ret = false;
        spin_lock_irqsave(&port->inbuf_lock, flags);
+
+       /* Port is going away. The vqs are already gone. */
+       if (!port->portdev)
+               goto out_unlock;
+
        port->inbuf = get_inbuf(port);
        if (port->inbuf)
                ret = true;
 
+out_unlock:
        spin_unlock_irqrestore(&port->inbuf_lock, flags);
        return ret;
 }
@@ -725,6 +731,16 @@ static ssize_t fill_readbuf(struct port *port, u8 __user 
*out_buf,
                spin_lock_irqsave(&port->inbuf_lock, flags);
                port->inbuf = NULL;
 
+               /*
+                * The port may have been unplugged while we were copying
+                * data out of the buffer: the vqs are then already gone.
+                */
+               if (!port->portdev) {
+                       spin_unlock_irqrestore(&port->inbuf_lock, flags);
+                       free_buf(buf, true);
+                       return out_count;
+               }
+
                if (add_inbuf(port->in_vq, buf) < 0)
                        dev_warn(port->dev, "failed add_buf\n");
 
@@ -1568,6 +1584,18 @@ static void unplug_port(struct port *port)
         * a close on an open port later from sending a stale control
         * message.
         */
+       /*
+        * Null out portdev under inbuf_lock as well, so the read path
+        * (port_has_data(), fill_readbuf()) cannot race with the vq
+        * teardown: it checks port->portdev inside the same lock and
+        * bails out if NULL, preventing any virtqueue access to an
+        * already torn-down virtqueue — mirroring the outvq_lock fence
+        * below for the write path.
+        */
+       spin_lock_irq(&port->inbuf_lock);
+       port->portdev = NULL;
+       spin_unlock_irq(&port->inbuf_lock);
+
        spin_lock_irq(&port->outvq_lock);
        port->portdev = NULL;
        spin_unlock_irq(&port->outvq_lock);
-- 
2.34.1