On Thu, Jan 19, 2023 at 03:57:16PM +0200, Alexander Shishkin wrote:
> From: Andi Kleen <ak@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> Cc: Amit Shah <amit@xxxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
> 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
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
