lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <27e87df7-2835-7f3e-c3da-76b844dbd780@redhat.com> Date: Tue, 24 Apr 2018 10:40:18 +0800 From: Jason Wang <jasowang@...hat.com> To: "Michael S. Tsirkin" <mst@...hat.com>, linux-kernel@...r.kernel.org Cc: Amit Shah <amit@...nel.org>, Arnd Bergmann <arnd@...db.de>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, virtualization@...ts.linux-foundation.org, stable@...r.kernel.org, Tiwei Bie <tiwei.bie@...el.com>, stable@...nel.org Subject: Re: [PATCH 3/6] virtio_console: free buffers after reset On 2018年04月21日 02:18, Michael S. Tsirkin wrote: > Console driver is out of spec. The spec says: > A driver MUST NOT decrement the available idx on a live > virtqueue (ie. there is no way to “unexpose” buffers). > and it does exactly that by trying to detach unused buffers > without doing a device reset first. > > Defer detaching the buffers until device unplug. > > Of course this means we might get an interrupt for > a vq without an attached port now. Handle that by > discarding the consumed buffer. > > Reported-by: Tiwei Bie <tiwei.bie@...el.com> > Fixes: b3258ff1d6 ("virtio: Decrement avail idx on buffer detach") > CC: stable@...nel.org > Signed-off-by: Michael S. Tsirkin <mst@...hat.com> I wonder whether or not we can have some BUG_ON() in virtqueue_detach_unused_buf() to detect such bugs (e.g by checking status?). Thanks > --- > drivers/char/virtio_console.c | 49 +++++++++++++++++++++---------------------- > 1 file changed, 24 insertions(+), 25 deletions(-) > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > index 3e56f32..26a66ff 100644 > --- a/drivers/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -1402,7 +1402,6 @@ static int add_port(struct ports_device *portdev, u32 id) > { > char debugfs_name[16]; > struct port *port; > - struct port_buffer *buf; > dev_t devt; > unsigned int nr_added_bufs; > int err; > @@ -1513,8 +1512,6 @@ static int add_port(struct ports_device *portdev, u32 id) > return 0; > > free_inbufs: > - while ((buf = virtqueue_detach_unused_buf(port->in_vq))) > - free_buf(buf, true); > free_device: > device_destroy(pdrvdata.class, port->dev->devt); > free_cdev: > @@ -1539,34 +1536,14 @@ static void remove_port(struct kref *kref) > > 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); > spin_unlock_irq(&port->inbuf_lock); > > - /* Remove buffers we queued up for the Host to send us data in. */ > - do { > - spin_lock_irq(&port->inbuf_lock); > - buf = virtqueue_detach_unused_buf(port->in_vq); > - spin_unlock_irq(&port->inbuf_lock); > - if (buf) > - free_buf(buf, true); > - } while (buf); > - > spin_lock_irq(&port->outvq_lock); > reclaim_consumed_buffers(port); > spin_unlock_irq(&port->outvq_lock); > - > - /* Free pending buffers from the out-queue. */ > - do { > - spin_lock_irq(&port->outvq_lock); > - buf = virtqueue_detach_unused_buf(port->out_vq); > - spin_unlock_irq(&port->outvq_lock); > - if (buf) > - free_buf(buf, true); > - } while (buf); > } > > /* > @@ -1791,13 +1768,24 @@ static void control_work_handler(struct work_struct *work) > spin_unlock(&portdev->c_ivq_lock); > } > > +static void flush_bufs(struct virtqueue *vq, bool can_sleep) > +{ > + struct port_buffer *buf; > + unsigned int len; > + > + while ((buf = virtqueue_get_buf(vq, &len))) > + free_buf(buf, can_sleep); > +} > + > static void out_intr(struct virtqueue *vq) > { > struct port *port; > > port = find_port_by_vq(vq->vdev->priv, vq); > - if (!port) > + if (!port) { > + flush_bufs(vq, false); > return; > + } > > wake_up_interruptible(&port->waitqueue); > } > @@ -1808,8 +1796,10 @@ static void in_intr(struct virtqueue *vq) > unsigned long flags; > > port = find_port_by_vq(vq->vdev->priv, vq); > - if (!port) > + if (!port) { > + flush_bufs(vq, false); > return; > + } > > spin_lock_irqsave(&port->inbuf_lock, flags); > port->inbuf = get_inbuf(port); > @@ -1984,6 +1974,15 @@ static const struct file_operations portdev_fops = { > > static void remove_vqs(struct ports_device *portdev) > { > + struct virtqueue *vq; > + > + virtio_device_for_each_vq(portdev->vdev, vq) { > + struct port_buffer *buf; > + > + flush_bufs(vq, true); > + while ((buf = virtqueue_detach_unused_buf(vq))) > + free_buf(buf, true); > + } > portdev->vdev->config->del_vqs(portdev->vdev); > kfree(portdev->in_vqs); > kfree(portdev->out_vqs);
Powered by blists - more mailing lists