* [PATCH v2 1/6] virtio-snd: convert virtio_error to error_report
2026-11-10 12:39 [PATCH v2 0/6] Various virtio-snd fixes again Manos Pitsidianakis
@ 2026-11-10 12:39 ` Manos Pitsidianakis
2026-11-11 10:46 ` Jim MacArthur
2026-11-10 12:39 ` [PATCH v2 2/6] virtio-snd: add pcm_state_to_str() Manos Pitsidianakis
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Manos Pitsidianakis @ 2026-11-10 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Michael S. Tsirkin, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/6] virtio-snd: convert virtio_error to error_report
2026-11-10 12:39 ` [PATCH v2 1/6] virtio-snd: convert virtio_error to error_report Manos Pitsidianakis
@ 2026-11-11 10:46 ` Jim MacArthur
0 siblings, 0 replies; 13+ messages in thread
From: Jim MacArthur @ 2026-11-11 10:46 UTC (permalink / raw)
To: Manos Pitsidianakis; +Cc: qemu-devel, Gerd Hoffmann, Michael S. Tsirkin
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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/6] virtio-snd: add pcm_state_to_str()
2026-11-10 12:39 [PATCH v2 0/6] Various virtio-snd fixes again Manos Pitsidianakis
2026-11-10 12:39 ` [PATCH v2 1/6] virtio-snd: convert virtio_error to error_report Manos Pitsidianakis
@ 2026-11-10 12:39 ` Manos Pitsidianakis
2026-11-11 10:47 ` Jim MacArthur
2026-11-10 12:39 ` [PATCH v2 3/6] virtio-snd: reject invalid feats in set_pcm_params Manos Pitsidianakis
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Manos Pitsidianakis @ 2026-11-10 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Michael S. Tsirkin, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/6] virtio-snd: add pcm_state_to_str()
2026-11-10 12:39 ` [PATCH v2 2/6] virtio-snd: add pcm_state_to_str() Manos Pitsidianakis
@ 2026-11-11 10:47 ` Jim MacArthur
0 siblings, 0 replies; 13+ messages in thread
From: Jim MacArthur @ 2026-11-11 10:47 UTC (permalink / raw)
To: Manos Pitsidianakis; +Cc: qemu-devel, Gerd Hoffmann, Michael S. Tsirkin
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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/6] virtio-snd: reject invalid feats in set_pcm_params
2026-11-10 12:39 [PATCH v2 0/6] Various virtio-snd fixes again Manos Pitsidianakis
2026-11-10 12:39 ` [PATCH v2 1/6] virtio-snd: convert virtio_error to error_report Manos Pitsidianakis
2026-11-10 12:39 ` [PATCH v2 2/6] virtio-snd: add pcm_state_to_str() Manos Pitsidianakis
@ 2026-11-10 12:39 ` Manos Pitsidianakis
2026-11-11 10:47 ` Jim MacArthur
2026-11-10 12:39 ` [PATCH v2 4/6] virtio-snd: don't allocate pcm_info zero-count buf Manos Pitsidianakis
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Manos Pitsidianakis @ 2026-11-10 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Michael S. Tsirkin, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/6] virtio-snd: reject invalid feats in set_pcm_params
2026-11-10 12:39 ` [PATCH v2 3/6] virtio-snd: reject invalid feats in set_pcm_params Manos Pitsidianakis
@ 2026-11-11 10:47 ` Jim MacArthur
0 siblings, 0 replies; 13+ messages in thread
From: Jim MacArthur @ 2026-11-11 10:47 UTC (permalink / raw)
To: Manos Pitsidianakis; +Cc: qemu-devel, Gerd Hoffmann, Michael S. Tsirkin
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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 4/6] virtio-snd: don't allocate pcm_info zero-count buf
2026-11-10 12:39 [PATCH v2 0/6] Various virtio-snd fixes again Manos Pitsidianakis
` (2 preceding siblings ...)
2026-11-10 12:39 ` [PATCH v2 3/6] virtio-snd: reject invalid feats in set_pcm_params Manos Pitsidianakis
@ 2026-11-10 12:39 ` Manos Pitsidianakis
2026-11-11 10:47 ` Jim MacArthur
2026-11-10 12:39 ` [PATCH v2 5/6] virtio-snd: cap maximum buffer_bytes to half a GiB Manos Pitsidianakis
2026-11-10 12:39 ` [PATCH v2 6/6] virtio-snd: period_bytes divides buffer_bytes Manos Pitsidianakis
5 siblings, 1 reply; 13+ messages in thread
From: Manos Pitsidianakis @ 2026-11-10 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Michael S. Tsirkin, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 4/6] virtio-snd: don't allocate pcm_info zero-count buf
2026-11-10 12:39 ` [PATCH v2 4/6] virtio-snd: don't allocate pcm_info zero-count buf Manos Pitsidianakis
@ 2026-11-11 10:47 ` Jim MacArthur
0 siblings, 0 replies; 13+ messages in thread
From: Jim MacArthur @ 2026-11-11 10:47 UTC (permalink / raw)
To: Manos Pitsidianakis; +Cc: qemu-devel, Gerd Hoffmann, Michael S. Tsirkin
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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 5/6] virtio-snd: cap maximum buffer_bytes to half a GiB
2026-11-10 12:39 [PATCH v2 0/6] Various virtio-snd fixes again Manos Pitsidianakis
` (3 preceding siblings ...)
2026-11-10 12:39 ` [PATCH v2 4/6] virtio-snd: don't allocate pcm_info zero-count buf Manos Pitsidianakis
@ 2026-11-10 12:39 ` Manos Pitsidianakis
2026-11-11 10:48 ` Jim MacArthur
2026-11-10 12:39 ` [PATCH v2 6/6] virtio-snd: period_bytes divides buffer_bytes Manos Pitsidianakis
5 siblings, 1 reply; 13+ messages in thread
From: Manos Pitsidianakis @ 2026-11-10 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Michael S. Tsirkin, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 5/6] virtio-snd: cap maximum buffer_bytes to half a GiB
2026-11-10 12:39 ` [PATCH v2 5/6] virtio-snd: cap maximum buffer_bytes to half a GiB Manos Pitsidianakis
@ 2026-11-11 10:48 ` Jim MacArthur
0 siblings, 0 replies; 13+ messages in thread
From: Jim MacArthur @ 2026-11-11 10:48 UTC (permalink / raw)
To: Manos Pitsidianakis; +Cc: qemu-devel, Gerd Hoffmann, Michael S. Tsirkin
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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 6/6] virtio-snd: period_bytes divides buffer_bytes
2026-11-10 12:39 [PATCH v2 0/6] Various virtio-snd fixes again Manos Pitsidianakis
` (4 preceding siblings ...)
2026-11-10 12:39 ` [PATCH v2 5/6] virtio-snd: cap maximum buffer_bytes to half a GiB Manos Pitsidianakis
@ 2026-11-10 12:39 ` Manos Pitsidianakis
2026-11-11 10:48 ` Jim MacArthur
5 siblings, 1 reply; 13+ messages in thread
From: Manos Pitsidianakis @ 2026-11-10 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Michael S. Tsirkin, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 6/6] virtio-snd: period_bytes divides buffer_bytes
2026-11-10 12:39 ` [PATCH v2 6/6] virtio-snd: period_bytes divides buffer_bytes Manos Pitsidianakis
@ 2026-11-11 10:48 ` Jim MacArthur
0 siblings, 0 replies; 13+ messages in thread
From: Jim MacArthur @ 2026-11-11 10:48 UTC (permalink / raw)
To: Manos Pitsidianakis; +Cc: qemu-devel, Gerd Hoffmann, Michael S. Tsirkin
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>
^ permalink raw reply [flat|nested] 13+ messages in thread