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: stable@vger.kernel.org Signed-off-by: Lorenz Bauer lmb@isovalent.com --- 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... --- 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,
On Mon, Dec 22, 2025, at 17:04, Lorenz Bauer wrote:
--- 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)
As far as I can tell, you got the interrupt flag handling wrong in both places: port_fops_poll() is called with interrupts enabled, so you have to use spin_lock_irq() to block the interrupt from hanging.
@@ -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);
The callback seems to always be called with interrupts disabled(), so here it's safe to use spin_lock() instead of spin_lock_irqsave().
Arnd
On Mon, Dec 22, 2025 at 5:13 PM Arnd Bergmann arnd@arndb.de wrote:
On Mon, Dec 22, 2025, at 17:04, Lorenz Bauer wrote:
--- 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)As far as I can tell, you got the interrupt flag handling wrong in both places: port_fops_poll() is called with interrupts enabled, so you have to use spin_lock_irq() to block the interrupt from hanging.
Ack.
@@ -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);The callback seems to always be called with interrupts disabled(), so here it's safe to use spin_lock() instead of spin_lock_irqsave().
This is pretty much just copied from in_intr which also uses _irqsave. I think it makes sense to stick to that for consistency's sake. What do you think?
On Mon, Dec 22, 2025, at 17:23, Lorenz Bauer wrote:
On Mon, Dec 22, 2025 at 5:13 PM Arnd Bergmann arnd@arndb.de wrote:
On Mon, Dec 22, 2025, at 17:04, Lorenz Bauer wrote:
@@ -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);The callback seems to always be called with interrupts disabled(), so here it's safe to use spin_lock() instead of spin_lock_irqsave().
This is pretty much just copied from in_intr which also uses _irqsave. I think it makes sense to stick to that for consistency's sake. What do you think?
The usual rule is that you must use spin_lock_irqsave() if the function can be called from either interrupt or non-interrupt context. It's also safe to be used if you are not sure.
However, in interrupt handlers you usually want to use the plain spin_lock() both for efficiency reasons and to document the calling conventions.
Arnd
linux-stable-mirror@lists.linaro.org