A process issuing blocking writes to a virtio console may get stuck
indefinitely if another thread polls the device. Here is how to trigger
the bug:

- Thread A writes to the port until the virtqueue is full.
- Thread A calls wait_port_writable() and goes to sleep, waiting on
  port->waitqueue.
- The host processes some of the write, marks buffers as used and raises
  an interrupt.
- Before the interrupt is serviced, thread B executes port_fops_poll().
  This calls reclaim_consumed_buffers() via will_write_block() and
  consumes all used buffers.
- The interrupt is serviced. vring_interrupt() finds no used buffers
  via more_used() and returns without waking port->waitqueue.
- Thread A is still in wait_event(port->waitqueue), waiting for a
  wakeup that never arrives.

The crux is that invoking reclaim_consumed_buffers() may cause
vring_interrupt() to omit wakeups.

Fix this by calling reclaim_consumed_buffers() in out_int() before
waking. This is similar to the call to discard_port_data() in
in_intr() which also frees buffer from a non-sleepable context.
This in turn guarantees that port->outvq_full is up to date when
handling polling. Since in_intr() already populates port->inbuf we
use that to avoid changing reader state.

Cc: [email protected]
Signed-off-by: Lorenz Bauer <[email protected]>
---
As far as I can tell all currently maintained stable series kernels need
this commit. Applies and builds cleanly on 5.10.247, verified to fix
the issue.
---
Changes in v2:
- Call reclaim_consumed_buffers() in out_intr instead of
  issuing another wake.
- Link to v1: 
https://lore.kernel.org/r/20251215-virtio-console-lost-wakeup-v1-1-79a5c5781...@isovalent.com
---
 drivers/char/virtio_console.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 
088182e54debd6029ea2c2a5542d7a28500e67b8..351e445da35e25910671615a8ecc79165dfc66b7
 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -971,10 +971,17 @@ static __poll_t port_fops_poll(struct file *filp, 
poll_table *wait)
                return EPOLLHUP;
        }
        ret = 0;
-       if (!will_read_block(port))
+
+       spin_lock(&port->inbuf_lock);
+       if (port->inbuf)
                ret |= EPOLLIN | EPOLLRDNORM;
-       if (!will_write_block(port))
+       spin_unlock(&port->inbuf_lock);
+
+       spin_lock(&port->outvq_lock);
+       if (!port->outvq_full)
                ret |= EPOLLOUT;
+       spin_unlock(&port->outvq_lock);
+
        if (!port->host_connected)
                ret |= EPOLLHUP;
 
@@ -1698,6 +1705,7 @@ static void flush_bufs(struct virtqueue *vq, bool 
can_sleep)
 static void out_intr(struct virtqueue *vq)
 {
        struct port *port;
+       unsigned long flags;
 
        port = find_port_by_vq(vq->vdev->priv, vq);
        if (!port) {
@@ -1705,6 +1713,10 @@ static void out_intr(struct virtqueue *vq)
                return;
        }
 
+       spin_lock_irqsave(&port->outvq_lock, flags);
+       reclaim_consumed_buffers(port);
+       spin_unlock_irqrestore(&port->outvq_lock, flags);
+
        wake_up_interruptible(&port->waitqueue);
 }
 

---
base-commit: d358e5254674b70f34c847715ca509e46eb81e6f
change-id: 20251215-virtio-console-lost-wakeup-0f566c5cd35f

Best regards,
-- 
Lorenz Bauer <[email protected]>


Reply via email to