mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [BUG] virtio_console: port_fops_write() dereferences port->portdev without any lock
@ 2026-10-24  8:35 Guikun Yang
  0 siblings, 0 replies; only message in thread
From: Guikun Yang @ 2026-10-24  8:35 UTC (permalink / raw)
  To: Amit Shah; +Cc: Arnd Bergmann, Greg Kroah-Hartman, virtualization, linux-kernel

Hi Amit,

port_fops_write() dereferences port->portdev->vdev to allocate the
output buffer, but by the time that line runs the port can already have
been hot-unplugged, which sets port->portdev to NULL.  The result is a
NULL pointer dereference on a plain write() to a virtio console port
whose device was removed concurrently.

The write path:

	port = filp->private_data;

	nonblock = filp->f_flags & O_NONBLOCK;

	ret = wait_port_writable(port, nonblock);
	if (ret < 0)
		return ret;

	count = min((size_t)(32 * 1024), count);

	buf = alloc_buf(port->portdev->vdev, count, 0, GFP_KERNEL);

wait_port_writable() ends with:

	/* Port got hot-unplugged. */
	if (!port->guest_connected)
		return -ENODEV;

That check reads guest_connected without any lock, and the code after
it (including the alloc_buf() argument evaluation) runs without a lock
either.

The unplug path (unplug_port(), running from the control workqueue on
VIRTIO_CONSOLE_PORT_REMOVE):

	spin_lock_irq(&port->inbuf_lock);
	if (port->guest_connected) {
		send_sigio_to_port(port);
		port->guest_connected = false;
		port->host_connected = false;
		wake_up_interruptible(&port->waitqueue);
	}
	spin_unlock_irq(&port->inbuf_lock);
	...
	spin_lock_irq(&port->outvq_lock);
	port->portdev = NULL;
	spin_unlock_irq(&port->outvq_lock);

	guest_connected is cleared under inbuf_lock, and portdev is cleared
under outvq_lock afterwards.  A writer that passed the guest_connected
check in wait_port_writable() can then reach alloc_buf() after the
unplug has cleared portdev, and port->portdev->vdev dereferences NULL.

The code itself shows the authors were aware of this hazard:
__send_to_port() checks portdev under outvq_lock and bails out when
NULL ("Null out portdev under outvq_lock so that __send_to_port()
cannot race").  port_fops_write() and the other file-operations paths
that read port->portdev outside that lock were left unprotected.  The
unprotected dereferences are:

	line 860: port_fops_write():
		buf = alloc_buf(port->portdev->vdev, count, 0, GFP_KERNEL);
	line 968: port_fops_splice_write():
		buf = alloc_buf(port->portdev->vdev, 0, occupancy, GFP_KERNEL);

Both run after wait_port_writable() with no lock held; a concurrent
unplug clears portdev in between and the next statement dereferences
NULL.  port_fops_splice_write() additionally reads port->out_vq->vdev
at its start (line 955) with no lock either.

Note on lifetime: this is a NULL dereference, not a use-after-free of
the port itself.  The fd holds a reference on the port, so the port
object survives; only port->portdev is nulled by the unplug.  cdev_del()
in unplug_port() only prevents new opens; already-open fds keep
returning data from the stale port structure.

Trigger: any KVM/kata-style guest with a virtio console port opened by
userspace while the host (or the virtio device) removes the port.
Hot-unplug of console ports is a normal operation, so the window does
not require a malicious setup, only timing.

I checked linux-next (2026-10-24, drivers/char/virtio_console.c as of
today): port_fops_write() still reads port->portdev->vdev unprotected,
and so does the read path at line 968.

Related work: Hari Mishal's series earlier this year added a kref to
find_port_by_vq() (upstream commit 18dd0b4e489e, now in stable), which
fixed the in_intr()/out_intr() use-after-free of the port object.  That
fix addresses a different lifetime problem (the interrupt paths racing
the kref_put in unplug_port()); this report is about the userspace
write/read paths dereferencing port->portdev, which that series did not
touch.

Suggested fix direction: read portdev once with READ_ONCE() after
wait_port_writable() and bail with -ENODEV when NULL, or take
outvq_lock around the alloc for consistency with __send_to_port()'s
existing convention.  port_fops_splice_write() needs the same
treatment (and its out_vq read at line 955 needs a look as well).

I do not have a reproducer program for this one; the race is the same
shape as the unplugged-port races already fixed in this file, and the
window is the gap between the unlocked guest_connected check and the
alloc_buf() call.

Best,
Guikun Yang


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2026-10-24  8:36 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-24  8:35 [BUG] virtio_console: port_fops_write() dereferences port->portdev without any lock Guikun Yang

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®