Michael S. Tsirkin
2023-Jan-20 13:01 UTC
[PATCH v1 1/6] virtio console: Harden multiport against invalid host input
On Thu, Jan 19, 2023 at 03:57:16PM +0200, Alexander Shishkin wrote:> From: Andi Kleen <ak at linux.intel.com> > > It's possible for the host to set the multiport flag, but pass in > 0 multiports, which results in: > > BUG: KASAN: slab-out-of-bounds in init_vqs+0x244/0x6c0 drivers/char/virtio_console.c:1878 > Write of size 8 at addr ffff888001cc24a0 by task swapper/1 > > CPU: 0 PID: 1 Comm: swapper Not tainted 5.15.0-rc1-140273-gaab0bb9fbaa1-dirty #588 > Call Trace: > init_vqs+0x244/0x6c0 drivers/char/virtio_console.c:1878 > virtcons_probe+0x1a3/0x5b0 drivers/char/virtio_console.c:2042 > virtio_dev_probe+0x2b9/0x500 drivers/virtio/virtio.c:263 > call_driver_probe drivers/base/dd.c:515 > really_probe+0x1c9/0x5b0 drivers/base/dd.c:601 > really_probe_debug drivers/base/dd.c:694 > __driver_probe_device+0x10d/0x1f0 drivers/base/dd.c:754 > driver_probe_device+0x68/0x150 drivers/base/dd.c:786 > __driver_attach+0xca/0x200 drivers/base/dd.c:1145 > bus_for_each_dev+0x108/0x190 drivers/base/bus.c:301 > driver_attach+0x30/0x40 drivers/base/dd.c:1162 > bus_add_driver+0x325/0x3c0 drivers/base/bus.c:618 > driver_register+0xf3/0x1d0 drivers/base/driver.c:171 > ... > > Add a suitable sanity check. > > Signed-off-by: Andi Kleen <ak at linux.intel.com> > Signed-off-by: Alexander Shishkin <alexander.shishkin at linux.intel.com> > Cc: Amit Shah <amit at kernel.org> > Cc: Arnd Bergmann <arnd at arndb.de> > Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org> > --- > drivers/char/virtio_console.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > index 6a821118d553..f4fd5fe7cd3a 100644 > --- a/drivers/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -1843,6 +1843,9 @@ static int init_vqs(struct ports_device *portdev) > int err; > > nr_ports = portdev->max_nr_ports; > + if (use_multiport(portdev) && nr_ports < 1) > + return -EINVAL; > + > nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2; > > vqs = kmalloc_array(nr_queues, sizeof(struct virtqueue *), GFP_KERNEL);Weird. Don't we already check for that? /* Don't test MULTIPORT at all if we're rproc: not a valid feature! */ if (!is_rproc_serial(vdev) && virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT, struct virtio_console_config, max_nr_ports, &portdev->max_nr_ports) == 0) { if (portdev->max_nr_ports == 0 || portdev->max_nr_ports > VIRTCONS_MAX_PORTS) { dev_err(&vdev->dev, "Invalidate max_nr_ports %d", portdev->max_nr_ports); err = -EINVAL; goto free; } multiport = true; }> -- > 2.39.0