mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Guikun Yang <yangguikun19@qq.com>
To: Amit Shah <amit@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	virtualization@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: [BUG] virtio_console: port_fops_write() dereferences port->portdev without any lock
Date: Sat, 24 Oct 2026 16:35:55 +0800	[thread overview]
Message-ID: <tencent_78459DA7B67FAB8F9039619AAE4699FB680A@qq.com> (raw)

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


                 reply	other threads:[~2026-10-24  8:36 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tencent_78459DA7B67FAB8F9039619AAE4699FB680A@qq.com \
    --to=yangguikun19@qq.com \
    --cc=amit@kernel.org \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=virtualization@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®