Skip to contentSkip to conversation

virtio-snd fixes

GIT PULL Manos Pitsidianakis 1 participant 7 messages Nov 11

lore
Share & tools
lore

Manos Pitsidianakis

author
#0 lore raw

The following changes since commit
29760efaf96bbe5928f8111881eddbf5ac06439e:

Update version for v11.2.0-rc0 release (2026-11-10 14:47:54 -0800)

are available in the Git repository at:

https://gitlab.com/epilys/qemu.git tags/virtio-snd-fixes-pr1

for you to fetch changes up to 9a0f03f81575168283e56dab51bc0f0a13894092:

virtio-snd: period_bytes divides buffer_bytes (2026-11-11 13:46:50 +0200)

----------------------------------------------------------------
virtio-snd-fixes-pr1
-----BEGIN PGP SIGNATURE-----

iQJPBAABCgA5FiEEzFwbTkSgVvOHcB0QdynHcH9+CdAFAmr0VikbFIAAAAAABAAO bWFudTIsMi41KzEuMTIsMCwzAAoJEHcpx3B/fgnQqiIP/RwyAXlUupo/L8Lgetbx bjOwtNBPZBKlZk6F3A4WNpqXZRLFlnH6GQSH7l0Xporokm8qiVytH50mHJcBFgpi PoICtqNooN5ud9YeK8UWQFXqGLSb4G1V2x6NQyLd1zGOrZkrnNaxRIRu0vN5s8RO R2dcrpYJ5S9IChbkWM/vLo6p9HAeGcD3F+zT1HhRpm7iKCKXZsyCCDGXu4RR6eUM d4Oxviglg1krjxqWy0ZNiohO1I3FOhYrrJiNTwv9jbbv+FXCAs45a5Ve1XauYgRn +8yPocxfFiuX9sobAXirNy084Xmv8kJCYThAucvho19dgEItDUamNhYg/fbsQHDv QZKb0JNtQm5B8xXmIK3zKGSGGdSUw1hn1YmQSgAy4clUpxhRgqCmFw/6I8Eo9Yt0 IOgimj1hYQ9Fq55VNDdEEr16PH8O0b7D/ztToD3pw0ejW50kNIkBz9RrebCNfLTW rShv67h+yhJOUGF4OH1DBDXPFIw9zAMH4ep+tR6CGNi5dK4s7kALZD1noN++NGGl bLYCJXqdAL1o1jkiVG5Qteen4FtX1a5YI88zsnu5AMu2GyxhVmX8XWaPh2+T/vT3 hgaMb0VsNH3pixhDMAU9IFwbP7QT+vugNI+B7QtwHvOXB3f2CrsxghmmZInSoKTn acOa8TNrie14G7F3jEdtqXHk
=j9s7
-----END PGP SIGNATURE-----

----------------------------------------------------------------
Manos Pitsidianakis (6):
virtio-snd: convert virtio_error to error_report
virtio-snd: add pcm_state_to_str()
virtio-snd: reject invalid feats in set_pcm_params
virtio-snd: don't allocate pcm_info zero-count buf
virtio-snd: cap maximum buffer_bytes to half a GiB
virtio-snd: period_bytes divides buffer_bytes

 hw/audio/virtio-snd.c | 91 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 80 insertions(+), 11 deletions(-)
author
#1 lore raw

[PULL 1/6] virtio-snd: convert virtio_error to error_report

virtio_error() also signals a RESET, which is not what we want: we want the guest driver to read the VIRTIO_SND_S_BAD_MSG response.

If a guest sends an invalid command and we do virtio_error() and reply with a BAD_MSG response, the guest needs to detect the device reset otherwise it hangs forever waiting for the response.

Reviewed-by Jim MacArthur <[email protected]>
Signed-off-by Manos Pitsidianakis <[email protected]>
---
 hw/audio/virtio-snd.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)
hw/audio/virtio-snd.c +14 -15
index fb22d79949..bed36203f2 100644
@@ -298,8 +298,8 @@ uint32_t virtio_snd_set_pcm_params(VirtIOSound *s,
 
     stream = virtio_snd_pcm_get_stream(s, stream_id);
     if (!stream) {
-        virtio_error(VIRTIO_DEVICE(s), "invalid stream id: %"PRIu32,
-                     stream_id);
+        error_report("%s invalid stream id: %"PRIu32,
+                     VIRTIO_DEVICE(s)->name, stream_id);
         return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
     }
 

@@ -310,8 +310,8 @@ uint32_t virtio_snd_set_pcm_params(VirtIOSound *s,
     case VIRTIO_SND_PCM_STATE_RELEASED:
         break;
     default:
-        virtio_error(VIRTIO_DEVICE(s), "unexpected stream state: %"PRIu32,
-                     stream->state);
+        error_report("%s unexpected stream state in %s: %"PRIu32,
+                     VIRTIO_DEVICE(s)->name, __func__, stream->state);
         return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
     }
 

@@ -535,8 +535,8 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s, uint32_t stream_id)
     case VIRTIO_SND_PCM_STATE_RELEASED:
         break;
     default:
