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 <[email protected]> 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/[email protected]?part=1
