hw/audio/virtio-snd.c | 82 +++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 42 deletions(-)
The control, invalid PCM, TX and RX completion paths pass fixed response
sizes to virtqueue_push() after copying into the guest's writable iovecs.
iov_from_buf() truncates the copy when those iovecs are undersized, so
the used length can exceed the writable descriptor capacity.
Use the byte count returned by iov_from_buf() for every completion. For
PCM_INFO, retain the actual payload byte count so the control completion
also remains bounded when the requested element size is smaller than the
native structure.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
---
Tested with the following virtio-villain reproducers:
SND0005:
https://github.com/weltling/virtio-villain/blob/e0c649562cf2e836a0a305e19fb72323e6543d10/tests/sound/snd0005_info_size_short.c
SND0012:
https://github.com/weltling/virtio-villain/blob/e0c649562cf2e836a0a305e19fb72323e6543d10/tests/sound/snd0012_pcm_info_size_zero.c
SND0015:
https://github.com/weltling/virtio-villain/blob/e0c649562cf2e836a0a305e19fb72323e6543d10/tests/sound/snd0015_pcm_info_status_short.c
SND0065:
https://github.com/weltling/virtio-villain/blob/e0c649562cf2e836a0a305e19fb72323e6543d10/tests/sound/snd0065_tx_missing_status.c
SND0066:
https://github.com/weltling/virtio-villain/blob/e0c649562cf2e836a0a305e19fb72323e6543d10/tests/sound/snd0066_tx_status_short.c
Before this change all five tests fail because the used length exceeds
the writable descriptor capacity. With this change all five pass.
hw/audio/virtio-snd.c | 82 +++++++++++++++++++++----------------------
1 file changed, 40 insertions(+), 42 deletions(-)
diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 6df7ea0eb0..52e12603dd 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -271,13 +271,12 @@ static void virtio_snd_handle_pcm_info(VirtIOSound *s,
memset(&pcm_info[i].padding, 0, 5);
}
- cmd->payload_size = sizeof(virtio_snd_pcm_info) * count;
cmd->resp.code = cpu_to_le32(VIRTIO_SND_S_OK);
- iov_from_buf(cmd->elem->in_sg,
- cmd->elem->in_num,
- sizeof(virtio_snd_hdr),
- pcm_info,
- cmd->payload_size);
+ cmd->payload_size = iov_from_buf(cmd->elem->in_sg,
+ cmd->elem->in_num,
+ sizeof(virtio_snd_hdr),
+ pcm_info,
+ sizeof(virtio_snd_pcm_info) * count);
}
/*
@@ -774,6 +773,7 @@ static inline void
process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
{
uint32_t code;
+ size_t written;
size_t msg_sz = iov_to_buf(cmd->elem->out_sg,
cmd->elem->out_num,
0,
@@ -828,13 +828,12 @@ process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
cmd->resp.code = cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
}
- iov_from_buf(cmd->elem->in_sg,
- cmd->elem->in_num,
- 0,
- &cmd->resp,
- sizeof(virtio_snd_hdr));
- virtqueue_push(cmd->vq, cmd->elem,
- sizeof(virtio_snd_hdr) + cmd->payload_size);
+ written = iov_from_buf(cmd->elem->in_sg,
+ cmd->elem->in_num,
+ 0,
+ &cmd->resp,
+ sizeof(virtio_snd_hdr));
+ virtqueue_push(cmd->vq, cmd->elem, written + cmd->payload_size);
g_assert(s->queue_inuse[VIRTIO_SND_VQ_CONTROL] > 0);
s->queue_inuse[VIRTIO_SND_VQ_CONTROL] -= 1;
virtio_notify(VIRTIO_DEVICE(s), cmd->vq);
@@ -917,6 +916,7 @@ static inline void empty_invalid_queue(VirtIODevice *vdev, VirtQueue *vq)
VirtIOSoundPCMBuffer *buffer = NULL;
virtio_snd_pcm_status resp = { 0 };
VirtIOSound *vsnd = VIRTIO_SND(vdev);
+ size_t written;
g_assert(!QSIMPLEQ_EMPTY(&vsnd->invalid));
@@ -926,14 +926,12 @@ static inline void empty_invalid_queue(VirtIODevice *vdev, VirtQueue *vq)
g_assert(buffer->vq == vq);
resp.status = cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
- iov_from_buf(buffer->elem->in_sg,
- buffer->elem->in_num,
- 0,
- &resp,
- sizeof(virtio_snd_pcm_status));
- virtqueue_push(vq,
- buffer->elem,
- sizeof(virtio_snd_pcm_status));
+ written = iov_from_buf(buffer->elem->in_sg,
+ buffer->elem->in_num,
+ 0,
+ &resp,
+ sizeof(virtio_snd_pcm_status));
+ virtqueue_push(vq, buffer->elem, written);
QSIMPLEQ_REMOVE_HEAD(&vsnd->invalid, entry);
virtio_snd_pcm_buffer_free(buffer);
}
@@ -1216,17 +1214,17 @@ static inline void return_tx_buffer(VirtIOSoundPCMStream *stream,
VirtIOSoundPCMBuffer *buffer)
{
virtio_snd_pcm_status resp = { 0 };
+ size_t written;
+
resp.status = cpu_to_le32(VIRTIO_SND_S_OK);
update_latency(stream, buffer->size);
resp.latency_bytes = cpu_to_le32(stream->latency_bytes);
- iov_from_buf(buffer->elem->in_sg,
- buffer->elem->in_num,
- 0,
- &resp,
- sizeof(virtio_snd_pcm_status));
- virtqueue_push(buffer->vq,
- buffer->elem,
- sizeof(virtio_snd_pcm_status));
+ written = iov_from_buf(buffer->elem->in_sg,
+ buffer->elem->in_num,
+ 0,
+ &resp,
+ sizeof(virtio_snd_pcm_status));
+ virtqueue_push(buffer->vq, buffer->elem, written);
g_assert(stream->s->queue_inuse[VIRTIO_SND_VQ_TX] > 0);
stream->s->queue_inuse[VIRTIO_SND_VQ_TX] -= 1;
virtio_notify(VIRTIO_DEVICE(stream->s), buffer->vq);
@@ -1306,22 +1304,22 @@ static inline void return_rx_buffer(VirtIOSoundPCMStream *stream,
VirtIOSoundPCMBuffer *buffer)
{
virtio_snd_pcm_status resp = { 0 };
+ size_t written;
+
resp.status = cpu_to_le32(VIRTIO_SND_S_OK);
resp.latency_bytes = 0;
/* Copy data -if any- to guest */
- iov_from_buf(buffer->elem->in_sg,
- buffer->elem->in_num,
- 0,
- buffer->data,
- buffer->size);
- iov_from_buf(buffer->elem->in_sg,
- buffer->elem->in_num,
- buffer->size,
- &resp,
- sizeof(virtio_snd_pcm_status));
- virtqueue_push(buffer->vq,
- buffer->elem,
- sizeof(virtio_snd_pcm_status) + buffer->size);
+ written = iov_from_buf(buffer->elem->in_sg,
+ buffer->elem->in_num,
+ 0,
+ buffer->data,
+ buffer->size);
+ written += iov_from_buf(buffer->elem->in_sg,
+ buffer->elem->in_num,
+ buffer->size,
+ &resp,
+ sizeof(virtio_snd_pcm_status));
+ virtqueue_push(buffer->vq, buffer->elem, written);
g_assert(stream->s->queue_inuse[VIRTIO_SND_VQ_RX] > 0);
stream->s->queue_inuse[VIRTIO_SND_VQ_RX] -= 1;
virtio_notify(VIRTIO_DEVICE(stream->s), buffer->vq);
--
2.55.0Cc'ing maintainers otherwise your patch won't be reviewed:
$ ./scripts/get_maintainer.pl -f hw/audio/virtio-snd.c
"Michael S. Tsirkin" <mst@redhat.com> (supporter:virtio, gitlab:@mstredhat)
Gerd Hoffmann <kraxel@redhat.com> (supporter:virtio-snd, gitlab:@kraxel)
Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
(supporter:virtio-snd, gitlab:@epilys)
qemu-devel@nongnu.org (open list:All patches CC here)
On 2/10/26 00:30, Anatol Belski wrote:
> The control, invalid PCM, TX and RX completion paths pass fixed response
> sizes to virtqueue_push() after copying into the guest's writable iovecs.
> iov_from_buf() truncates the copy when those iovecs are undersized, so
> the used length can exceed the writable descriptor capacity.
>
> Use the byte count returned by iov_from_buf() for every completion. For
> PCM_INFO, retain the actual payload byte count so the control completion
> also remains bounded when the requested element size is smaller than the
> native structure.
>
> Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
> ---
> Tested with the following virtio-villain reproducers:
>
> SND0005:
> https://github.com/weltling/virtio-villain/blob/e0c649562cf2e836a0a305e19fb72323e6543d10/tests/sound/snd0005_info_size_short.c
>
> SND0012:
> https://github.com/weltling/virtio-villain/blob/e0c649562cf2e836a0a305e19fb72323e6543d10/tests/sound/snd0012_pcm_info_size_zero.c
>
> SND0015:
> https://github.com/weltling/virtio-villain/blob/e0c649562cf2e836a0a305e19fb72323e6543d10/tests/sound/snd0015_pcm_info_status_short.c
>
> SND0065:
> https://github.com/weltling/virtio-villain/blob/e0c649562cf2e836a0a305e19fb72323e6543d10/tests/sound/snd0065_tx_missing_status.c
>
> SND0066:
> https://github.com/weltling/virtio-villain/blob/e0c649562cf2e836a0a305e19fb72323e6543d10/tests/sound/snd0066_tx_status_short.c
>
> Before this change all five tests fail because the used length exceeds
> the writable descriptor capacity. With this change all five pass.
>
> hw/audio/virtio-snd.c | 82 +++++++++++++++++++++----------------------
> 1 file changed, 40 insertions(+), 42 deletions(-)
>
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index 6df7ea0eb0..52e12603dd 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -271,13 +271,12 @@ static void virtio_snd_handle_pcm_info(VirtIOSound *s,
> memset(&pcm_info[i].padding, 0, 5);
> }
>
> - cmd->payload_size = sizeof(virtio_snd_pcm_info) * count;
> cmd->resp.code = cpu_to_le32(VIRTIO_SND_S_OK);
> - iov_from_buf(cmd->elem->in_sg,
> - cmd->elem->in_num,
> - sizeof(virtio_snd_hdr),
> - pcm_info,
> - cmd->payload_size);
> + cmd->payload_size = iov_from_buf(cmd->elem->in_sg,
> + cmd->elem->in_num,
> + sizeof(virtio_snd_hdr),
> + pcm_info,
> + sizeof(virtio_snd_pcm_info) * count);
> }
>
> /*
> @@ -774,6 +773,7 @@ static inline void
> process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
> {
> uint32_t code;
> + size_t written;
> size_t msg_sz = iov_to_buf(cmd->elem->out_sg,
> cmd->elem->out_num,
> 0,
> @@ -828,13 +828,12 @@ process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
> cmd->resp.code = cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
> }
>
> - iov_from_buf(cmd->elem->in_sg,
> - cmd->elem->in_num,
> - 0,
> - &cmd->resp,
> - sizeof(virtio_snd_hdr));
> - virtqueue_push(cmd->vq, cmd->elem,
> - sizeof(virtio_snd_hdr) + cmd->payload_size);
> + written = iov_from_buf(cmd->elem->in_sg,
> + cmd->elem->in_num,
> + 0,
> + &cmd->resp,
> + sizeof(virtio_snd_hdr));
> + virtqueue_push(cmd->vq, cmd->elem, written + cmd->payload_size);
> g_assert(s->queue_inuse[VIRTIO_SND_VQ_CONTROL] > 0);
> s->queue_inuse[VIRTIO_SND_VQ_CONTROL] -= 1;
> virtio_notify(VIRTIO_DEVICE(s), cmd->vq);
> @@ -917,6 +916,7 @@ static inline void empty_invalid_queue(VirtIODevice *vdev, VirtQueue *vq)
> VirtIOSoundPCMBuffer *buffer = NULL;
> virtio_snd_pcm_status resp = { 0 };
> VirtIOSound *vsnd = VIRTIO_SND(vdev);
> + size_t written;
>
> g_assert(!QSIMPLEQ_EMPTY(&vsnd->invalid));
>
> @@ -926,14 +926,12 @@ static inline void empty_invalid_queue(VirtIODevice *vdev, VirtQueue *vq)
> g_assert(buffer->vq == vq);
>
> resp.status = cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
> - iov_from_buf(buffer->elem->in_sg,
> - buffer->elem->in_num,
> - 0,
> - &resp,
> - sizeof(virtio_snd_pcm_status));
> - virtqueue_push(vq,
> - buffer->elem,
> - sizeof(virtio_snd_pcm_status));
> + written = iov_from_buf(buffer->elem->in_sg,
> + buffer->elem->in_num,
> + 0,
> + &resp,
> + sizeof(virtio_snd_pcm_status));
> + virtqueue_push(vq, buffer->elem, written);
> QSIMPLEQ_REMOVE_HEAD(&vsnd->invalid, entry);
> virtio_snd_pcm_buffer_free(buffer);
> }
> @@ -1216,17 +1214,17 @@ static inline void return_tx_buffer(VirtIOSoundPCMStream *stream,
> VirtIOSoundPCMBuffer *buffer)
> {
> virtio_snd_pcm_status resp = { 0 };
> + size_t written;
> +
> resp.status = cpu_to_le32(VIRTIO_SND_S_OK);
> update_latency(stream, buffer->size);
> resp.latency_bytes = cpu_to_le32(stream->latency_bytes);
> - iov_from_buf(buffer->elem->in_sg,
> - buffer->elem->in_num,
> - 0,
> - &resp,
> - sizeof(virtio_snd_pcm_status));
> - virtqueue_push(buffer->vq,
> - buffer->elem,
> - sizeof(virtio_snd_pcm_status));
> + written = iov_from_buf(buffer->elem->in_sg,
> + buffer->elem->in_num,
> + 0,
> + &resp,
> + sizeof(virtio_snd_pcm_status));
> + virtqueue_push(buffer->vq, buffer->elem, written);
> g_assert(stream->s->queue_inuse[VIRTIO_SND_VQ_TX] > 0);
> stream->s->queue_inuse[VIRTIO_SND_VQ_TX] -= 1;
> virtio_notify(VIRTIO_DEVICE(stream->s), buffer->vq);
> @@ -1306,22 +1304,22 @@ static inline void return_rx_buffer(VirtIOSoundPCMStream *stream,
> VirtIOSoundPCMBuffer *buffer)
> {
> virtio_snd_pcm_status resp = { 0 };
> + size_t written;
> +
> resp.status = cpu_to_le32(VIRTIO_SND_S_OK);
> resp.latency_bytes = 0;
> /* Copy data -if any- to guest */
> - iov_from_buf(buffer->elem->in_sg,
> - buffer->elem->in_num,
> - 0,
> - buffer->data,
> - buffer->size);
> - iov_from_buf(buffer->elem->in_sg,
> - buffer->elem->in_num,
> - buffer->size,
> - &resp,
> - sizeof(virtio_snd_pcm_status));
> - virtqueue_push(buffer->vq,
> - buffer->elem,
> - sizeof(virtio_snd_pcm_status) + buffer->size);
> + written = iov_from_buf(buffer->elem->in_sg,
> + buffer->elem->in_num,
> + 0,
> + buffer->data,
> + buffer->size);
> + written += iov_from_buf(buffer->elem->in_sg,
> + buffer->elem->in_num,
> + buffer->size,
> + &resp,
> + sizeof(virtio_snd_pcm_status));
> + virtqueue_push(buffer->vq, buffer->elem, written);
> g_assert(stream->s->queue_inuse[VIRTIO_SND_VQ_RX] > 0);
> stream->s->queue_inuse[VIRTIO_SND_VQ_RX] -= 1;
> virtio_notify(VIRTIO_DEVICE(stream->s), buffer->vq);
© 2016 - 2026 Red Hat, Inc.