[v1,4/4] virtio_snd_set_config: validate and re-setup card

Commit Message

Manos Pitsidianakis April 22, 2024, 12:52 p.m. UTC

Validate new configuration values and re-setup audio card.

Changing the number of streams via virtio_snd_set_config() did not
re-configure the audio card, leaving it in an invalid state. This can be
demonstrated by this heap buffer overflow:

```shell
cat << EOF | qemu-system-x86_64 -display none \
-machine accel=qtest -m 512M -machine q35 -device \
virtio-sound,audiodev=my_audiodev,streams=2 -audiodev \
alsa,id=my_audiodev -qtest stdio
outl 0xcf8 0x80001804
outw 0xcfc 0x06
outl 0xcf8 0x80001820
outl 0xcfc 0xe0008000
write 0xe0008016 0x1 0x03
write 0xe0008020 0x4 0x00901000
write 0xe0008028 0x4 0x00a01000
write 0xe000801c 0x1 0x01
write 0xe000a004 0x1 0x40
write 0x10c000 0x1 0x02
write 0x109001 0x1 0xc0
write 0x109002 0x1 0x10
write 0x109008 0x1 0x04
write 0x10a002 0x1 0x01
write 0xe000b00d 0x1 0x00
EOF
```

When built with `--enable-sanitizers`, QEMU prints this error:

  ERROR: AddressSanitizer: heap-buffer-overflow [..snip..] in
  virtio_snd_handle_rx_xfer().

Closes #2296.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2296
Reported-by: Zheyu Ma <[e-mail]>
Suggested-by: Zheyu Ma <[e-mail]>
Signed-off-by: Manos Pitsidianakis <[e-mail]>
---
 hw/audio/virtio-snd.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

Patch

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index a9cfaea046..d47af2ed69 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -36,7 +36,11 @@  static void virtio_snd_pcm_out_cb(void *data, int available);
 static void virtio_snd_process_cmdq(VirtIOSound *s);
 static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream);
 static void virtio_snd_pcm_in_cb(void *data, int available);
+static bool virtio_snd_setup(VirtIOSound *vsnd, Error **errp);
+static void virtio_snd_unsetup(VirtIOSound *vsnd);
 static void virtio_snd_unrealize(DeviceState *dev);
+static bool virtio_snd_is_config_valid(virtio_snd_config snd_conf,
+                                       Error **errp);
 
 static uint32_t supported_formats = BIT(VIRTIO_SND_PCM_FMT_S8)
                                   | BIT(VIRTIO_SND_PCM_FMT_U8)
@@ -111,23 +115,26 @@  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;
+    virtio_snd_config new_value =
+        *(const virtio_snd_config *)config;
 
+    le32_to_cpus(&new_value.jacks);
+    le32_to_cpus(&new_value.streams);
+    le32_to_cpus(&new_value.chmaps);
 
-   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);
+    trace_virtio_snd_set_config(vdev,
+                                s->snd_conf.jacks,
+                                new_value.jacks,
+                                s->snd_conf.streams,
+                                new_value.streams,
+                                s->snd_conf.chmaps,
+                                new_value.chmaps);
 
+    if (virtio_snd_is_config_valid(new_value, &error_warn)) {
+        virtio_snd_unsetup(s);
+        s->snd_conf = new_value;
+        virtio_snd_setup(s, &error_fatal);
+    }
 }
 
 static void