[PATCH v2 0/6] Various virtio-snd fixes again

[PATCH v2 0/6] Various virtio-snd fixes again

[Manos Pitsidianakis]

Some various virtio sound fixes.

---
Changes in v2:
- Moved fixup hunk to the patch it was meant to fixup
- Link to v1: https://lore.kernel.org/qemu-devel/20261110-virtio-snd-fixes-v1-0-544d6f9eca38@linaro.org

To: qemu-devel@nongnu.org
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

---
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(-)
---
base-commit: 723622bedb0b423de7d1c61ebbe6e7a71934949c
change-id: 20261110-virtio-snd-fixes-f770a01c9960

Best regards,
--  
Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

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

[Manos Pitsidianakis]

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.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 hw/audio/virtio-snd.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index fb22d79949..bed36203f2 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -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;
     }

-- 
2.56.0

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

[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.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 hw/audio/virtio-snd.c | 49 +++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index bed36203f2..f1dc03d1b8 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -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;
     }

-- 
2.56.0

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

[Manos Pitsidianakis]

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

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 hw/audio/virtio-snd.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index f1dc03d1b8..89406f0a3a 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -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);
 

-- 
2.56.0

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

[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
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 hw/audio/virtio-snd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 89406f0a3a..a7d749f24f 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -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;

-- 
2.56.0

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

[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
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 hw/audio/virtio-snd.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index a7d749f24f..a6036d5b75 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -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;
         }

-- 
2.56.0

[PATCH v2 6/6] virtio-snd: period_bytes divides buffer_bytes

[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.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 hw/audio/virtio-snd.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index a6036d5b75..1cbc4da752 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -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);

-- 
2.56.0

Re: [PATCH v2 1/6] virtio-snd: convert virtio_error to error_report

[Jim MacArthur]

On Tue, Nov 10, 2026 at 02:39:30PM +0200, Manos Pitsidianakis wrote:
> 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.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>  hw/audio/virtio-snd.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index fb22d79949..bed36203f2 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -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;
>      }

Reviewed-by: Jim MacArthur <jim.macarthur@linaro.org>

Re: [PATCH v2 2/6] virtio-snd: add pcm_state_to_str()

[Jim MacArthur]

On Tue, Nov 10, 2026 at 02:39:31PM +0200, Manos Pitsidianakis wrote:
> 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.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>  hw/audio/virtio-snd.c | 49 +++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index bed36203f2..f1dc03d1b8 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -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;
>      }
> 

Reviewed-by: Jim MacArthur <jim.macarthur@linaro.org>

Re: [PATCH v2 3/6] virtio-snd: reject invalid feats in set_pcm_params

[Jim MacArthur]

On Tue, Nov 10, 2026 at 02:39:32PM +0200, Manos Pitsidianakis wrote:
> Do not blindly accept features for a stream, check them against what we
> had reported to the guest with pcm_info.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>  hw/audio/virtio-snd.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index f1dc03d1b8..89406f0a3a 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -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);
>  
> 
> -- 
> 2.56.0
> 
> 

Reviewed-by: Jim MacArthur <jim.macarthur@linaro.org>

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

[Jim MacArthur]

On Tue, Nov 10, 2026 at 02:39:33PM +0200, Manos Pitsidianakis wrote:
> 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
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>  hw/audio/virtio-snd.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index 89406f0a3a..a7d749f24f 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -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;
> 
> -- 
> 2.56.0
> 
> 

Reviewed-by: Jim MacArthur <jim.macarthur@linaro.org>

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

[Jim MacArthur]

On Tue, Nov 10, 2026 at 02:39:34PM +0200, Manos Pitsidianakis wrote:
> 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
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>  hw/audio/virtio-snd.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index a7d749f24f..a6036d5b75 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -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;
>          }
> 
> -- 
> 2.56.0
> 
> 

Reviewed-by: Jim MacArthur <jim.macarthur@linaro.org>

Re: [PATCH v2 6/6] virtio-snd: period_bytes divides buffer_bytes

[Jim MacArthur]

On Tue, Nov 10, 2026 at 02:39:35PM +0200, Manos Pitsidianakis wrote:
> 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.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>  hw/audio/virtio-snd.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index a6036d5b75..1cbc4da752 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -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);
> 
> -- 
> 2.56.0

Reviewed-by: Jim MacArthur <jim.macarthur@linaro.org>