-        virtio_error(VIRTIO_DEVICE(s), "unexpected stream state: %"PRIu32,
-                     stream->state);
+        error_report("%s unexpected stream state in %s: %"PRIu32,
+                     VIRTIO_DEVICE(s)->name, __func__, stream->state);
         return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
     }
 

@@ -624,8 +624,8 @@ static uint32_t virtio_snd_pcm_start_stop(VirtIOSound *s,
         case VIRTIO_SND_PCM_STATE_STOPPED:
             break;
         default:
-            virtio_error(VIRTIO_DEVICE(s), "unexpected stream state: %"PRIu32,
-                         stream->state);
+            error_report("%s unexpected stream state in %s start: %"PRIu32,
+                         VIRTIO_DEVICE(s)->name, __func__, stream->state);
             return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
         }
 

@@ -635,7 +635,8 @@ static uint32_t virtio_snd_pcm_start_stop(VirtIOSound *s,
         case VIRTIO_SND_PCM_STATE_STARTED:
             break;
         default:
-            virtio_error(VIRTIO_DEVICE(s), "unexpected stream state: %"PRIu32,
+            error_report("%s unexpected stream state in %s stop: %"PRIu32,
+                         VIRTIO_DEVICE(s)->name, __func__,
                          stream->state);
             return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
         }

@@ -724,10 +725,8 @@ static void virtio_snd_handle_pcm_release(VirtIOSound *s,
     trace_virtio_snd_handle_pcm_release(stream_id);
     stream = virtio_snd_pcm_get_stream(s, stream_id);
     if (stream == NULL) {
-        error_report("already released stream %"PRIu32, stream_id);
-        virtio_error(VIRTIO_DEVICE(s),
-                     "already released stream %"PRIu32,
-                     stream_id);
+        error_report("%s already released stream %"PRIu32,
+                     VIRTIO_DEVICE(s)->name, stream_id);
         cmd->resp.code = cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
         return;
     }

@@ -737,8 +736,8 @@ static void virtio_snd_handle_pcm_release(VirtIOSound *s,
     case VIRTIO_SND_PCM_STATE_STOPPED:
         break;
     default:
-        virtio_error(VIRTIO_DEVICE(s), "unexpected stream state: %"PRIu32,
-                     stream->state);
+        error_report("%s unexpected stream state in %s: %"PRIu32,
+                     VIRTIO_DEVICE(s)->name, __func__, stream->state);
         cmd->resp.code = cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
         return;
     }
Signature · 1 line
γαῖα πυρί μιχθήτω
author
#2 lore raw

[PULL 2/6] virtio-snd: add pcm_state_to_str()

In reply to Manos Pitsidianakis

Add macro to print the stream state in a human-understandable state, since this is an internal enum and one would have to inspect the source code to understand what a e.g. "2" state means in an error message.

Reviewed-by Jim MacArthur <[email protected]>
Signed-off-by Manos Pitsidianakis <[email protected]>
---
 hw/audio/virtio-snd.c | 49 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 10 deletions(-)
hw/audio/virtio-snd.c +39 -10
index bed36203f2..f1dc03d1b8 100644
@@ -38,6 +38,26 @@ typedef enum virtio_snd_pcm_state {
     VIRTIO_SND_PCM_STATE_RELEASED,
 } virtio_snd_pcm_state;
 
+static inline const char *pcm_state_to_str(virtio_snd_pcm_state s)
+{
+    switch (s) {
+    case VIRTIO_SND_PCM_STATE_UNINIT:
+        return "UNINIT";
+    case VIRTIO_SND_PCM_STATE_PARAMS_SET:
+        return "PARAMS_SET";
+    case VIRTIO_SND_PCM_STATE_PREPARED:
+        return "PREPARED";
+    case VIRTIO_SND_PCM_STATE_STARTED:
+        return "STARTED";
+    case VIRTIO_SND_PCM_STATE_STOPPED:
+        return "STOPPED";
+    case VIRTIO_SND_PCM_STATE_RELEASED:
+        return "RELEASED";
+    default:
+        return "invalid";
+    }
+}
+
 static inline bool virtio_snd_pcm_state_prepared(virtio_snd_pcm_state s)
 {
     return s > VIRTIO_SND_PCM_STATE_PARAMS_SET &&

@@ -310,8 +330,10 @@ uint32_t virtio_snd_set_pcm_params(VirtIOSound *s,
     case VIRTIO_SND_PCM_STATE_RELEASED:
         break;
     default:
-        error_report("%s unexpected stream state in %s: %"PRIu32,
-                     VIRTIO_DEVICE(s)->name, __func__, stream->state);
+        error_report("%s unexpected stream state in %s: "
+                     "%s (%"PRIu32")", VIRTIO_DEVICE(s)->name,
+                     __func__, pcm_state_to_str(stream->state),
+                     stream->state);
         return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
     }
 

@@ -535,8 +557,10 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s, uint32_t stream_id)
     case VIRTIO_SND_PCM_STATE_RELEASED:
         break;
     default:
-        error_report("%s unexpected stream state in %s: %"PRIu32,
-                     VIRTIO_DEVICE(s)->name, __func__, stream->state);
+        error_report("%s unexpected stream state in %s: "
+                     "%s (%"PRIu32")", VIRTIO_DEVICE(s)->name,
+                     __func__, pcm_state_to_str(stream->state),
+                     stream->state);
         return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
     }
 

@@ -624,8 +648,10 @@ static uint32_t virtio_snd_pcm_start_stop(VirtIOSound *s,
         case VIRTIO_SND_PCM_STATE_STOPPED:
             break;
         default:
-            error_report("%s unexpected stream state in %s start: %"PRIu32,
-                         VIRTIO_DEVICE(s)->name, __func__, stream->state);
+            error_report("%s unexpected stream state in %s start: "
+                         "%s (%"PRIu32")", VIRTIO_DEVICE(s)->name,
+                         __func__, pcm_state_to_str(stream->state),
+                         stream->state);
             return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
         }
 

@@ -635,8 +661,9 @@ static uint32_t virtio_snd_pcm_start_stop(VirtIOSound *s,
         case VIRTIO_SND_PCM_STATE_STARTED:
             break;
         default:
-            error_report("%s unexpected stream state in %s stop: %"PRIu32,
-                         VIRTIO_DEVICE(s)->name, __func__,
+            error_report("%s unexpected stream state in %s stop: "
+                         "%s (%"PRIu32")", VIRTIO_DEVICE(s)->name,
+                         __func__, pcm_state_to_str(stream->state),
                          stream->state);
             return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
         }

@@ -736,8 +763,10 @@ static void virtio_snd_handle_pcm_release(VirtIOSound *s,
     case VIRTIO_SND_PCM_STATE_STOPPED:
         break;
     default:
-        error_report("%s unexpected stream state in %s: %"PRIu32,
-                     VIRTIO_DEVICE(s)->name, __func__, stream->state);
+        error_report("%s unexpected stream state in %s: "
+                     "%s (%"PRIu32")", VIRTIO_DEVICE(s)->name,
+                     __func__, pcm_state_to_str(stream->state),
+                     stream->state);
         cmd->resp.code = cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
         return;
     }
Signature · 1 line
γαῖα πυρί μιχθήτω
author
#3 lore raw

[PULL 6/6] virtio-snd: period_bytes divides buffer_bytes

In reply to Manos Pitsidianakis

Add a check for the VIRTIO spec statement:

5.14.6.6.3.2 Driver Requirements: Stream Parameters

      period_bytes MUST be a divider buffer_bytes, i.e. buffer_bytes % period_bytes == 0.
Reviewed-by Jim MacArthur <[email protected]>
Signed-off-by Manos Pitsidianakis <[email protected]>
---
 hw/audio/virtio-snd.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
hw/audio/virtio-snd.c +11 -0
index a6036d5b75..1cbc4da752 100644
@@ -380,6 +380,17 @@ uint32_t virtio_snd_set_pcm_params(VirtIOSound *s,
         return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
     }
 
+    if (le32_to_cpu(params->buffer_bytes)
+        % le32_to_cpu(params->period_bytes) != 0) {
+        error_report("%s: 5.14.6.6.3.2 period_bytes MUST be a divider of "
+                     "buffer_bytes, i.e. buffer_bytes %% period_bytes == 0. "
+                     "Got buffer_bytes = %"PRIu32", period_bytes = %"PRIu32,
+                     VIRTIO_DEVICE(s)->name,
+                     le32_to_cpu(params->buffer_bytes),
+                     le32_to_cpu(params->period_bytes));
+        return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
+    }
+
     st_params = virtio_snd_pcm_get_params(s, stream_id);
 
     st_params->buffer_bytes = le32_to_cpu(params->buffer_bytes);
Signature · 1 line
γαῖα πυρί μιχθήτω
author
#4 lore raw

[PULL 4/6] virtio-snd: don't allocate pcm_info zero-count buf

In reply to Manos Pitsidianakis

g_new0() might return NULL, which is UB if passed to a memcpy implementation that does not expect it (not in the usual case).

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3778

Reviewed-by Jim MacArthur <[email protected]>
Signed-off-by Manos Pitsidianakis <[email protected]>
---
 hw/audio/virtio-snd.c | 5 +++++
 1 file changed, 5 insertions(+)
hw/audio/virtio-snd.c +5 -0
index 89406f0a3a..a7d749f24f 100644
@@ -267,6 +267,11 @@ static void virtio_snd_handle_pcm_info(VirtIOSound *s,
         return;
     }
 
+    if (count == 0) {
+        cmd->payload_size = 0;
+        cmd->resp.code = cpu_to_le32(VIRTIO_SND_S_OK);
+        return;
+    }
     pcm_info = g_new0(virtio_snd_pcm_info, count);
     for (uint32_t i = 0; i < count; i++) {
         stream_id = i + start_id;
Signature · 1 line
γαῖα πυρί μιχθήτω
author
#5 lore raw

[PULL 3/6] virtio-snd: reject invalid feats in set_pcm_params

In reply to Manos Pitsidianakis

Do not blindly accept features for a stream, check them against what we had reported to the guest with pcm_info.

Reviewed-by Jim MacArthur <[email protected]>
Signed-off-by Manos Pitsidianakis <[email protected]>
---
 hw/audio/virtio-snd.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
hw/audio/virtio-snd.c +12 -0
index f1dc03d1b8..89406f0a3a 100644
@@ -351,6 +351,18 @@ uint32_t virtio_snd_set_pcm_params(VirtIOSound *s,
         error_report("Stream rate is not supported.");
         return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP);
     }
+    if ((le32_to_cpu(params->features) & ~(stream->info.features)) > 0) {
+        error_report("%s: invalid stream features. "
+                     "Stream %d offers 0x%"PRIx32", "
+                     "guest requested 0x%"PRIx32
+                     " (invalid: 0x%"PRIx32")",
+                     VIRTIO_DEVICE(s)->name, stream_id,
+                     stream->info.features,
+                     le32_to_cpu(params->features),
+                     le32_to_cpu(params->features)
+                     & ~(stream->info.features));
+        return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
+    }
 
     st_params = virtio_snd_pcm_get_params(s, stream_id);
Signature · 1 line
γαῖα πυρί μιχθήτω
author
#6 lore raw

[PULL 5/6] virtio-snd: cap maximum buffer_bytes to half a GiB

In reply to Manos Pitsidianakis

A faulty/malicious guest can try to exhaust host memory. Cap the buffer size to 500 MiB.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3587

Reviewed-by Jim MacArthur <[email protected]>
Signed-off-by Manos Pitsidianakis <[email protected]>
---
 hw/audio/virtio-snd.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
hw/audio/virtio-snd.c +13 -0
index a7d749f24f..a6036d5b75 100644
@@ -17,6 +17,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/iov.h"
+#include "qemu/units.h"
 #include "qemu/log.h"
 #include "qemu/error-report.h"
 #include "system/runstate.h"

@@ -24,6 +25,8 @@
 #include "qapi/error.h"
 #include "hw/audio/virtio-snd.h"
 
+#define MAX_BUFFER_MIB 500
+
 #define VIRTIO_SOUND_JACK_DEFAULT 0
 #define VIRTIO_SOUND_STREAM_DEFAULT 2
 #define VIRTIO_SOUND_CHMAP_DEFAULT 0

@@ -368,6 +371,14 @@ uint32_t virtio_snd_set_pcm_params(VirtIOSound *s,
                      & ~(stream->info.features));
         return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
     }
+    if (le32_to_cpu(params->buffer_bytes) > MAX_BUFFER_MIB * MiB) {
+        error_report("%s: given buffer_bytes value %"PRIu32" "
+                     "is too big: over %d MiB",
+                     VIRTIO_DEVICE(s)->name,
+                     le32_to_cpu(params->buffer_bytes),
+                     MAX_BUFFER_MIB);
+        return cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
+    }
 
     st_params = virtio_snd_pcm_get_params(s, stream_id);
 

@@ -1037,6 +1048,8 @@ static void virtio_snd_handle_tx_xfer(VirtIODevice *vdev, VirtQueue *vq)
         }
 
         stream = &vsnd->streams[stream_id];
+        size = MIN(stream->params.buffer_bytes, size);
+
         if (stream->info.direction != VIRTIO_SND_D_OUTPUT) {
             goto tx_err;
         }
Signature · 1 line
γαῖα πυρί μιχθήτω