__send_control_msg() publishes &portdev->cpkt as the control-out
virtqueue cookie. remove_vqs() walks every virtqueue and passes leftover
cookies to free_buf(), which treats them as struct port_buffer and
reads sgpages.
If a control message is still on c_ovq when the device is unbound,
free_buf() reads past the ports_device object.
KASAN reported slab-out-of-bounds in free_buf():
free_buf
remove_vqs
virtcons_remove
unbind_store
The object was the ports_device allocated in virtcons_probe().
Drain c_ovq without freeing. The packet lives in portdev and is released
with it.
Fixes: a7a69ec0d8e4 ("virtio_console: free buffers after reset")
Signed-off-by: Jia Jia <[email protected]>
---
Changes in v2:
- Guard c_ovq handling with use_multiport() because c_ovq and
c_ovq_lock are only initialized for multiport devices.
drivers/char/virtio_console.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 9a33217c68d9..8e0d10d91763 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1884,12 +1884,27 @@ static const struct file_operations portdev_fops = {
static void remove_vqs(struct ports_device *portdev)
{
struct virtqueue *vq;
+ bool multiport = use_multiport(portdev);
virtio_device_for_each_vq(portdev->vdev, vq) {
struct port_buffer *buf;
+ unsigned int len;
- flush_bufs(vq, true);
- while ((buf = virtqueue_detach_unused_buf(vq)))
- free_buf(buf, true);
+ /*
+ * c_ovq cookies are &portdev->cpkt, not port_buffer.
+ * Detach them but do not free_buf().
+ */
+ if (multiport && vq == portdev->c_ovq) {
+ spin_lock(&portdev->c_ovq_lock);
+ while (virtqueue_get_buf(vq, &len))
+ ;
+ while (virtqueue_detach_unused_buf(vq))
+ ;
+ spin_unlock(&portdev->c_ovq_lock);
+ } else {
+ flush_bufs(vq, true);
+ while ((buf = virtqueue_detach_unused_buf(vq)))
+ free_buf(buf, true);
+ }
cond_resched();
}
portdev->vdev->config->del_vqs(portdev->vdev);
--
2.34.1
