* [PATCH v3 1/5] ALSA: virtio: Propagate PCM send failures through ACK
2026-10-13 8:57 [PATCH v3 0/5] ALSA: virtio: Fix request lifetime and error recovery Joonho Lee
@ 2026-10-13 8:57 ` Joonho Lee
2026-10-13 8:57 ` [PATCH v3 2/5] ALSA: virtio: Recover PCM state after a failed start Joonho Lee
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Joonho Lee @ 2026-10-13 8:57 UTC (permalink / raw)
To: Takashi Iwai
Cc: Michael S . Tsirkin, Jaroslav Kysela, virtualization, linux-sound,
linux-kernel, Matias Ezequiel Vara Larsen, Eugenio Perez
virtsnd_pcm_msg_send() can fail while queueing a transfer. Restore the
failed message's length and notify the device about earlier messages
queued by the same call, so they do not remain stranded.
Propagate that failure through the indirect PCM transfer callback instead
of discarding it. Commit the updated indirect state only after successful
submission and stop further transfers when a send fails. Return an XRUN
error to ALSA so the stream can recover.
Keep the polling feature path unchanged.
Fixes: fe981e67568c ("ALSA: virtio: use ack callback")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
Changes in v3:
- No code or commit-message changes from v2.
sound/virtio/virtio_pcm_msg.c | 12 +++++
sound/virtio/virtio_pcm_ops.c | 82 +++++++++++++++++++++++++++++++----
2 files changed, 85 insertions(+), 9 deletions(-)
diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
index 72103f606716..5371f4985fd0 100644
--- a/sound/virtio/virtio_pcm_msg.c
+++ b/sound/virtio/virtio_pcm_msg.c
@@ -260,6 +260,18 @@ int virtsnd_pcm_msg_send(struct virtio_pcm_substream *vss, unsigned long offset,
dev_err(&vdev->dev,
"SID %u: failed to send I/O message\n",
vss->sid);
+ msg->length -= n;
+
+ /*
+ * Do not leave earlier messages queued without notifying
+ * the device when a later message could not be added.
+ */
+ if (msg_count != vss->msg_count &&
+ !(vss->features &
+ (1U << VIRTIO_SND_PCM_F_MSG_POLLING)) &&
+ virtqueue_kick_prepare(vqueue))
+ virtqueue_notify(vqueue);
+
return rc;
}
diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
index 1091100a4c98..a40ea394bf17 100644
--- a/sound/virtio/virtio_pcm_ops.c
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -466,36 +466,100 @@ virtsnd_pcm_cp_pointer(struct snd_pcm_substream *substream)
vss->hw_ptr);
}
+struct virtsnd_pcm_transfer {
+ struct snd_pcm_indirect indirect;
+ int error;
+};
+
static void virtsnd_pcm_trans_copy(struct snd_pcm_substream *substream,
struct snd_pcm_indirect *rec, size_t bytes)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+ struct virtsnd_pcm_transfer *transfer =
+ container_of(rec, struct virtsnd_pcm_transfer, indirect);
+
+ if (transfer->error)
+ return;
- virtsnd_pcm_msg_send(vss, rec->sw_data, bytes);
+ transfer->error = virtsnd_pcm_msg_send(vss, rec->sw_data, bytes);
+}
+
+/* Caller holds vss->lock. */
+static void virtsnd_pcm_transfer_failed(struct virtio_pcm_substream *vss)
+{
+ vss->xfer_enabled = false;
+ vss->stopped = true;
+ vss->substream->runtime->stop_operating = true;
}
static int virtsnd_pcm_pb_ack(struct snd_pcm_substream *substream)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
+ struct virtsnd_pcm_transfer transfer = {};
+ int rc;
- guard(spinlock_irqsave)(&queue->lock);
- guard(spinlock)(&vss->lock);
+ scoped_guard(spinlock_irqsave, &queue->lock) {
+ guard(spinlock)(&vss->lock);
+ transfer.indirect = vss->pcm_indirect;
+ if (vss->stopped) {
+ rc = -EPIPE;
+ } else {
+ rc = snd_pcm_indirect_playback_transfer(substream,
+ &transfer.indirect,
+ virtsnd_pcm_trans_copy);
+ }
+ if (!rc)
+ rc = transfer.error;
+ if (rc)
+ virtsnd_pcm_transfer_failed(vss);
+ else
+ vss->pcm_indirect = transfer.indirect;
+ }
+ if (rc) {
+ dev_err(&vss->snd->vdev->dev,
+ "SID %u: playback ACK transfer failed: %d\n",
+ vss->sid, rc);
+ /* ALSA treats -EPIPE as an XRUN in ACK and pointer paths. */
+ return -EPIPE;
+ }
- return snd_pcm_indirect_playback_transfer(substream, &vss->pcm_indirect,
- virtsnd_pcm_trans_copy);
+ return 0;
}
static int virtsnd_pcm_cp_ack(struct snd_pcm_substream *substream)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
+ struct virtsnd_pcm_transfer transfer = {};
+ int rc;
- guard(spinlock_irqsave)(&queue->lock);
- guard(spinlock)(&vss->lock);
+ scoped_guard(spinlock_irqsave, &queue->lock) {
+ guard(spinlock)(&vss->lock);
+ transfer.indirect = vss->pcm_indirect;
+ if (vss->stopped) {
+ rc = -EPIPE;
+ } else {
+ rc = snd_pcm_indirect_capture_transfer(substream,
+ &transfer.indirect,
+ virtsnd_pcm_trans_copy);
+ }
+ if (!rc)
+ rc = transfer.error;
+ if (rc)
+ virtsnd_pcm_transfer_failed(vss);
+ else
+ vss->pcm_indirect = transfer.indirect;
+ }
+ if (rc) {
+ dev_err(&vss->snd->vdev->dev,
+ "SID %u: capture ACK transfer failed: %d\n",
+ vss->sid, rc);
+ /* ALSA treats -EPIPE as an XRUN in ACK and pointer paths. */
+ return -EPIPE;
+ }
- return snd_pcm_indirect_capture_transfer(substream, &vss->pcm_indirect,
- virtsnd_pcm_trans_copy);
+ return 0;
}
/* PCM substream operators map. */
--
2.54.0 (Apple Git-157)
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 2/5] ALSA: virtio: Recover PCM state after a failed start
2026-10-13 8:57 [PATCH v3 0/5] ALSA: virtio: Fix request lifetime and error recovery Joonho Lee
2026-10-13 8:57 ` [PATCH v3 1/5] ALSA: virtio: Propagate PCM send failures through ACK Joonho Lee
@ 2026-10-13 8:57 ` Joonho Lee
2026-10-13 8:57 ` [PATCH v3 3/5] ALSA: virtio: Retain queued messages through device reset Joonho Lee
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Joonho Lee @ 2026-10-13 8:57 UTC (permalink / raw)
To: Takashi Iwai
Cc: Michael S . Tsirkin, Jaroslav Kysela, virtualization, linux-sound,
linux-kernel, Matias Ezequiel Vara Larsen, Eugenio Perez
Capture trigger queues I/O messages before PCM_START. If the START
message cannot be allocated or sent, transfers may remain queued, and
ALSA does not invoke the action undo callback for the failed trigger.
Mark the stream stopped and disable transfers for every failed START
path. Arm runtime->stop_operating so sync_stop releases pending I/O and
prevents prepare, hw_params, or START retries until release succeeds.
The stopped flag is shared with ACK callbacks that can run without the
ALSA stream lock. Protect every stopped access with vss->lock. Read the
pending message count under the same lock in open and hw_params. In
sync_stop, decide whether RELEASE is needed and mark the stream stopped
in one locked transition; leave it stopped when release or draining fails.
Always cancel elapsed-period work in sync_stop, even when no RELEASE
is needed.
Fixes: da76e9f3e43a ("ALSA: virtio: PCM substream operators")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
Changes in v3:
- Protect all stopped accesses with vss->lock. Keep the RELEASE
decision and stopped transition in the same critical section.
sound/virtio/virtio_pcm.h | 4 +-
sound/virtio/virtio_pcm_ops.c | 81 +++++++++++++++++++++++++++++------
2 files changed, 69 insertions(+), 16 deletions(-)
diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h
index 8837dfc12828..8048b14e6c80 100644
--- a/sound/virtio/virtio_pcm.h
+++ b/sound/virtio/virtio_pcm.h
@@ -26,13 +26,13 @@ struct virtio_pcm_msg;
* @hw: Kernel ALSA substream hardware descriptor.
* @elapsed_period: Kernel work to handle the elapsed period state.
* @lock: Spinlock that protects fields shared by interrupt handlers and
- * substream operators.
+ * substream operators, including @stopped.
* @buffer_bytes: Current buffer size in bytes.
* @hw_ptr: Substream hardware pointer value in bytes [0 ... buffer_bytes).
* @xfer_enabled: Data transfer state (0 - off, 1 - on).
* @xfer_xrun: Data underflow/overflow state (0 - no xrun, 1 - xrun).
* @stopped: True if the substream is stopped and must be released on the device
- * side.
+ * side; protected by @lock.
* @suspended: True if the substream is suspended and must be reconfigured on
* the device side at resume.
* @msgs: Allocated I/O messages.
diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
index a40ea394bf17..970499fe419b 100644
--- a/sound/virtio/virtio_pcm_ops.c
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -96,6 +96,15 @@ static const struct virtsnd_a2v_rate g_a2v_rate_map[] = {
static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream);
+static void virtsnd_pcm_start_failed(struct virtio_pcm_substream *vss)
+{
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->xfer_enabled = false;
+ vss->stopped = true;
+ }
+ vss->substream->runtime->stop_operating = true;
+}
+
/**
* virtsnd_pcm_open() - Open the PCM substream.
* @substream: Kernel ALSA substream.
@@ -115,7 +124,8 @@ static int virtsnd_pcm_open(struct snd_pcm_substream *substream)
snd_pcm_hw_constraint_integer(substream->runtime,
SNDRV_PCM_HW_PARAM_PERIODS);
- vss->stopped = !!virtsnd_pcm_msg_pending_num(vss);
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ vss->stopped |= !!vss->msg_count;
vss->suspended = false;
/*
@@ -217,13 +227,25 @@ static int virtsnd_pcm_hw_params(struct snd_pcm_substream *substream,
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_device *vdev = vss->snd->vdev;
+ unsigned int msg_count;
+ bool stopped;
int rc;
- if (virtsnd_pcm_msg_pending_num(vss)) {
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ msg_count = vss->msg_count;
+ stopped = vss->stopped;
+ }
+ if (msg_count) {
dev_err(&vdev->dev, "SID %u: invalid I/O queue state\n",
vss->sid);
return -EBADFD;
}
+ if (stopped) {
+ dev_err(&vdev->dev, "SID %u: stream release failed\n",
+ vss->sid);
+ substream->runtime->stop_operating = true;
+ return -EBADFD;
+ }
rc = virtsnd_pcm_dev_set_params(vss, params_buffer_bytes(hw_params),
params_period_bytes(hw_params),
@@ -273,6 +295,16 @@ static int virtsnd_pcm_prepare(struct snd_pcm_substream *substream)
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_device *vdev = vss->snd->vdev;
struct virtio_snd_msg *msg;
+ bool stopped;
+
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ stopped = vss->stopped;
+ if (stopped) {
+ dev_err(&vdev->dev, "SID %u: stream release failed\n",
+ vss->sid);
+ substream->runtime->stop_operating = true;
+ return -EBADFD;
+ }
if (!vss->suspended) {
if (virtsnd_pcm_msg_pending_num(vss)) {
@@ -338,31 +370,40 @@ static int virtsnd_pcm_trigger(struct snd_pcm_substream *substream, int command)
scoped_guard(spinlock_irqsave, &queue->lock) {
guard(spinlock)(&vss->lock);
+ if (vss->stopped) {
+ substream->runtime->stop_operating = true;
+ return -EBADFD;
+ }
if (vss->direction == SNDRV_PCM_STREAM_CAPTURE)
rc = virtsnd_pcm_msg_send(vss, 0, vss->buffer_bytes);
- if (rc)
+ if (rc) {
+ vss->stopped = true;
+ substream->runtime->stop_operating = true;
return rc;
+ }
vss->xfer_enabled = true;
}
msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_START,
GFP_KERNEL);
if (!msg) {
- guard(spinlock_irqsave)(&vss->lock);
- vss->xfer_enabled = false;
-
+ virtsnd_pcm_start_failed(vss);
return -ENOMEM;
}
- return virtsnd_ctl_msg_send_sync(snd, msg);
+ rc = virtsnd_ctl_msg_send_sync(snd, msg);
+ if (rc)
+ virtsnd_pcm_start_failed(vss);
+
+ return rc;
case SNDRV_PCM_TRIGGER_SUSPEND:
vss->suspended = true;
fallthrough;
case SNDRV_PCM_TRIGGER_STOP:
- vss->stopped = true;
- fallthrough;
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (command != SNDRV_PCM_TRIGGER_PAUSE_PUSH)
+ vss->stopped = true;
vss->xfer_enabled = false;
}
@@ -393,21 +434,31 @@ static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream)
struct virtio_snd *snd = vss->snd;
struct virtio_snd_msg *msg;
unsigned int js = msecs_to_jiffies(virtsnd_msg_timeout_ms);
+ bool need_release;
int rc;
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->xfer_enabled = false;
+ need_release = vss->stopped || vss->msg_count;
+ if (need_release)
+ vss->stopped = true;
+ }
cancel_work_sync(&vss->elapsed_period);
-
- if (!vss->stopped)
+ if (!need_release)
return 0;
msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_RELEASE,
GFP_KERNEL);
- if (!msg)
+ if (!msg) {
+ substream->runtime->stop_operating = true;
return -ENOMEM;
+ }
rc = virtsnd_ctl_msg_send_sync(snd, msg);
- if (rc)
+ if (rc) {
+ substream->runtime->stop_operating = true;
return rc;
+ }
/*
* The spec states that upon receipt of the RELEASE command "the device
@@ -421,11 +472,13 @@ static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream)
if (rc <= 0) {
dev_warn(&snd->vdev->dev, "SID %u: failed to flush I/O queue\n",
vss->sid);
+ substream->runtime->stop_operating = true;
return !rc ? -ETIMEDOUT : rc;
}
- vss->stopped = false;
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ vss->stopped = false;
return 0;
}
--
2.54.0 (Apple Git-157)
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 3/5] ALSA: virtio: Retain queued messages through device reset
2026-10-13 8:57 [PATCH v3 0/5] ALSA: virtio: Fix request lifetime and error recovery Joonho Lee
2026-10-13 8:57 ` [PATCH v3 1/5] ALSA: virtio: Propagate PCM send failures through ACK Joonho Lee
2026-10-13 8:57 ` [PATCH v3 2/5] ALSA: virtio: Recover PCM state after a failed start Joonho Lee
@ 2026-10-13 8:57 ` Joonho Lee
2026-10-13 8:57 ` [PATCH v3 4/5] ALSA: virtio: Keep PCM DMA buffers alive through device ownership Joonho Lee
2026-10-13 8:57 ` [PATCH v3 5/5] ALSA: virtio: Keep control payloads alive through DMA completion Joonho Lee
4 siblings, 0 replies; 6+ messages in thread
From: Joonho Lee @ 2026-10-13 8:57 UTC (permalink / raw)
To: Takashi Iwai
Cc: Michael S . Tsirkin, Jaroslav Kysela, virtualization, linux-sound,
linux-kernel, Matias Ezequiel Vara Larsen, Eugenio Perez
Keep the control queue available through snd_card_free() so close-time
STOP and RELEASE commands can still be submitted. Block new control
requests and mark pending messages canceled before reset, then release
their queue references only after reset and queue deletion.
For PCM queues, detach each remaining virtqueue buffer after the device
has stopped DMA and retire its message without reporting a normal
completion. Detach all queues before deleting them so outstanding DMA
mappings are unmapped and no token remains for a late device completion.
Fixes: 9d45e514da88 ("ALSA: virtio: handling control messages")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
Changes in v3:
- No code or commit-message changes from v2.
sound/virtio/virtio_card.c | 44 ++++++++++++++++++++++---
sound/virtio/virtio_card.h | 2 ++
sound/virtio/virtio_ctl_msg.c | 62 +++++++++++++++++++++++++++++++----
sound/virtio/virtio_ctl_msg.h | 4 +++
sound/virtio/virtio_pcm.h | 2 ++
sound/virtio/virtio_pcm_msg.c | 29 ++++++++++++++++
6 files changed, 131 insertions(+), 12 deletions(-)
diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
index 6f35276416fe..df7091b74a52 100644
--- a/sound/virtio/virtio_card.c
+++ b/sound/virtio/virtio_card.c
@@ -337,6 +337,38 @@ static int virtsnd_probe(struct virtio_device *vdev)
return rc;
}
+/**
+ * virtsnd_reset_vqs() - Stop device access and unmap outstanding buffers.
+ * @snd: VirtIO sound device.
+ *
+ * Context: Any context that permits to sleep.
+ */
+static void virtsnd_reset_vqs(struct virtio_snd *snd)
+{
+ struct virtio_device *vdev = snd->vdev;
+ unsigned int i;
+
+ virtio_reset_device(vdev);
+
+ /* Deleting a virtqueue does not unmap its outstanding descriptors. */
+ for (i = 0; i < VIRTIO_SND_VQ_MAX; ++i) {
+ struct virtio_snd_queue *queue = &snd->queues[i];
+ unsigned long flags;
+ void *msg;
+
+ spin_lock_irqsave(&queue->lock, flags);
+ if (queue->vqueue)
+ while ((msg = virtqueue_detach_unused_buf(queue->vqueue)))
+ if (i == VIRTIO_SND_VQ_TX || i == VIRTIO_SND_VQ_RX)
+ virtsnd_pcm_msg_cancel(msg);
+ spin_unlock_irqrestore(&queue->lock, flags);
+ }
+
+ vdev->config->del_vqs(vdev);
+ for (i = 0; i < VIRTIO_SND_VQ_MAX; ++i)
+ snd->queues[i].vqueue = NULL;
+}
+
/**
* virtsnd_remove() - Remove VirtIO and ALSA devices.
* @vdev: VirtIO parent device.
@@ -349,13 +381,14 @@ static void virtsnd_remove(struct virtio_device *vdev)
unsigned int i;
virtsnd_disable_event_vq(snd);
- virtsnd_ctl_msg_cancel_all(snd);
if (snd->card)
snd_card_free(snd->card);
- virtio_reset_device(vdev);
- vdev->config->del_vqs(vdev);
+ virtsnd_ctl_msg_cancel_all(snd);
+
+ virtsnd_reset_vqs(snd);
+ virtsnd_ctl_msg_cleanup(snd);
for (i = 0; snd->substreams && i < snd->nsubstreams; ++i) {
struct virtio_pcm_substream *vss = &snd->substreams[i];
@@ -383,8 +416,8 @@ static int virtsnd_freeze(struct virtio_device *vdev)
virtsnd_disable_event_vq(snd);
virtsnd_ctl_msg_cancel_all(snd);
- virtio_reset_device(vdev);
- vdev->config->del_vqs(vdev);
+ virtsnd_reset_vqs(snd);
+ virtsnd_ctl_msg_cleanup(snd);
for (i = 0; i < snd->nsubstreams; ++i)
cancel_work_sync(&snd->substreams[i].elapsed_period);
@@ -412,6 +445,7 @@ static int virtsnd_restore(struct virtio_device *vdev)
return rc;
virtio_device_ready(vdev);
+ virtsnd_ctl_msg_enable(snd);
virtsnd_enable_event_vq(snd);
diff --git a/sound/virtio/virtio_card.h b/sound/virtio/virtio_card.h
index 3ceee4e416fc..bd065bd00d25 100644
--- a/sound/virtio/virtio_card.h
+++ b/sound/virtio/virtio_card.h
@@ -47,6 +47,7 @@ struct virtio_kctl {
* @queues: Virtqueue wrappers.
* @card: ALSA sound card.
* @ctl_msgs: Pending control request list.
+ * @ctl_msgs_blocked: Control requests are blocked during teardown or suspend.
* @event_msgs: Device events.
* @pcm_list: VirtIO PCM device list.
* @jacks: VirtIO jacks.
@@ -64,6 +65,7 @@ struct virtio_snd {
struct virtio_snd_queue queues[VIRTIO_SND_VQ_MAX];
struct snd_card *card;
struct list_head ctl_msgs;
+ bool ctl_msgs_blocked;
struct virtio_snd_event *event_msgs;
struct list_head pcm_list;
struct virtio_jack *jacks;
diff --git a/sound/virtio/virtio_ctl_msg.c b/sound/virtio/virtio_ctl_msg.c
index 6433c870f271..7114bea4033a 100644
--- a/sound/virtio/virtio_ctl_msg.c
+++ b/sound/virtio/virtio_ctl_msg.c
@@ -15,6 +15,7 @@
* @list: Pending message list entry.
* @notify: Request completed notification.
* @ref_count: Reference count used to manage a message lifetime.
+ * @cancelled: Request canceled during device teardown.
*/
struct virtio_snd_msg {
struct scatterlist sg_request;
@@ -22,6 +23,7 @@ struct virtio_snd_msg {
struct list_head list;
struct completion notify;
refcount_t ref_count;
+ bool cancelled;
};
/**
@@ -100,7 +102,7 @@ struct virtio_snd_msg *virtsnd_ctl_msg_alloc(size_t request_size,
INIT_LIST_HEAD(&msg->list);
init_completion(&msg->notify);
- /* This reference is dropped in virtsnd_ctl_msg_complete(). */
+ /* The caller owns this reference; send adds a queue reference. */
refcount_set(&msg->ref_count, 1);
return msg;
@@ -134,6 +136,7 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
int rc;
virtsnd_ctl_msg_ref(msg);
+ /* This reference stays with the queue until completion or teardown. */
/* Set the default status in case the message was canceled. */
response->code = cpu_to_le32(VIRTIO_SND_S_IO_ERR);
@@ -147,12 +150,17 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
psgs[nouts + nins++] = in_sgs;
scoped_guard(spinlock_irqsave, &queue->lock) {
- rc = virtqueue_add_sgs(queue->vqueue, psgs, nouts, nins, msg,
- GFP_ATOMIC);
+ if (snd->ctl_msgs_blocked)
+ rc = -ENODEV;
+ else
+ rc = virtqueue_add_sgs(queue->vqueue, psgs, nouts, nins,
+ msg, GFP_ATOMIC);
if (!rc) {
notify = virtqueue_kick_prepare(queue->vqueue);
list_add_tail(&msg->list, &snd->ctl_msgs);
+ if (notify)
+ virtqueue_notify(queue->vqueue);
}
}
@@ -169,9 +177,6 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
goto on_exit;
}
- if (notify)
- virtqueue_notify(queue->vqueue);
-
if (nowait)
goto on_exit;
@@ -187,6 +192,11 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
goto on_exit;
}
+ if (READ_ONCE(msg->cancelled)) {
+ rc = -ENODEV;
+ goto on_exit;
+ }
+
switch (le32_to_cpu(response->code)) {
case VIRTIO_SND_S_OK:
rc = 0;
@@ -227,9 +237,33 @@ void virtsnd_ctl_msg_complete(struct virtio_snd_msg *msg)
* virtsnd_ctl_msg_cancel_all() - Cancel all pending control messages.
* @snd: VirtIO sound device.
*
- * Context: Any context.
+ * Context: Any context. Blocks new senders and marks pending requests canceled.
+ * Existing senders are woken when their request completes or after
+ * the device has been reset.
*/
void virtsnd_ctl_msg_cancel_all(struct virtio_snd *snd)
+{
+ struct virtio_snd_queue *queue = virtsnd_control_queue(snd);
+ struct virtio_snd_msg *msg;
+
+ guard(spinlock_irqsave)(&queue->lock);
+ snd->ctl_msgs_blocked = true;
+ list_for_each_entry(msg, &snd->ctl_msgs, list) {
+ WRITE_ONCE(msg->cancelled, true);
+ }
+}
+
+/**
+ * virtsnd_ctl_msg_cleanup() - Release messages left after queue teardown.
+ * @snd: VirtIO sound device.
+ *
+ * This must be called after device reset, buffer detachment, and del_vqs(),
+ * when no queue callback or DMA access can still reach a message. The queue
+ * object and its lock must remain alive until this function returns.
+ *
+ * Context: Any context.
+ */
+void virtsnd_ctl_msg_cleanup(struct virtio_snd *snd)
{
struct virtio_snd_queue *queue = virtsnd_control_queue(snd);
@@ -243,6 +277,20 @@ void virtsnd_ctl_msg_cancel_all(struct virtio_snd *snd)
}
}
+/**
+ * virtsnd_ctl_msg_enable() - Allow control requests after queue restoration.
+ * @snd: VirtIO sound device.
+ *
+ * Context: Any context.
+ */
+void virtsnd_ctl_msg_enable(struct virtio_snd *snd)
+{
+ struct virtio_snd_queue *queue = virtsnd_control_queue(snd);
+
+ guard(spinlock_irqsave)(&queue->lock);
+ snd->ctl_msgs_blocked = false;
+}
+
/**
* virtsnd_ctl_query_info() - Query the item configuration from the device.
* @snd: VirtIO sound device.
diff --git a/sound/virtio/virtio_ctl_msg.h b/sound/virtio/virtio_ctl_msg.h
index 7f4db044f28e..d917435409d0 100644
--- a/sound/virtio/virtio_ctl_msg.h
+++ b/sound/virtio/virtio_ctl_msg.h
@@ -68,6 +68,10 @@ static inline int virtsnd_ctl_msg_send_async(struct virtio_snd *snd,
void virtsnd_ctl_msg_cancel_all(struct virtio_snd *snd);
+void virtsnd_ctl_msg_cleanup(struct virtio_snd *snd);
+
+void virtsnd_ctl_msg_enable(struct virtio_snd *snd);
+
void virtsnd_ctl_msg_complete(struct virtio_snd_msg *msg);
int virtsnd_ctl_query_info(struct virtio_snd *snd, int command, int start_id,
diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h
index 8048b14e6c80..e31f090a0124 100644
--- a/sound/virtio/virtio_pcm.h
+++ b/sound/virtio/virtio_pcm.h
@@ -120,6 +120,8 @@ void virtsnd_pcm_msg_free(struct virtio_pcm_substream *vss);
void virtsnd_pcm_msg_reset(struct virtio_pcm_substream *vss);
+void virtsnd_pcm_msg_cancel(struct virtio_pcm_msg *msg);
+
int virtsnd_pcm_msg_send(struct virtio_pcm_substream *vss, unsigned long offset,
unsigned long bytes);
diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
index 5371f4985fd0..762480363b86 100644
--- a/sound/virtio/virtio_pcm_msg.c
+++ b/sound/virtio/virtio_pcm_msg.c
@@ -307,6 +307,35 @@ unsigned int virtsnd_pcm_msg_pending_num(struct virtio_pcm_substream *vss)
return vss->msg_count;
}
+/**
+ * virtsnd_pcm_msg_cancel() - Cancel a message detached during device reset.
+ * @msg: Message returned by virtqueue_detach_unused_buf().
+ *
+ * Reset cancellation retires the queue token without treating it as a device
+ * completion. In particular, it does not advance the PCM hardware pointer or
+ * schedule period work.
+ *
+ * Context: Process context with the PCM queue lock held.
+ */
+void virtsnd_pcm_msg_cancel(struct virtio_pcm_msg *msg)
+{
+ struct virtio_pcm_substream *vss = msg->substream;
+ bool wake;
+
+ {
+ guard(spinlock)(&vss->lock);
+ msg->length = 0;
+ vss->xfer_enabled = false;
+ if (WARN_ON_ONCE(!vss->msg_count))
+ return;
+ vss->msg_count--;
+ wake = !vss->msg_count;
+ }
+
+ if (wake)
+ wake_up_all(&vss->msg_empty);
+}
+
/**
* virtsnd_pcm_msg_complete() - Complete an I/O message.
* @msg: I/O message.
--
2.54.0 (Apple Git-157)
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 4/5] ALSA: virtio: Keep PCM DMA buffers alive through device ownership
2026-10-13 8:57 [PATCH v3 0/5] ALSA: virtio: Fix request lifetime and error recovery Joonho Lee
` (2 preceding siblings ...)
2026-10-13 8:57 ` [PATCH v3 3/5] ALSA: virtio: Retain queued messages through device reset Joonho Lee
@ 2026-10-13 8:57 ` Joonho Lee
2026-10-13 8:57 ` [PATCH v3 5/5] ALSA: virtio: Keep control payloads alive through DMA completion Joonho Lee
4 siblings, 0 replies; 6+ messages in thread
From: Joonho Lee @ 2026-10-13 8:57 UTC (permalink / raw)
To: Takashi Iwai
Cc: Michael S . Tsirkin, Jaroslav Kysela, virtualization, linux-sound,
linux-kernel, Matias Ezequiel Vara Larsen, Eugenio Perez
Retain references to the pages backing ALSA's vmalloc buffer until its
queued I/O messages are freed. On NOMMU systems, use driver-owned memory
for DMA and copy playback data before submission and capture data after
completion, so ALSA buffer release cannot invalidate an active request.
Validate the PCM buffer and each period before building scatterlists.
Clamp device-reported capture lengths to the message capacity before
advancing the hardware pointer, preventing an oversized completion from
accounting bytes beyond the buffer actually submitted.
Fixes: f40a28679e0b ("ALSA: virtio: handling control and I/O messages for the PCM device")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
Changes in v3:
- No code or commit-message changes from v2.
sound/virtio/virtio_pcm.h | 11 ++
sound/virtio/virtio_pcm_msg.c | 252 +++++++++++++++++++++++++++++++---
sound/virtio/virtio_pcm_ops.c | 2 +
3 files changed, 243 insertions(+), 22 deletions(-)
diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h
index e31f090a0124..61faf7b13cbc 100644
--- a/sound/virtio/virtio_pcm.h
+++ b/sound/virtio/virtio_pcm.h
@@ -13,6 +13,7 @@
struct virtio_pcm;
struct virtio_pcm_msg;
+struct page;
/**
* struct virtio_pcm_substream - VirtIO PCM substream.
@@ -38,6 +39,10 @@ struct virtio_pcm_msg;
* @msgs: Allocated I/O messages.
* @nmsgs: Number of allocated I/O messages.
* @msg_count: Number of pending I/O messages in the virtqueue.
+ * @dma_pages: Pages backing DMA messages, retained until messages are freed.
+ * @ndma_pages: Number of retained pages.
+ * @owned_dma_area: Driver-owned PCM buffer backing asynchronous messages.
+ * @owned_dma_bytes: Size of @owned_dma_area.
* @msg_empty: Notify when msg_count is zero.
*/
struct virtio_pcm_substream {
@@ -60,6 +65,12 @@ struct virtio_pcm_substream {
struct virtio_pcm_msg **msgs;
unsigned int nmsgs;
unsigned int msg_count;
+ struct page **dma_pages;
+ unsigned int ndma_pages;
+#ifndef CONFIG_MMU
+ u8 *owned_dma_area;
+ size_t owned_dma_bytes;
+#endif
wait_queue_head_t msg_empty;
};
diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
index 762480363b86..b7f2334bb221 100644
--- a/sound/virtio/virtio_pcm_msg.c
+++ b/sound/virtio/virtio_pcm_msg.c
@@ -3,6 +3,9 @@
* virtio-snd: Virtio sound device
* Copyright (C) 2021 OpenSynergy GmbH
*/
+#include <linux/limits.h>
+#include <linux/mm.h>
+#include <linux/overflow.h>
#include <sound/pcm_params.h>
#include "virtio_card.h"
@@ -13,6 +16,7 @@
* @xfer: Request header payload.
* @status: Response header payload.
* @length: Data length in bytes.
+ * @data_offset: Offset in the driver-owned PCM buffer on NOMMU systems.
* @sgs: Payload scatter-gather table.
*/
struct virtio_pcm_msg {
@@ -20,6 +24,9 @@ struct virtio_pcm_msg {
struct virtio_snd_pcm_xfer xfer;
struct virtio_snd_pcm_status status;
size_t length;
+#ifndef CONFIG_MMU
+ size_t data_offset;
+#endif
struct scatterlist sgs[];
};
@@ -37,9 +44,8 @@ enum pcm_msg_sg_index {
};
/**
- * virtsnd_pcm_sg_num() - Count the number of sg-elements required to represent
- * vmalloc'ed buffer.
- * @data: Pointer to vmalloc'ed buffer.
+ * virtsnd_pcm_sg_num() - Count the sg-elements for a page-backed buffer.
+ * @data: Pointer to the page-backed buffer.
* @length: Buffer size.
*
* Context: Any context.
@@ -47,15 +53,19 @@ enum pcm_msg_sg_index {
*/
static int virtsnd_pcm_sg_num(u8 *data, unsigned int length)
{
- phys_addr_t sg_address;
- unsigned int sg_length;
+ phys_addr_t sg_address = 0;
+ unsigned int sg_length = 0;
int num = 0;
while (length) {
struct page *pg = vmalloc_to_page(data);
- phys_addr_t pg_address = page_to_phys(pg);
+ phys_addr_t pg_address;
size_t pg_length;
+ if (!pg)
+ return -EINVAL;
+ pg_address = page_to_phys(pg);
+
pg_length = PAGE_SIZE - offset_in_page(data);
if (pg_length > length)
pg_length = length;
@@ -75,11 +85,80 @@ static int virtsnd_pcm_sg_num(u8 *data, unsigned int length)
return num;
}
+#ifdef CONFIG_MMU
+/**
+ * virtsnd_pcm_dma_pages_put() - Release retained PCM buffer pages.
+ * @vss: VirtIO PCM substream.
+ *
+ * Context: Process context.
+ */
+static void virtsnd_pcm_dma_pages_put(struct virtio_pcm_substream *vss)
+{
+ while (vss->ndma_pages)
+ put_page(vss->dma_pages[--vss->ndma_pages]);
+ kfree(vss->dma_pages);
+ vss->dma_pages = NULL;
+}
+
/**
- * virtsnd_pcm_sg_from() - Build sg-list from vmalloc'ed buffer.
+ * virtsnd_pcm_dma_pages_get() - Retain the pages backing the PCM buffer.
+ * @vss: VirtIO PCM substream.
+ * @data: PCM buffer address.
+ * @length: PCM buffer size.
+ *
+ * Keep the backing pages alive while virtqueue descriptors can still refer to
+ * them, even if ALSA releases the vmalloc mapping after a failed drain.
+ *
+ * Context: Process context.
+ * Return: 0 on success, -errno on failure.
+ */
+static int virtsnd_pcm_dma_pages_get(struct virtio_pcm_substream *vss,
+ u8 *data, size_t length)
+{
+ size_t i;
+ size_t npages;
+ size_t span;
+
+ if (!data || !length || !is_vmalloc_addr(data))
+ return -EINVAL;
+ if (vss->dma_pages || vss->ndma_pages)
+ return -EBUSY;
+
+ if (check_add_overflow(length, (size_t)offset_in_page(data), &span))
+ return -EOVERFLOW;
+
+ npages = span / PAGE_SIZE + !!(span % PAGE_SIZE);
+ if (npages > UINT_MAX)
+ return -EOVERFLOW;
+
+ vss->dma_pages = kcalloc(npages, sizeof(*vss->dma_pages), GFP_KERNEL);
+ if (!vss->dma_pages)
+ return -ENOMEM;
+
+ for (i = 0; i < npages; i++) {
+ struct page *page = vmalloc_to_page(data + i * PAGE_SIZE);
+
+ if (!page)
+ goto err_put_pages;
+
+ get_page(page);
+ vss->dma_pages[vss->ndma_pages++] = page;
+ }
+
+ return 0;
+
+err_put_pages:
+ virtsnd_pcm_dma_pages_put(vss);
+
+ return -EINVAL;
+}
+#endif
+
+/**
+ * virtsnd_pcm_sg_from() - Build an sg-list from a page-backed buffer.
* @sgs: Preallocated sg-list to populate.
* @nsgs: The maximum number of elements in the @sgs.
- * @data: Pointer to vmalloc'ed buffer.
+ * @data: Pointer to the page-backed buffer.
* @length: Buffer size.
*
* Splits the buffer into physically contiguous parts and makes an sg-list of
@@ -132,36 +211,99 @@ static void virtsnd_pcm_sg_from(struct scatterlist *sgs, int nsgs, u8 *data,
int virtsnd_pcm_msg_alloc(struct virtio_pcm_substream *vss,
unsigned int periods, unsigned int period_bytes)
{
- struct snd_pcm_runtime *runtime = vss->substream->runtime;
+ struct snd_pcm_runtime *runtime;
+ size_t buffer_bytes;
unsigned int i;
+ int rc;
- vss->msgs = kzalloc_objs(*vss->msgs, periods);
- if (!vss->msgs)
+ if (vss->msgs || vss->nmsgs || vss->dma_pages || vss->ndma_pages)
+ return -EBUSY;
+#ifndef CONFIG_MMU
+ if (vss->owned_dma_area || vss->owned_dma_bytes)
+ return -EBUSY;
+#endif
+ if (!periods || !period_bytes)
+ return -EINVAL;
+
+ if (check_mul_overflow((size_t)periods, (size_t)period_bytes,
+ &buffer_bytes))
+ return -EOVERFLOW;
+
+ if (!vss->substream || !vss->substream->runtime)
+ return -EINVAL;
+ runtime = vss->substream->runtime;
+ if (!runtime->dma_area || buffer_bytes > runtime->dma_bytes)
+ return -EINVAL;
+
+#ifdef CONFIG_MMU
+ rc = virtsnd_pcm_dma_pages_get(vss, runtime->dma_area, buffer_bytes);
+ if (rc)
+ return rc;
+#else
+ vss->owned_dma_area = kzalloc(buffer_bytes, GFP_KERNEL);
+ if (!vss->owned_dma_area)
return -ENOMEM;
+ vss->owned_dma_bytes = buffer_bytes;
+#endif
+
+ vss->msgs = kzalloc_objs(*vss->msgs, periods);
+ if (!vss->msgs) {
+ rc = -ENOMEM;
+ goto err_free_pages;
+ }
vss->nmsgs = periods;
for (i = 0; i < periods; ++i) {
- u8 *data = runtime->dma_area + period_bytes * i;
- int sg_num = virtsnd_pcm_sg_num(data, period_bytes);
+ int sg_num;
struct virtio_pcm_msg *msg;
+ u8 *data;
+#ifdef CONFIG_MMU
+ data = runtime->dma_area + (size_t)period_bytes * i;
+#else
+ data = vss->owned_dma_area + (size_t)period_bytes * i;
+#endif
+ sg_num = virtsnd_pcm_sg_num(data, period_bytes);
+ if (sg_num < 0) {
+ rc = sg_num;
+ goto err_free_msgs;
+ }
msg = kzalloc_flex(*msg, sgs, sg_num + 2);
- if (!msg)
- return -ENOMEM;
+ if (!msg) {
+ rc = -ENOMEM;
+ goto err_free_msgs;
+ }
msg->substream = vss;
+ vss->msgs[i] = msg;
sg_init_one(&msg->sgs[PCM_MSG_SG_XFER], &msg->xfer,
sizeof(msg->xfer));
sg_init_one(&msg->sgs[PCM_MSG_SG_STATUS], &msg->status,
sizeof(msg->status));
+#ifndef CONFIG_MMU
+ msg->data_offset = (size_t)period_bytes * i;
+#endif
virtsnd_pcm_sg_from(&msg->sgs[PCM_MSG_SG_DATA], sg_num, data,
period_bytes);
-
- vss->msgs[i] = msg;
}
return 0;
+
+err_free_msgs:
+ virtsnd_pcm_msg_free(vss);
+ return rc;
+
+err_free_pages:
+#ifdef CONFIG_MMU
+ virtsnd_pcm_dma_pages_put(vss);
+#else
+ kfree(vss->owned_dma_area);
+ vss->owned_dma_area = NULL;
+ vss->owned_dma_bytes = 0;
+#endif
+
+ return rc;
}
/**
@@ -177,6 +319,13 @@ void virtsnd_pcm_msg_free(struct virtio_pcm_substream *vss)
for (i = 0; vss->msgs && i < vss->nmsgs; ++i)
kfree(vss->msgs[i]);
kfree(vss->msgs);
+#ifdef CONFIG_MMU
+ virtsnd_pcm_dma_pages_put(vss);
+#else
+ kfree(vss->owned_dma_area);
+ vss->owned_dma_area = NULL;
+ vss->owned_dma_bytes = 0;
+#endif
vss->msgs = NULL;
vss->nmsgs = 0;
@@ -249,6 +398,33 @@ int virtsnd_pcm_msg_send(struct virtio_pcm_substream *vss, unsigned long offset,
msg->xfer.stream_id = cpu_to_le32(vss->sid);
memset(&msg->status, 0, sizeof(msg->status));
+#ifndef CONFIG_MMU
+ if (vss->direction == SNDRV_PCM_STREAM_PLAYBACK) {
+ struct snd_pcm_runtime *runtime =
+ vss->substream ? vss->substream->runtime : NULL;
+ size_t data_bytes = msg->length;
+
+ if (!runtime || !runtime->dma_area || !vss->owned_dma_area ||
+ msg->data_offset > runtime->dma_bytes ||
+ data_bytes > runtime->dma_bytes - msg->data_offset ||
+ msg->data_offset > vss->owned_dma_bytes ||
+ data_bytes > vss->owned_dma_bytes - msg->data_offset) {
+ dev_err_ratelimited(&vdev->dev,
+ "SID %u: invalid playback buffer for I/O message\n",
+ vss->sid);
+ msg->length -= n;
+ if (msg_count != vss->msg_count &&
+ !(vss->features &
+ (1U << VIRTIO_SND_PCM_F_MSG_POLLING)) &&
+ virtqueue_kick_prepare(vqueue))
+ virtqueue_notify(vqueue);
+ return -EINVAL;
+ }
+ memcpy(vss->owned_dma_area + msg->data_offset,
+ runtime->dma_area + msg->data_offset, data_bytes);
+ }
+#endif
+
if (vss->direction == SNDRV_PCM_STREAM_PLAYBACK)
rc = virtqueue_add_sgs(vqueue, psgs, 2, 1, msg,
GFP_ATOMIC);
@@ -363,15 +539,48 @@ static void virtsnd_pcm_msg_complete(struct virtio_pcm_msg *msg,
* the hw_ptr value is unconditionally advanced.
*/
guard(spinlock)(&vss->lock);
+#ifndef CONFIG_MMU
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ vss->xfer_enabled && written_bytes > sizeof(msg->status)) {
+ struct snd_pcm_runtime *runtime =
+ vss->substream ? vss->substream->runtime : NULL;
+ size_t reported_bytes = written_bytes - sizeof(msg->status);
+ size_t data_bytes = min_t(size_t, reported_bytes, msg->length);
+
+ if (!runtime || !runtime->dma_area || !vss->owned_dma_area ||
+ msg->data_offset > runtime->dma_bytes ||
+ data_bytes > runtime->dma_bytes - msg->data_offset ||
+ msg->data_offset > vss->owned_dma_bytes ||
+ data_bytes > vss->owned_dma_bytes - msg->data_offset)
+ dev_err_ratelimited(&vss->snd->vdev->dev,
+ "SID %u: invalid capture buffer for I/O message\n",
+ vss->sid);
+ else
+ memcpy(runtime->dma_area + msg->data_offset,
+ vss->owned_dma_area + msg->data_offset, data_bytes);
+ }
+#endif
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ written_bytes > sizeof(msg->status) &&
+ written_bytes - sizeof(msg->status) > msg->length)
+ dev_err_ratelimited(&vss->snd->vdev->dev,
+ "SID %u: capture length exceeds I/O message capacity\n",
+ vss->sid);
/*
* If the capture substream returned an incorrect status, then just
- * increase the hw_ptr by the message size.
+ * increase the hw_ptr by the message size. Clamp valid capture lengths to
+ * the actual SG payload capacity before accounting them.
*/
if (vss->direction == SNDRV_PCM_STREAM_PLAYBACK ||
- written_bytes <= sizeof(msg->status))
+ written_bytes <= sizeof(msg->status)) {
vss->hw_ptr += msg->length;
- else
- vss->hw_ptr += written_bytes - sizeof(msg->status);
+ } else {
+ size_t data_bytes = min_t(size_t,
+ written_bytes - sizeof(msg->status),
+ msg->length);
+
+ vss->hw_ptr += data_bytes;
+ }
if (vss->hw_ptr >= vss->buffer_bytes)
vss->hw_ptr -= vss->buffer_bytes;
@@ -387,7 +596,6 @@ static void virtsnd_pcm_msg_complete(struct virtio_pcm_msg *msg,
runtime->delay =
bytes_to_frames(runtime,
le32_to_cpu(msg->status.latency_bytes));
-
schedule_work(&vss->elapsed_period);
} else if (!vss->msg_count) {
wake_up_all(&vss->msg_empty);
diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
index 970499fe419b..f74bf365065b 100644
--- a/sound/virtio/virtio_pcm_ops.c
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -25,6 +25,8 @@
* signal or due to a timeout. In this case, the device can still access
* messages even after calling ops->hw_free(). It can also issue an interrupt,
* and the interrupt handler will also try to access message structures.
+ * The messages retain references to the PCM buffer pages until they are
+ * freed, so late DMA does not target pages released by ALSA.
*
* Therefore, freeing of already allocated messages occurs:
*
--
2.54.0 (Apple Git-157)
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 5/5] ALSA: virtio: Keep control payloads alive through DMA completion
2026-10-13 8:57 [PATCH v3 0/5] ALSA: virtio: Fix request lifetime and error recovery Joonho Lee
` (3 preceding siblings ...)
2026-10-13 8:57 ` [PATCH v3 4/5] ALSA: virtio: Keep PCM DMA buffers alive through device ownership Joonho Lee
@ 2026-10-13 8:57 ` Joonho Lee
4 siblings, 0 replies; 6+ messages in thread
From: Joonho Lee @ 2026-10-13 8:57 UTC (permalink / raw)
To: Takashi Iwai
Cc: Michael S . Tsirkin, Jaroslav Kysela, virtualization, linux-sound,
linux-kernel, Matias Ezequiel Vara Larsen, Eugenio Perez
Control requests can expose caller-owned payloads to the device after a
synchronous send times out. Keep request and response headers and payloads
in the reference-counted message so caller cleanup cannot invalidate DMA.
Keep the queue reference until device completion or post-reset cleanup,
even when the caller times out or is interrupted. Consume the caller's
reference on every send path, and let response readers retain an extra
reference through copyout. Check payload sizes and allocation arithmetic
before building the scatterlists.
A control value payload is also read by the CPU as a typed structure. DMA
cacheline alignment alone does not guarantee the structure's CPU alignment.
Align regions to the greater of the DMA cacheline and control-value type
alignment, and reserve prefix slack from the buffer base so typed accesses
remain aligned while scatterlist regions stay on cacheline boundaries.
Fixes: 9d45e514da88 ("ALSA: virtio: handling control messages")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
Changes in v3:
- Respect both DMA and CPU type alignment, remove the +1 offset,
and retain alignment for every region, including odd-sized payloads.
sound/virtio/virtio_ctl_msg.c | 275 ++++++++++++++++++++++++++--------
sound/virtio/virtio_ctl_msg.h | 24 ++-
sound/virtio/virtio_kctl.c | 104 +++++++------
sound/virtio/virtio_pcm_msg.c | 2 +-
4 files changed, 285 insertions(+), 120 deletions(-)
diff --git a/sound/virtio/virtio_ctl_msg.c b/sound/virtio/virtio_ctl_msg.c
index 7114bea4033a..2e0ff15e729d 100644
--- a/sound/virtio/virtio_ctl_msg.c
+++ b/sound/virtio/virtio_ctl_msg.c
@@ -3,7 +3,10 @@
* virtio-snd: Virtio sound device
* Copyright (C) 2021 OpenSynergy GmbH
*/
+#include <linux/dma-mapping.h>
+#include <linux/limits.h>
#include <linux/moduleparam.h>
+#include <linux/overflow.h>
#include <linux/virtio_config.h>
#include "virtio_card.h"
@@ -11,7 +14,12 @@
/**
* struct virtio_snd_msg - Control message.
* @sg_request: Scattergather list containing a device request (header).
+ * @sg_request_data: Scattergather list containing request data.
* @sg_response: Scattergather list containing a device response (status).
+ * @sg_response_data: Scattergather list containing response data.
+ * @dma_buffer: Message-owned storage for all DMA-visible SG regions.
+ * @request_data_size: Size of the request data buffer.
+ * @response_data_size: Size of the response data buffer.
* @list: Pending message list entry.
* @notify: Request completed notification.
* @ref_count: Reference count used to manage a message lifetime.
@@ -19,7 +27,12 @@
*/
struct virtio_snd_msg {
struct scatterlist sg_request;
+ struct scatterlist sg_request_data;
struct scatterlist sg_response;
+ struct scatterlist sg_response_data;
+ void *dma_buffer;
+ size_t request_data_size;
+ size_t response_data_size;
struct list_head list;
struct completion notify;
refcount_t ref_count;
@@ -47,8 +60,10 @@ void virtsnd_ctl_msg_ref(struct virtio_snd_msg *msg)
*/
void virtsnd_ctl_msg_unref(struct virtio_snd_msg *msg)
{
- if (refcount_dec_and_test(&msg->ref_count))
+ if (refcount_dec_and_test(&msg->ref_count)) {
+ kfree(msg->dma_buffer);
kfree(msg);
+ }
}
/**
@@ -73,10 +88,71 @@ void *virtsnd_ctl_msg_response(struct virtio_snd_msg *msg)
return sg_virt(&msg->sg_response);
}
+/**
+ * virtsnd_ctl_msg_request_data() - Get the request data buffer.
+ * @msg: Control message.
+ *
+ * Context: Any context.
+ */
+void *virtsnd_ctl_msg_request_data(struct virtio_snd_msg *msg)
+{
+ return msg->request_data_size ? sg_virt(&msg->sg_request_data) : NULL;
+}
+
+/**
+ * virtsnd_ctl_msg_response_data() - Get the response data buffer.
+ * @msg: Control message.
+ *
+ * Context: Any context.
+ */
+void *virtsnd_ctl_msg_response_data(struct virtio_snd_msg *msg)
+{
+ return msg->response_data_size ? sg_virt(&msg->sg_response_data) : NULL;
+}
+
+/**
+ * virtsnd_ctl_msg_sg_init() - Place one DMA region on cacheline boundaries.
+ * @sg: Scattergather entry for the region.
+ * @cursor: Current location in the message-owned DMA buffer.
+ * @size: Region length.
+ * @alignment: Alignment for DMA cachelines and typed CPU accesses.
+ *
+ * Context: Any context during message allocation.
+ * Return: 0 on success, -EOVERFLOW if the address cannot be represented.
+ */
+static int virtsnd_ctl_msg_sg_init(struct scatterlist *sg, u8 **cursor,
+ size_t size, size_t alignment)
+{
+ uintptr_t address = (uintptr_t)*cursor;
+ uintptr_t end;
+ size_t padding;
+
+ padding = address % alignment;
+ if (padding &&
+ check_add_overflow(address, (uintptr_t)(alignment - padding),
+ &address))
+ return -EOVERFLOW;
+
+ if (check_add_overflow(address, (uintptr_t)size, &end))
+ return -EOVERFLOW;
+
+ padding = end % alignment;
+ if (padding &&
+ check_add_overflow(end, (uintptr_t)(alignment - padding), &end))
+ return -EOVERFLOW;
+
+ sg_init_one(sg, (void *)address, (unsigned int)size);
+ *cursor = (u8 *)end;
+
+ return 0;
+}
+
/**
* virtsnd_ctl_msg_alloc() - Allocate and initialize a control message.
* @request_size: Size of request header.
* @response_size: Size of response header.
+ * @request_data_size: Size of request data buffer.
+ * @response_data_size: Size of response data buffer.
* @gfp: Kernel flags for memory allocation.
*
* The message will be automatically freed when the ref_count value is 0.
@@ -85,20 +161,85 @@ void *virtsnd_ctl_msg_response(struct virtio_snd_msg *msg)
* Return: Allocated message on success, NULL on failure.
*/
struct virtio_snd_msg *virtsnd_ctl_msg_alloc(size_t request_size,
- size_t response_size, gfp_t gfp)
+ size_t response_size,
+ size_t request_data_size,
+ size_t response_data_size,
+ gfp_t gfp)
{
struct virtio_snd_msg *msg;
+ struct scatterlist *sgs[4];
+ size_t sizes[4] = {
+ request_size,
+ response_size,
+ request_data_size,
+ response_data_size,
+ };
+ size_t alignment;
+ size_t dma_buffer_size;
+ size_t aligned_size;
+ size_t padding;
+ u8 *data;
+ unsigned int i;
+ int cache_alignment;
+ int rc;
- if (!request_size || !response_size)
+ if (!request_size || !response_size || request_size > UINT_MAX ||
+ response_size > UINT_MAX || request_data_size > UINT_MAX ||
+ response_data_size > UINT_MAX)
return NULL;
+ cache_alignment = dma_get_cache_alignment();
+ if (cache_alignment <= 0)
+ return NULL;
+ /* DMA coherence does not remove the CPU's type alignment requirement. */
+ alignment = max_t(size_t, cache_alignment,
+ __alignof__(struct virtio_snd_ctl_value));
+
+ /* Reserve prefix alignment slack before the first DMA region. */
+ dma_buffer_size = alignment - 1;
+ for (i = 0; i < ARRAY_SIZE(sizes); ++i) {
+ if (!sizes[i])
+ continue;
+
+ padding = sizes[i] % alignment;
+ aligned_size = sizes[i];
+ if (padding &&
+ check_add_overflow(aligned_size, alignment - padding,
+ &aligned_size))
+ return NULL;
+
+ if (check_add_overflow(dma_buffer_size, aligned_size,
+ &dma_buffer_size))
+ return NULL;
+ }
- msg = kzalloc(sizeof(*msg) + request_size + response_size, gfp);
+ msg = kzalloc(sizeof(*msg), gfp);
if (!msg)
return NULL;
- sg_init_one(&msg->sg_request, (u8 *)msg + sizeof(*msg), request_size);
- sg_init_one(&msg->sg_response, (u8 *)msg + sizeof(*msg) + request_size,
- response_size);
+ msg->dma_buffer = kzalloc(dma_buffer_size, gfp);
+ if (!msg->dma_buffer) {
+ kfree(msg);
+ return NULL;
+ }
+ data = msg->dma_buffer;
+ sgs[0] = &msg->sg_request;
+ sgs[1] = &msg->sg_response;
+ sgs[2] = &msg->sg_request_data;
+ sgs[3] = &msg->sg_response_data;
+ for (i = 0; i < ARRAY_SIZE(sizes); ++i) {
+ if (!sizes[i])
+ continue;
+
+ rc = virtsnd_ctl_msg_sg_init(sgs[i], &data, sizes[i], alignment);
+ if (rc) {
+ kfree(msg->dma_buffer);
+ kfree(msg);
+ return NULL;
+ }
+ }
+
+ msg->request_data_size = request_data_size;
+ msg->response_data_size = response_data_size;
INIT_LIST_HEAD(&msg->list);
init_completion(&msg->notify);
@@ -112,17 +253,15 @@ struct virtio_snd_msg *virtsnd_ctl_msg_alloc(size_t request_size,
* virtsnd_ctl_msg_send() - Send a control message.
* @snd: VirtIO sound device.
* @msg: Control message.
- * @out_sgs: Additional sg-list to attach to the request header (may be NULL).
- * @in_sgs: Additional sg-list to attach to the response header (may be NULL).
* @nowait: Flag indicating whether to wait for completion.
*
* Context: Any context. Takes and releases the control queue spinlock.
* May sleep if @nowait is false.
+ * Consumes the caller's reference on every return path.
* Return: 0 on success, -errno on failure.
*/
int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
- struct scatterlist *out_sgs,
- struct scatterlist *in_sgs, bool nowait)
+ bool nowait)
{
struct virtio_device *vdev = snd->vdev;
struct virtio_snd_queue *queue = virtsnd_control_queue(snd);
@@ -133,6 +272,7 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
unsigned int nins = 0;
struct scatterlist *psgs[4];
bool notify = false;
+ unsigned long flags;
int rc;
virtsnd_ctl_msg_ref(msg);
@@ -142,27 +282,27 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
response->code = cpu_to_le32(VIRTIO_SND_S_IO_ERR);
psgs[nouts++] = &msg->sg_request;
- if (out_sgs)
- psgs[nouts++] = out_sgs;
+ if (msg->request_data_size)
+ psgs[nouts++] = &msg->sg_request_data;
psgs[nouts + nins++] = &msg->sg_response;
- if (in_sgs)
- psgs[nouts + nins++] = in_sgs;
-
- scoped_guard(spinlock_irqsave, &queue->lock) {
- if (snd->ctl_msgs_blocked)
- rc = -ENODEV;
- else
- rc = virtqueue_add_sgs(queue->vqueue, psgs, nouts, nins,
- msg, GFP_ATOMIC);
- if (!rc) {
- notify = virtqueue_kick_prepare(queue->vqueue);
+ if (msg->response_data_size)
+ psgs[nouts + nins++] = &msg->sg_response_data;
- list_add_tail(&msg->list, &snd->ctl_msgs);
- if (notify)
- virtqueue_notify(queue->vqueue);
- }
+ spin_lock_irqsave(&queue->lock, flags);
+ if (snd->ctl_msgs_blocked)
+ rc = -ENODEV;
+ else
+ rc = virtqueue_add_sgs(queue->vqueue, psgs, nouts, nins, msg,
+ GFP_ATOMIC);
+ if (!rc) {
+ notify = virtqueue_kick_prepare(queue->vqueue);
+
+ list_add_tail(&msg->list, &snd->ctl_msgs);
+ if (notify)
+ virtqueue_notify(queue->vqueue);
}
+ spin_unlock_irqrestore(&queue->lock, flags);
if (rc) {
dev_err(&vdev->dev, "failed to send control message (0x%08x)\n",
@@ -173,46 +313,36 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
* called, it is necessary to decrement the reference count.
*/
virtsnd_ctl_msg_unref(msg);
-
- goto on_exit;
- }
-
- if (nowait)
- goto on_exit;
-
- rc = wait_for_completion_interruptible_timeout(&msg->notify, js);
- if (rc <= 0) {
+ } else if (!nowait) {
+ rc = wait_for_completion_interruptible_timeout(&msg->notify, js);
if (!rc) {
dev_err(&vdev->dev,
"control message (0x%08x) timeout\n",
le32_to_cpu(request->code));
rc = -ETIMEDOUT;
+ } else if (rc > 0) {
+ if (READ_ONCE(msg->cancelled)) {
+ rc = -ENODEV;
+ } else {
+ switch (le32_to_cpu(response->code)) {
+ case VIRTIO_SND_S_OK:
+ rc = 0;
+ break;
+ case VIRTIO_SND_S_NOT_SUPP:
+ rc = -EOPNOTSUPP;
+ break;
+ case VIRTIO_SND_S_IO_ERR:
+ rc = -EIO;
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+ }
}
-
- goto on_exit;
- }
-
- if (READ_ONCE(msg->cancelled)) {
- rc = -ENODEV;
- goto on_exit;
- }
-
- switch (le32_to_cpu(response->code)) {
- case VIRTIO_SND_S_OK:
- rc = 0;
- break;
- case VIRTIO_SND_S_NOT_SUPP:
- rc = -EOPNOTSUPP;
- break;
- case VIRTIO_SND_S_IO_ERR:
- rc = -EIO;
- break;
- default:
- rc = -EINVAL;
- break;
}
-on_exit:
+ /* Drop the reference consumed by this send attempt. */
virtsnd_ctl_msg_unref(msg);
return rc;
@@ -308,10 +438,23 @@ int virtsnd_ctl_query_info(struct virtio_snd *snd, int command, int start_id,
{
struct virtio_snd_msg *msg;
struct virtio_snd_query_info *query;
- struct scatterlist sg;
+ size_t info_size;
+ int rc;
+
+ if (count < 0)
+ return -EINVAL;
+
+ if (size > UINT_MAX ||
+ check_mul_overflow((size_t)count, size, &info_size) ||
+ info_size > UINT_MAX)
+ return -EOVERFLOW;
+
+ if (info_size && !info)
+ return -EINVAL;
msg = virtsnd_ctl_msg_alloc(sizeof(*query),
- sizeof(struct virtio_snd_hdr), GFP_KERNEL);
+ sizeof(struct virtio_snd_hdr), 0, info_size,
+ GFP_KERNEL);
if (!msg)
return -ENOMEM;
@@ -321,9 +464,13 @@ int virtsnd_ctl_query_info(struct virtio_snd *snd, int command, int start_id,
query->count = cpu_to_le32(count);
query->size = cpu_to_le32(size);
- sg_init_one(&sg, info, count * size);
+ virtsnd_ctl_msg_ref(msg);
+ rc = virtsnd_ctl_msg_send(snd, msg, false);
+ if (!rc && info_size)
+ memcpy(info, virtsnd_ctl_msg_response_data(msg), info_size);
+ virtsnd_ctl_msg_unref(msg);
- return virtsnd_ctl_msg_send(snd, msg, NULL, &sg, false);
+ return rc;
}
/**
diff --git a/sound/virtio/virtio_ctl_msg.h b/sound/virtio/virtio_ctl_msg.h
index d917435409d0..f867b9b9233d 100644
--- a/sound/virtio/virtio_ctl_msg.h
+++ b/sound/virtio/virtio_ctl_msg.h
@@ -21,20 +21,28 @@ void *virtsnd_ctl_msg_request(struct virtio_snd_msg *msg);
void *virtsnd_ctl_msg_response(struct virtio_snd_msg *msg);
struct virtio_snd_msg *virtsnd_ctl_msg_alloc(size_t request_size,
- size_t response_size, gfp_t gfp);
+ size_t response_size,
+ size_t request_data_size,
+ size_t response_data_size,
+ gfp_t gfp);
+
+void *virtsnd_ctl_msg_request_data(struct virtio_snd_msg *msg);
+
+void *virtsnd_ctl_msg_response_data(struct virtio_snd_msg *msg);
int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
- struct scatterlist *out_sgs,
- struct scatterlist *in_sgs, bool nowait);
+ bool nowait);
/**
* virtsnd_ctl_msg_send_sync() - Simplified sending of synchronous message.
* @snd: VirtIO sound device.
* @msg: Control message.
*
- * After returning from this function, the message will be deleted. If message
- * content is still needed, the caller must additionally to
- * virtsnd_ctl_msg_ref/unref() it.
+ * This function consumes the caller's message reference on return. A message
+ * accepted by the control queue keeps its queue reference until completion or
+ * post-reset cleanup, including when this function returns on timeout or
+ * interruption. Callers that read the response after success must retain an
+ * additional reference through that access.
*
* The msg_timeout_ms module parameter defines the message completion timeout.
* If the message is not completed within this time, the function will return an
@@ -49,7 +57,7 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
static inline int virtsnd_ctl_msg_send_sync(struct virtio_snd *snd,
struct virtio_snd_msg *msg)
{
- return virtsnd_ctl_msg_send(snd, msg, NULL, NULL, false);
+ return virtsnd_ctl_msg_send(snd, msg, false);
}
/**
@@ -63,7 +71,7 @@ static inline int virtsnd_ctl_msg_send_sync(struct virtio_snd *snd,
static inline int virtsnd_ctl_msg_send_async(struct virtio_snd *snd,
struct virtio_snd_msg *msg)
{
- return virtsnd_ctl_msg_send(snd, msg, NULL, NULL, true);
+ return virtsnd_ctl_msg_send(snd, msg, true);
}
void virtsnd_ctl_msg_cancel_all(struct virtio_snd *snd);
diff --git a/sound/virtio/virtio_kctl.c b/sound/virtio/virtio_kctl.c
index 45f7b6a5b308..4cf75dedbe9a 100644
--- a/sound/virtio/virtio_kctl.c
+++ b/sound/virtio/virtio_kctl.c
@@ -3,8 +3,10 @@
* virtio-snd: Virtio sound device
* Copyright (C) 2022 OpenSynergy GmbH
*/
-#include <sound/control.h>
+#include <linux/limits.h>
+#include <linux/overflow.h>
#include <linux/virtio_config.h>
+#include <sound/control.h>
#include "virtio_card.h"
@@ -161,10 +163,12 @@ static int virtsnd_kctl_get(struct snd_kcontrol *kcontrol,
unsigned int i;
int rc;
- msg = virtsnd_ctl_msg_alloc(request_size, response_size, GFP_KERNEL);
+ msg = virtsnd_ctl_msg_alloc(request_size, response_size, 0, 0,
+ GFP_KERNEL);
if (!msg)
return -ENOMEM;
+ /* Response lifetime through ALSA value copyout. */
virtsnd_ctl_msg_ref(msg);
hdr = virtsnd_ctl_msg_request(msg);
@@ -233,7 +237,8 @@ static int virtsnd_kctl_put(struct snd_kcontrol *kcontrol,
size_t response_size = sizeof(struct virtio_snd_hdr);
unsigned int i;
- msg = virtsnd_ctl_msg_alloc(request_size, response_size, GFP_KERNEL);
+ msg = virtsnd_ctl_msg_alloc(request_size, response_size, 0, 0,
+ GFP_KERNEL);
if (!msg)
return -ENOMEM;
@@ -288,23 +293,28 @@ static int virtsnd_kctl_tlv_op(struct snd_kcontrol *kcontrol, int op_flag,
struct virtio_snd *snd = snd_kcontrol_chip(kcontrol);
struct virtio_snd_msg *msg;
struct virtio_snd_ctl_hdr *hdr;
- unsigned int *tlv;
- struct scatterlist sg;
+ size_t request_data_size = 0;
+ size_t response_data_size = 0;
int rc;
+ switch (op_flag) {
+ case SNDRV_CTL_TLV_OP_READ:
+ response_data_size = size;
+ break;
+ case SNDRV_CTL_TLV_OP_WRITE:
+ case SNDRV_CTL_TLV_OP_CMD:
+ request_data_size = size;
+ break;
+ default:
+ return -EINVAL;
+ }
+
msg = virtsnd_ctl_msg_alloc(sizeof(*hdr), sizeof(struct virtio_snd_hdr),
+ request_data_size, response_data_size,
GFP_KERNEL);
if (!msg)
return -ENOMEM;
- tlv = kzalloc(size, GFP_KERNEL);
- if (!tlv) {
- rc = -ENOMEM;
- goto on_msg_unref;
- }
-
- sg_init_one(&sg, tlv, size);
-
hdr = virtsnd_ctl_msg_request(msg);
hdr->control_id = cpu_to_le32(kcontrol->private_value);
@@ -312,13 +322,16 @@ static int virtsnd_kctl_tlv_op(struct snd_kcontrol *kcontrol, int op_flag,
case SNDRV_CTL_TLV_OP_READ:
hdr->hdr.code = cpu_to_le32(VIRTIO_SND_R_CTL_TLV_READ);
- rc = virtsnd_ctl_msg_send(snd, msg, NULL, &sg, false);
- if (!rc) {
- if (copy_to_user(utlv, tlv, size))
- rc = -EFAULT;
- }
+ /* Response lifetime through user copyout. */
+ virtsnd_ctl_msg_ref(msg);
+ rc = virtsnd_ctl_msg_send(snd, msg, false);
+ if (!rc && size &&
+ copy_to_user(utlv, virtsnd_ctl_msg_response_data(msg),
+ size))
+ rc = -EFAULT;
+ virtsnd_ctl_msg_unref(msg);
- break;
+ return rc;
case SNDRV_CTL_TLV_OP_WRITE:
case SNDRV_CTL_TLV_OP_CMD:
if (op_flag == SNDRV_CTL_TLV_OP_WRITE)
@@ -327,28 +340,16 @@ static int virtsnd_kctl_tlv_op(struct snd_kcontrol *kcontrol, int op_flag,
hdr->hdr.code =
cpu_to_le32(VIRTIO_SND_R_CTL_TLV_COMMAND);
- if (copy_from_user(tlv, utlv, size)) {
- rc = -EFAULT;
- goto on_msg_unref;
- } else {
- rc = virtsnd_ctl_msg_send(snd, msg, &sg, NULL, false);
+ if (size && copy_from_user(virtsnd_ctl_msg_request_data(msg), utlv,
+ size)) {
+ virtsnd_ctl_msg_unref(msg);
+ return -EFAULT;
}
- break;
- default:
- rc = -EINVAL;
- /* We never get here - we listed all values for op_flag */
- WARN_ON(1);
- goto on_msg_unref;
+ return virtsnd_ctl_msg_send(snd, msg, false);
}
- kfree(tlv);
- return rc;
-
-on_msg_unref:
- virtsnd_ctl_msg_unref(msg);
- kfree(tlv);
- return rc;
+ return -EINVAL;
}
/**
@@ -369,27 +370,36 @@ static int virtsnd_kctl_get_enum_items(struct virtio_snd *snd, unsigned int cid)
struct virtio_snd_msg *msg;
struct virtio_snd_ctl_hdr *hdr;
unsigned int n = le32_to_cpu(kinfo->value.enumerated.items);
- struct scatterlist sg;
+ size_t items_size;
+ int rc;
- msg = virtsnd_ctl_msg_alloc(sizeof(*hdr),
- sizeof(struct virtio_snd_hdr), GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
+ if (check_mul_overflow((size_t)n, sizeof(*kctl->items), &items_size) ||
+ items_size > UINT_MAX)
+ return -EOVERFLOW;
kctl->items = devm_kcalloc(&vdev->dev, n, sizeof(*kctl->items),
GFP_KERNEL);
- if (!kctl->items) {
- virtsnd_ctl_msg_unref(msg);
+ if (!kctl->items)
return -ENOMEM;
- }
- sg_init_one(&sg, kctl->items, n * sizeof(*kctl->items));
+ msg = virtsnd_ctl_msg_alloc(sizeof(*hdr),
+ sizeof(struct virtio_snd_hdr), 0, items_size,
+ GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
hdr = virtsnd_ctl_msg_request(msg);
hdr->hdr.code = cpu_to_le32(VIRTIO_SND_R_CTL_ENUM_ITEMS);
hdr->control_id = cpu_to_le32(cid);
- return virtsnd_ctl_msg_send(snd, msg, NULL, &sg, false);
+ /* Item-buffer lifetime through the control-list copyout. */
+ virtsnd_ctl_msg_ref(msg);
+ rc = virtsnd_ctl_msg_send(snd, msg, false);
+ if (!rc && items_size)
+ memcpy(kctl->items, virtsnd_ctl_msg_response_data(msg), items_size);
+ virtsnd_ctl_msg_unref(msg);
+
+ return rc;
}
/**
diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
index b7f2334bb221..b54642a9521d 100644
--- a/sound/virtio/virtio_pcm_msg.c
+++ b/sound/virtio/virtio_pcm_msg.c
@@ -671,7 +671,7 @@ virtsnd_pcm_ctl_msg_alloc(struct virtio_pcm_substream *vss,
break;
}
- msg = virtsnd_ctl_msg_alloc(request_size, response_size, gfp);
+ msg = virtsnd_ctl_msg_alloc(request_size, response_size, 0, 0, gfp);
if (msg) {
struct virtio_snd_pcm_hdr *hdr = virtsnd_ctl_msg_request(msg);
--
2.54.0 (Apple Git-157)
^ permalink raw reply related [flat|nested] 6+ messages in thread