[PATCH v3 2/4] virtio_console: fix hot-unplug races in TX paths

[Sungho Bae]

From: Sungho Bae <baver.bae@xxxxxxx>

When a port is hot-unplugged, unplug_port() nullifies port->portdev.
However, concurrent TX paths (__send_to_port, put_chars) could read a
stale pointer or encounter a NULL pointer dereference.

Add READ_ONCE(port->portdev) and NULL checks in the TX paths. In
__send_to_port(), move the out_vq assignment inside the outvq_lock and
check portdev under the lock. Correspondingly, update unplug_port() to
NULL out port->portdev while holding the outvq_lock to serialize with
__send_to_port().

In put_chars(), return count instead of 0 on unplug to prevent the hvc
layer from spinning in an infinite retry loop.

Signed-off-by: Sungho Bae <baver.bae@xxxxxxx>
---
 drivers/char/virtio_console.c | 37 +++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index bbf5b3825f12..589a12261e23 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -600,11 +600,19 @@ static ssize_t __send_to_port(struct port *port, struct scatterlist *sg,
 	int err;
 	unsigned long flags;
 	unsigned int len;
-
-	out_vq = port->out_vq;
+	struct ports_device *portdev;
 
 	spin_lock_irqsave(&port->outvq_lock, flags);
 
+	portdev = READ_ONCE(port->portdev);
+
+	if (!portdev) {
+		in_count = 0;
+		goto free_and_done;
+	}
+
+	out_vq = port->out_vq;
+
 	reclaim_consumed_buffers(port);
 
 	err = virtqueue_add_outbuf(out_vq, sg, nents, buf, GFP_ATOMIC);
@@ -1110,12 +1118,24 @@ static ssize_t put_chars(u32 vtermno, const u8 *buf, size_t count)
 	struct port *port;
 	struct scatterlist sg[1];
 	struct port_buffer *pbuf;
+	struct ports_device *portdev;
 
 	port = find_port_by_vtermno(vtermno);
 	if (!port)
 		return -EPIPE;
 
-	pbuf = alloc_buf(port->portdev->vdev, count, 0, GFP_ATOMIC);
+	/*
+	 * Silently drop output if device hot-unplug is in progress.
+	 * portdev was NULLed by unplug_port() after hvc_remove() was
+	 * already called, so the hvc layer will stop invoking put_chars()
+	 * very soon. Returning count avoids a pointless retry loop in the
+	 * interim.
+	 */
+	portdev = READ_ONCE(port->portdev);
+	if (!portdev)
+		return count;
+
+	pbuf = alloc_buf(portdev->vdev, count, 0, GFP_ATOMIC);
 	if (!pbuf)
 		return -ENOMEM;
 
@@ -1503,11 +1523,16 @@ static void unplug_port(struct port *port)
 	remove_port_data(port);
 
 	/*
-	 * We should just assume the device itself has gone off --
-	 * else a close on an open port later will try to send out a
-	 * control message.
+	 * Null out portdev under outvq_lock so that __send_to_port()
+	 * cannot race: it checks port->portdev inside the same lock
+	 * and bails out if NULL, preventing any buffer from being
+	 * enqueued to an already torn-down virtqueue.  Also prevents
+	 * a close on an open port later from sending a stale control
+	 * message.
 	 */
+	spin_lock_irq(&port->outvq_lock);
 	port->portdev = NULL;
+	spin_unlock_irq(&port->outvq_lock);
 
 	sysfs_remove_group(&port->dev->kobj, &port_attribute_group);
 	device_destroy(&port_class, port->dev->devt);
-- 
2.34.1