From ee4c973ea162a299fbb18686d1f0a19670544854 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Mon, 8 Jul 2024 10:09:49 +0300 Subject: [PATCH 1/2] virtio-snd: add max size bounds check in input cb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit 98e77e3dd8dd6e7aa9a7dffa60f49c8c8a49d4e3 upstream When reading input audio in the virtio-snd input callback, virtio_snd_pcm_in_cb(), we do not check whether the iov can actually fit the data buffer. This is because we use the buffer->size field as a total-so-far accumulator instead of byte-size-left like in TX buffers. This triggers an out of bounds write if the size of the virtio queue element is equal to virtio_snd_pcm_status, which makes the available space for audio data zero. This commit adds a check for reaching the maximum buffer size before attempting any writes. Reported-by: Zheyu Ma Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2427 Signed-off-by: Manos Pitsidianakis Message-Id: Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Signed-off-by: Xuchun Shang --- hw/audio/virtio-snd.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c index 137fa77a01c..15986af41e2 100644 --- a/hw/audio/virtio-snd.c +++ b/hw/audio/virtio-snd.c @@ -1274,7 +1274,7 @@ static void virtio_snd_pcm_in_cb(void *data, int available) { VirtIOSoundPCMStream *stream = data; VirtIOSoundPCMBuffer *buffer; - size_t size; + size_t size, max_size; WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { while (!QSIMPLEQ_EMPTY(&stream->queue)) { @@ -1288,7 +1288,12 @@ static void virtio_snd_pcm_in_cb(void *data, int available) continue; } + max_size = iov_size(buffer->elem->in_sg, buffer->elem->in_num); for (;;) { + if (buffer->size >= max_size) { + return_rx_buffer(stream, buffer); + break; + } size = AUD_read(stream->voice.in, buffer->data + buffer->size, MIN(available, (stream->params.period_bytes - -- Gitee From 040ca8080f6bc91ae676ba5c5abcc20edf863487 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Thu, 11 Jul 2024 10:38:49 +0300 Subject: [PATCH 2/2] virtio-snd: check for invalid param shift operands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit 9b6083465fb8311f2410615f8303a41f580a2a20 upstream When setting the parameters of a PCM stream, we compute the bit flag with the format and rate values as shift operand to check if they are set in supported_formats and supported_rates. If the guest provides a format/rate value which when shifting 1 results in a value bigger than the number of bits in supported_formats/supported_rates, we must report an error. Previously, this ended up triggering the not reached assertions later when converting to internal QEMU values. Reported-by: Zheyu Ma Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2416 Signed-off-by: Manos Pitsidianakis Message-Id: Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Signed-off-by: Xuchun Shang --- hw/audio/virtio-snd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c index 15986af41e2..3ac5f787141 100644 --- a/hw/audio/virtio-snd.c +++ b/hw/audio/virtio-snd.c @@ -281,11 +281,13 @@ uint32_t virtio_snd_set_pcm_params(VirtIOSound *s, error_report("Number of channels is not supported."); return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP); } - if (!(supported_formats & BIT(params->format))) { + if (BIT(params->format) > sizeof(supported_formats) || + !(supported_formats & BIT(params->format))) { error_report("Stream format is not supported."); return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP); } - if (!(supported_rates & BIT(params->rate))) { + if (BIT(params->rate) > sizeof(supported_rates) || + !(supported_rates & BIT(params->rate))) { error_report("Stream rate is not supported."); return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP); } -- Gitee