* [PATCH] virtio_console: add console port to list before hvc_alloc()
@ 2026-11-01 2:24 Foo Bar
2026-11-01 2:37 ` sashiko-bot
2026-11-01 2:59 ` Murat Seker
0 siblings, 2 replies; 3+ messages in thread
From: Foo Bar @ 2026-11-01 2:24 UTC (permalink / raw)
To: Amit Shah
Cc: Arnd Bergmann, Greg Kroah-Hartman, Rusty Russell, virtualization,
linux-kernel, muratseker
From: Murat Seker <muratseker@meta.com>
init_port_console() calls hvc_alloc() before adding the port to
pdrvdata.consoles. When the port is the preferred console (for example
console=hvc0), hvc_alloc() registers it, and register_console() replays
the log buffer through put_chars(). put_chars() looks the port up by
vtermno in pdrvdata.consoles, does not find it yet and returns -EPIPE,
so hvc discards every message printed before virtio-console probed.
The messages are still in the kernel log, but a host capturing the
virtio console never receives them.
Add the port to the list before calling hvc_alloc(), and remove it again
if hvc_alloc() fails. The out_vq, outvq_lock and portdev used by
put_chars() are set up in add_port() before init_port_console() runs,
and the other users of cons.hvc check it through is_console_port().
Tested on x86_64 QEMU/KVM with KASAN, PROVE_LOCKING and DEBUG_LIST:
with console=hvc0 the virtio console now starts at [0.000000] instead of
at virtio-console probe; with hvc0 next to ttyS0 both carry the full
log; with earlycon (no replay) output is unchanged; and hot-adding and
removing extra virtconsole ports still works. No new warnings.
Fixes: 38edf58d73c2 ("virtio: console: don't assume a single console port.")
Signed-off-by: Murat Seker <muratseker@meta.com>
---
drivers/char/virtio_console.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 019bcae81af5..a31fdac65891 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1280,18 +1280,28 @@ static int init_port_console(struct port *port)
return ret;
port->cons.vtermno = ret;
+
+ /*
+ * hvc_alloc() may register the console, which replays the log buffer
+ * through put_chars(). put_chars() finds the port on this list, so
+ * add it first or the replayed messages are dropped.
+ */
+ spin_lock_irq(&pdrvdata_lock);
+ list_add_tail(&port->cons.list, &pdrvdata.consoles);
+ spin_unlock_irq(&pdrvdata_lock);
+
port->cons.hvc = hvc_alloc(port->cons.vtermno, 0, &hv_ops, PAGE_SIZE);
if (IS_ERR(port->cons.hvc)) {
ret = PTR_ERR(port->cons.hvc);
dev_err(port->dev,
"error %d allocating hvc for port\n", ret);
port->cons.hvc = NULL;
+ spin_lock_irq(&pdrvdata_lock);
+ list_del(&port->cons.list);
+ spin_unlock_irq(&pdrvdata_lock);
ida_free(&vtermno_ida, port->cons.vtermno);
return ret;
}
- spin_lock_irq(&pdrvdata_lock);
- list_add_tail(&port->cons.list, &pdrvdata.consoles);
- spin_unlock_irq(&pdrvdata_lock);
port->guest_connected = true;
/* Notify host of port being opened */
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] virtio_console: add console port to list before hvc_alloc()
2026-11-01 2:24 [PATCH] virtio_console: add console port to list before hvc_alloc() Foo Bar
@ 2026-11-01 2:37 ` sashiko-bot
2026-11-01 2:59 ` Murat Seker
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-11-01 2:37 UTC (permalink / raw)
To: Foo Bar; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] virtio_console: race between __send_to_port() and out_intr() in add_port()
--
commit e1686190742677c49792956e3ec4f167b8b193e9
Author: Murat Seker <muratseker@meta.com>
virtio_console: add console port to list before hvc_alloc()
This commit adds the console port to pdrvdata.consoles before calling
hvc_alloc() in init_port_console() so that put_chars() can find the port
when register_console() replays the kernel log buffer. If hvc_alloc()
fails, the port is removed from pdrvdata.consoles before freeing the
vtermno.
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 019bcae81af5c..a31fdac658913 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -1280,18 +1280,28 @@ static int init_port_console(struct port *port)
> return ret;
>
> port->cons.vtermno = ret;
> +
> + /*
> + * hvc_alloc() may register the console, which replays the log buffer
> + * through put_chars(). put_chars() finds the port on this list, so
> + * add it first or the replayed messages are dropped.
> + */
> + spin_lock_irq(&pdrvdata_lock);
> + list_add_tail(&port->cons.list, &pdrvdata.consoles);
> + spin_unlock_irq(&pdrvdata_lock);
> +
> port->cons.hvc = hvc_alloc(port->cons.vtermno, 0, &hv_ops, PAGE_SIZE);
[Severity: Critical]
When probing a non-multiport device with a preferred hvc console, can this
cause a race between __send_to_port() and out_intr() on port->out_vq?
In virtcons_probe(), init_vqs() sets out_intr() as the callback for
portdev->out_vqs[0] and virtio_device_ready() enables virtqueue interrupts
before add_port() is called.
In add_port(), when !use_multiport(port->portdev) is true,
init_port_console() is called before the port is added to
port->portdev->ports:
drivers/char/virtio_console.c:add_port() {
...
else if (!use_multiport(port->portdev)) {
/*
* If we're not using multiport support,
* this has to be a console port.
*/
err = init_port_console(port);
if (err)
goto free_inbufs;
}
spin_lock_irq(&portdev->ports_lock);
list_add_tail(&port->list, &port->portdev->ports);
spin_unlock_irq(&portdev->ports_lock);
...
}
Because port->cons.list is now in pdrvdata.consoles during hvc_alloc(),
hvc_alloc() -> hvc_check_console() -> register_console() synchronously
replays the kernel log buffer through hvc_console_print() -> put_chars() ->
__send_to_port() while port is still not in portdev->ports.
__send_to_port() acquires port->outvq_lock with spin_lock_irqsave(), queues
the buffer on out_vq, kicks the host, and spins waiting for completion:
drivers/char/virtio_console.c:__send_to_port() {
...
/* Tell Host to go! */
virtqueue_kick(out_vq);
if (!in_blocking) {
...
while (!(buf = virtqueue_get_buf(out_vq, &len))
&& !virtqueue_is_broken(out_vq))
cpu_relax();
}
...
}
If the host completes the buffer and raises an interrupt on another CPU
before add_port() adds the port to portdev->ports, out_intr() runs and
find_port_by_vq() returns NULL:
drivers/char/virtio_console.c:out_intr() {
struct port *port;
port = find_port_by_vq(vq->vdev->priv, vq);
if (!port) {
flush_bufs(vq, false);
return;
}
...
}
Wouldn't flush_bufs() then call virtqueue_get_buf() and free_buf() on
out_vq without holding port->outvq_lock, corrupting the virtqueue ring
concurrently with __send_to_port() and stealing the completed buffer so
that __send_to_port() spins forever with interrupts disabled?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261101022435.3466820-1-muratseker@meta.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] virtio_console: add console port to list before hvc_alloc()
2026-11-01 2:24 [PATCH] virtio_console: add console port to list before hvc_alloc() Foo Bar
2026-11-01 2:37 ` sashiko-bot
@ 2026-11-01 2:59 ` Murat Seker
1 sibling, 0 replies; 3+ messages in thread
From: Murat Seker @ 2026-11-01 2:59 UTC (permalink / raw)
To: muratseker; +Cc: amit, arnd, gregkh, linux-kernel, rusty, virtualization
On Sat, 31 Oct 2026 19:24:35 -0700, Murat Seker wrote:
> Subject: [PATCH] virtio_console: add console port to list before hvc_alloc()
Please disregard this patch. I'll send a different fix.
Sorry for the noise.
Murat Seker
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-01 3:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-01 2:24 [PATCH] virtio_console: add console port to list before hvc_alloc() Foo Bar
2026-11-01 2:37 ` sashiko-bot
2026-11-01 2:59 ` Murat Seker
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.