All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.