mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* virtio_console: lost wakeup due to race between port_fops_poll() and vring_interrupt()
@ 2025-12-05 18:06 Lorenz Bauer
  2025-12-10 15:58 ` Amit Shah
  0 siblings, 1 reply; 3+ messages in thread
From: Lorenz Bauer @ 2025-12-05 18:06 UTC (permalink / raw)
  To: Amit Shah, Arnd Bergmann, Greg Kroah-Hartman, virtualization, LKML

Hi,

I've been chasing a bug when using virtio console to exchange data
between guest and host. It manifests in a process inside the guest
getting stuck while writing to the serial port:

    [<0>] wait_port_writable+0x139/0x2d0
    [<0>] port_fops_write+0x88/0x130
    [<0>] vfs_write+0xf3/0x450
    [<0>] ksys_write+0x6d/0xe0
    [<0>] do_syscall_64+0x9e/0x1a0
    [<0>] entry_SYSCALL_64_after_hwframe+0x77/0x7

I managed to track this down to a race in virtio_console.c. The driver
doesn't properly account for a port that receives blocking writes and
is polled at the same time. I suspect that a similar problem exists in
port_fops_read().

Here is how it goes. We need two threads inside the guest, A and B.

- Thread A writes to the serial port until the virtqueue fills up.
- Thread A invokes port_fops_write() which calls wait_port_writable().
There are no used buffers to reclaim and the thread is suspended,
waiting on port->waitqueue.
- The host side of the device makes some progress, marks some buffers
as consumed / used and  sends an interrupt to the guest.
- Before the guest can service the interrupt, thread B executes
port_fops_poll(). This calls into
  reclaim_consumed_buffers() via will_write_block(), which removes all
used buffers.
- The interrupt gets serviced, calling into vring_interrupt(). Here
the check for more_used() returns false because the poll just went
through all the work. The handler returns without waking
port->waitqueue.
- port_fops_write() never returns.

I'm not exactly sure how to best fix this. Maybe it's enough to only
check outvq_full in poll? It'd also be nice to fix this on the read
side as well.

I only have a reproducer which requires a Go toolchain unfortunately:
https://github.com/lmb/vimto/issues/29

Best
Lorenz

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: virtio_console: lost wakeup due to race between port_fops_poll() and vring_interrupt()
  2025-12-05 18:06 virtio_console: lost wakeup due to race between port_fops_poll() and vring_interrupt() Lorenz Bauer
@ 2025-12-10 15:58 ` Amit Shah
  2025-12-15  9:51   ` Lorenz Bauer
  0 siblings, 1 reply; 3+ messages in thread
From: Amit Shah @ 2025-12-10 15:58 UTC (permalink / raw)
  To: Lorenz Bauer, Arnd Bergmann, Greg Kroah-Hartman, virtualization, LKML

On Fri, 2025-12-05 at 18:06 +0000, Lorenz Bauer wrote:
> Hi,
> 
> I've been chasing a bug when using virtio console to exchange data
> between guest and host. It manifests in a process inside the guest
> getting stuck while writing to the serial port:
> 
>     [<0>] wait_port_writable+0x139/0x2d0
>     [<0>] port_fops_write+0x88/0x130
>     [<0>] vfs_write+0xf3/0x450
>     [<0>] ksys_write+0x6d/0xe0
>     [<0>] do_syscall_64+0x9e/0x1a0
>     [<0>] entry_SYSCALL_64_after_hwframe+0x77/0x7
> 
> I managed to track this down to a race in virtio_console.c. The
> driver
> doesn't properly account for a port that receives blocking writes and
> is polled at the same time. I suspect that a similar problem exists
> in
> port_fops_read().
> 
> Here is how it goes. We need two threads inside the guest, A and B.
> 
> - Thread A writes to the serial port until the virtqueue fills up.
> - Thread A invokes port_fops_write() which calls
> wait_port_writable().
> There are no used buffers to reclaim and the thread is suspended,
> waiting on port->waitqueue.
> - The host side of the device makes some progress, marks some buffers
> as consumed / used and  sends an interrupt to the guest.
> - Before the guest can service the interrupt, thread B executes
> port_fops_poll(). This calls into
>   reclaim_consumed_buffers() via will_write_block(), which removes
> all
> used buffers.
> - The interrupt gets serviced, calling into vring_interrupt(). Here
> the check for more_used() returns false because the poll just went
> through all the work. The handler returns without waking
> port->waitqueue.
> - port_fops_write() never returns.

Both port_fops_write() and port_fops_poll() call will_write_block() for
checking the block condition -- which is whether port->outvq_full is
set.  Even if buffers were consumed via poll, outvq_full should just
return false, right?

Hm, does adding wake_up(port->waitqueue) in case 'ret' is false in
will_write_block() help?

> I'm not exactly sure how to best fix this. Maybe it's enough to only
> check outvq_full in poll? It'd also be nice to fix this on the read
> side as well.
> 
> I only have a reproducer which requires a Go toolchain unfortunately:
> https://github.com/lmb/vimto/issues/29
> 
> Best
> Lorenz

		Amit

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: virtio_console: lost wakeup due to race between port_fops_poll() and vring_interrupt()
  2025-12-10 15:58 ` Amit Shah
@ 2025-12-15  9:51   ` Lorenz Bauer
  0 siblings, 0 replies; 3+ messages in thread
From: Lorenz Bauer @ 2025-12-15  9:51 UTC (permalink / raw)
  To: Amit Shah; +Cc: Arnd Bergmann, Greg Kroah-Hartman, virtualization, LKML

On Wed, Dec 10, 2025 at 3:58 PM Amit Shah <amit@kernel.org> wrote:
>
> Both port_fops_write() and port_fops_poll() call will_write_block() for
> checking the block condition -- which is whether port->outvq_full is
> set.  Even if buffers were consumed via poll, outvq_full should just
> return false, right?

Yes, it would. However, the problem is that port_fops_write never gets
to that part. It is stuck in wait_event_freezable(port->waitqueue).

> Hm, does adding wake_up(port->waitqueue) in case 'ret' is false in
> will_write_block() help?

It would, but I think it only addresses half of the problem. The read path
also calls reclaim_consumed_buffers. I've thrown together a patch which
issues a wakeup from reclaim_consumed_buffers instead, which seems
to work.

Best
Lorenz

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-12-15  9:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-05 18:06 virtio_console: lost wakeup due to race between port_fops_poll() and vring_interrupt() Lorenz Bauer
2025-12-10 15:58 ` Amit Shah
2025-12-15  9:51   ` Lorenz Bauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®