[PATCH] hw/audio/virtio-sound: fix heap buffer overflow

Volker Rümelin posted 1 patch 2 months, 3 weeks ago
hw/audio/trace-events |  1 -
hw/audio/virtio-snd.c | 24 ------------------------
2 files changed, 25 deletions(-)
[PATCH] hw/audio/virtio-sound: fix heap buffer overflow
Posted by Volker Rümelin 2 months, 3 weeks ago
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>
---
 hw/audio/trace-events |  1 -
 hw/audio/virtio-snd.c | 24 ------------------------
 2 files changed, 25 deletions(-)

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"
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;
-- 
2.35.3


Re: [PATCH] hw/audio/virtio-sound: fix heap buffer overflow
Posted by Philippe Mathieu-Daudé 2 months, 2 weeks ago
On 1/9/24 15:01, Volker Rümelin wrote:
> 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>
> ---
>   hw/audio/trace-events |  1 -
>   hw/audio/virtio-snd.c | 24 ------------------------
>   2 files changed, 25 deletions(-)

Patch queued, thanks.

Re: [PATCH] hw/audio/virtio-sound: fix heap buffer overflow
Posted by Gerd Hoffmann 2 months, 3 weeks ago
On Sun, Sep 01, 2024 at 03:01:12PM GMT, Volker Rümelin wrote:
> 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>

Acked-by: Gerd Hoffmann <kraxel@redhat.com>