From c6017e793b932e84b1c998f9b4c08d74ff0ea9c0 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Mon, 29 Jul 2013 14:23:46 +0930 Subject: virtio: console: add locks around buffer removal in port unplug path The removal functions act on the vqs, and the vq operations need to be locked. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 1b456fe9b87a..601962b1708b 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1513,18 +1513,22 @@ static void remove_port_data(struct port *port) { struct port_buffer *buf; + spin_lock_irq(&port->inbuf_lock); /* Remove unused data this port might have received. */ discard_port_data(port); - reclaim_consumed_buffers(port); - /* Remove buffers we queued up for the Host to send us data in. */ while ((buf = virtqueue_detach_unused_buf(port->in_vq))) free_buf(buf, true); + spin_unlock_irq(&port->inbuf_lock); + + spin_lock_irq(&port->outvq_lock); + reclaim_consumed_buffers(port); /* Free pending buffers from the out-queue. */ while ((buf = virtqueue_detach_unused_buf(port->out_vq))) free_buf(buf, true); + spin_unlock_irq(&port->outvq_lock); } /* -- cgit v1.2.3-58-ga151 From 5549fb25811710585d5ec77a0e6a1fbc8808df93 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Mon, 29 Jul 2013 14:24:15 +0930 Subject: virtio: console: add locking in port unplug path Port unplug can race with close() in port_fops_release(). port_fops_release() already takes the necessary locks, ensure unplug_port() does that too. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 601962b1708b..3572c5a92e8e 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1542,6 +1542,7 @@ static void unplug_port(struct port *port) list_del(&port->list); spin_unlock_irq(&port->portdev->ports_lock); + spin_lock_irq(&port->inbuf_lock); if (port->guest_connected) { port->guest_connected = false; port->host_connected = false; @@ -1550,6 +1551,7 @@ static void unplug_port(struct port *port) /* Let the app know the port is going down. */ send_sigio_to_port(port); } + spin_unlock_irq(&port->inbuf_lock); if (is_console_port(port)) { spin_lock_irq(&pdrvdata_lock); -- cgit v1.2.3-58-ga151 From 314081f1025e9ea77b69261f21183e9be18a6f91 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Mon, 29 Jul 2013 14:25:38 +0930 Subject: virtio: console: fix locking around send_sigio_to_port() send_sigio_to_port() checks the value of guest_connected, which we always modify under the inbuf_lock; make sure invocations of send_sigio_to_port() have take the inbuf_lock around the call. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 3572c5a92e8e..d5e2c7050291 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1661,7 +1661,9 @@ static void handle_control_message(struct ports_device *portdev, * If the guest is connected, it'll be interested in * knowing the host connection state changed. */ + spin_lock_irq(&port->inbuf_lock); send_sigio_to_port(port); + spin_unlock_irq(&port->inbuf_lock); break; case VIRTIO_CONSOLE_PORT_NAME: /* @@ -1781,13 +1783,13 @@ static void in_intr(struct virtqueue *vq) if (!port->guest_connected && !is_rproc_serial(port->portdev->vdev)) discard_port_data(port); + /* Send a SIGIO indicating new data in case the process asked for it */ + send_sigio_to_port(port); + spin_unlock_irqrestore(&port->inbuf_lock, flags); wake_up_interruptible(&port->waitqueue); - /* Send a SIGIO indicating new data in case the process asked for it */ - send_sigio_to_port(port); - if (is_console_port(port) && hvc_poll(port->cons.hvc)) hvc_kick(); } -- cgit v1.2.3-58-ga151 From 3f0d0c9b47e09d47fcb755fed786a1ee88e110b5 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 30 Jul 2013 15:54:07 +0930 Subject: virtio: console: cleanup an error message The PTR_ERR(NULL) here is not useful. Signed-off-by: Dan Carpenter Reviewed-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index d5e2c7050291..5b60956edc28 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -2223,10 +2223,8 @@ static int __init init(void) } pdrvdata.debugfs_dir = debugfs_create_dir("virtio-ports", NULL); - if (!pdrvdata.debugfs_dir) { - pr_warning("Error %ld creating debugfs dir for virtio-ports\n", - PTR_ERR(pdrvdata.debugfs_dir)); - } + if (!pdrvdata.debugfs_dir) + pr_warning("Error creating debugfs dir for virtio-ports\n"); INIT_LIST_HEAD(&pdrvdata.consoles); INIT_LIST_HEAD(&pdrvdata.portdevs); -- cgit v1.2.3-58-ga151 From 3b868a4073cdedf395f26d843874414e0e0e9cfd Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 7 Aug 2013 15:54:17 +0930 Subject: virtio: console: prevent use-after-free of port name in port unplug Remove the debugfs path before freeing port->name, to prevent a possible use-after-free. Reported-by: Jason Wang Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index d0e75aa904ac..b79cf3e1b793 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1591,9 +1591,8 @@ static void unplug_port(struct port *port) device_destroy(pdrvdata.class, port->dev->devt); cdev_del(port->cdev); - kfree(port->name); - debugfs_remove(port->debugfs_file); + kfree(port->name); /* * Locks around here are not necessary - a port can't be -- cgit v1.2.3-58-ga151