[PULL 16/18] hw/audio/virtio-sound: fix heap buffer overflow

Michael S. Tsirkin posted 18 patches 2 months, 1 week ago
There is a newer version of this series
[PULL 16/18] hw/audio/virtio-sound: fix heap buffer overflow
Posted by Michael S. Tsirkin 2 months, 1 week ago
From: Volker Rümelin <vr_qemu@t-online.de>

Currently, the guest may write to the device configuration space,
whereas the virtio sound device specification in chapter 5.14.4
clearly states that the fields in the device configuration space
are driver-read-only.

Remove the set_config function from the virtio_snd class.

This also prevents a heap buffer overflow. See QEMU issue #2296.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2296
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20240901130112.8242-1-vr_qemu@t-online.de>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/audio/virtio-snd.c | 24 ------------------------
 hw/audio/trace-events |  1 -
 2 files changed, 25 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index d1cf5eb445..69838181dd 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -107,29 +107,6 @@ virtio_snd_get_config(VirtIODevice *vdev, uint8_t *config)
 
 }
 
-static void
-virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
-{
-    VirtIOSound *s = VIRTIO_SND(vdev);
-    const virtio_snd_config *sndconfig =
-        (const virtio_snd_config *)config;
-
-
-   trace_virtio_snd_set_config(vdev,
-                               s->snd_conf.jacks,
-                               sndconfig->jacks,
-                               s->snd_conf.streams,
-                               sndconfig->streams,
-                               s->snd_conf.chmaps,
-                               sndconfig->chmaps);
-
-    memcpy(&s->snd_conf, sndconfig, sizeof(virtio_snd_config));
-    le32_to_cpus(&s->snd_conf.jacks);
-    le32_to_cpus(&s->snd_conf.streams);
-    le32_to_cpus(&s->snd_conf.chmaps);
-
-}
-
 static void
 virtio_snd_pcm_buffer_free(VirtIOSoundPCMBuffer *buffer)
 {
@@ -1400,7 +1377,6 @@ static void virtio_snd_class_init(ObjectClass *klass, void *data)
     vdc->realize = virtio_snd_realize;
     vdc->unrealize = virtio_snd_unrealize;
     vdc->get_config = virtio_snd_get_config;
-    vdc->set_config = virtio_snd_set_config;
     vdc->get_features = get_features;
     vdc->reset = virtio_snd_reset;
     vdc->legacy_features = 0;
diff --git a/hw/audio/trace-events b/hw/audio/trace-events
index b1870ff224..b8ef572767 100644
--- a/hw/audio/trace-events
+++ b/hw/audio/trace-events
@@ -41,7 +41,6 @@ asc_update_irq(int irq, int a, int b) "set IRQ to %d (A: 0x%x B: 0x%x)"
 
 #virtio-snd.c
 virtio_snd_get_config(void *vdev, uint32_t jacks, uint32_t streams, uint32_t chmaps) "snd %p: get_config jacks=%"PRIu32" streams=%"PRIu32" chmaps=%"PRIu32""
-virtio_snd_set_config(void *vdev, uint32_t jacks, uint32_t new_jacks, uint32_t streams, uint32_t new_streams, uint32_t chmaps, uint32_t new_chmaps) "snd %p: set_config jacks from %"PRIu32"->%"PRIu32", streams from %"PRIu32"->%"PRIu32", chmaps from %"PRIu32"->%"PRIu32
 virtio_snd_get_features(void *vdev, uint64_t features) "snd %p: get_features 0x%"PRIx64
 virtio_snd_vm_state_running(void) "vm state running"
 virtio_snd_vm_state_stopped(void) "vm state stopped"
-- 
MST


Re: [PULL 16/18] hw/audio/virtio-sound: fix heap buffer overflow
Posted by Volker Rümelin 2 months, 1 week ago
Cc: qemu-stable@nongnu.org

stable-8.2, stable-9.0 and stable-9.1

> From: Volker Rümelin <vr_qemu@t-online.de>
>
> Currently, the guest may write to the device configuration space,
> whereas the virtio sound device specification in chapter 5.14.4
> clearly states that the fields in the device configuration space
> are driver-read-only.
>
> Remove the set_config function from the virtio_snd class.
>
> This also prevents a heap buffer overflow. See QEMU issue #2296.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2296
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> Message-Id: <20240901130112.8242-1-vr_qemu@t-online.de>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/audio/virtio-snd.c | 24 ------------------------
>  hw/audio/trace-events |  1 -
>  2 files changed, 25 deletions(-)
>
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index d1cf5eb445..69838181dd 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -107,29 +107,6 @@ virtio_snd_get_config(VirtIODevice *vdev, uint8_t *config)
>  
>  }
>  
> -static void
> -virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
> -{
> -    VirtIOSound *s = VIRTIO_SND(vdev);
> -    const virtio_snd_config *sndconfig =
> -        (const virtio_snd_config *)config;
> -
> -
> -   trace_virtio_snd_set_config(vdev,
> -                               s->snd_conf.jacks,
> -                               sndconfig->jacks,
> -                               s->snd_conf.streams,
> -                               sndconfig->streams,
> -                               s->snd_conf.chmaps,
> -                               sndconfig->chmaps);
> -
> -    memcpy(&s->snd_conf, sndconfig, sizeof(virtio_snd_config));
> -    le32_to_cpus(&s->snd_conf.jacks);
> -    le32_to_cpus(&s->snd_conf.streams);
> -    le32_to_cpus(&s->snd_conf.chmaps);
> -
> -}
> -
>  static void
>  virtio_snd_pcm_buffer_free(VirtIOSoundPCMBuffer *buffer)
>  {
> @@ -1400,7 +1377,6 @@ static void virtio_snd_class_init(ObjectClass *klass, void *data)
>      vdc->realize = virtio_snd_realize;
>      vdc->unrealize = virtio_snd_unrealize;
>      vdc->get_config = virtio_snd_get_config;
> -    vdc->set_config = virtio_snd_set_config;
>      vdc->get_features = get_features;
>      vdc->reset = virtio_snd_reset;
>      vdc->legacy_features = 0;
> diff --git a/hw/audio/trace-events b/hw/audio/trace-events
> index b1870ff224..b8ef572767 100644
> --- a/hw/audio/trace-events
> +++ b/hw/audio/trace-events
> @@ -41,7 +41,6 @@ asc_update_irq(int irq, int a, int b) "set IRQ to %d (A: 0x%x B: 0x%x)"
>  
>  #virtio-snd.c
>  virtio_snd_get_config(void *vdev, uint32_t jacks, uint32_t streams, uint32_t chmaps) "snd %p: get_config jacks=%"PRIu32" streams=%"PRIu32" chmaps=%"PRIu32""
> -virtio_snd_set_config(void *vdev, uint32_t jacks, uint32_t new_jacks, uint32_t streams, uint32_t new_streams, uint32_t chmaps, uint32_t new_chmaps) "snd %p: set_config jacks from %"PRIu32"->%"PRIu32", streams from %"PRIu32"->%"PRIu32", chmaps from %"PRIu32"->%"PRIu32
>  virtio_snd_get_features(void *vdev, uint64_t features) "snd %p: get_features 0x%"PRIx64
>  virtio_snd_vm_state_running(void) "vm state running"
>  virtio_snd_vm_state_stopped(void) "vm state stopped"