* [PATCH v1 2/5] ALSA: virtio: Recover PCM state after a failed start
2026-10-13 5:30 [PATCH v1 0/5] ALSA: virtio: Fix request lifetime and error recovery Joonho Lee
2026-10-13 5:30 ` [PATCH v1 1/5] ALSA: virtio: Notify queued PCM messages after send errors Joonho Lee
@ 2026-10-13 5:30 ` Joonho Lee
2026-10-13 5:30 ` [PATCH v1 3/5] ALSA: virtio: Retain control requests through device reset Joonho Lee
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Joonho Lee @ 2026-10-13 5:30 UTC (permalink / raw)
To: Anton Yakovlev, Takashi Iwai
Cc: Joonho Lee, Michael S. Tsirkin, Jaroslav Kysela, virtualization,
linux-sound, linux-kernel
Capture trigger queues I/O messages before sending PCM_START. If
allocation of the START control message fails, trigger returns an
error with capture I/O messages potentially pending, but without
marking the stream stopped or arming sync_stop. If sending START
fails, trigger also leaves transfers enabled. ALSA does not run the
action undo callback after this trigger failure, so the normal STOP
path cannot be relied on to drain those requests. Retrying START can
then submit the same capture messages again.
Disable transfers and mark the stream stopped after a failed start,
and arm runtime->stop_operating so ALSA invokes sync_stop. Make
sync_stop send RELEASE when the stream is stopped or has pending
I/O, and keep the stream stopped when RELEASE or queue draining
fails. Reject prepare, hw_params, and START retries until a later
sync_stop resolves the outstanding state.
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>
---
Dependency: patch 1 notifies capture messages successfully queued before
a later enqueue fails, so this error path can drain them via sync_stop.
Validation: source-extracted ASan/UBSan START allocation/send failures,
retry gating and sync_stop recovery; included in the W=1 builds through
patches 3 and 5. This commit was not separately kernel-built.
sound/virtio/virtio_pcm_ops.c | 63 +++++++++++++++++++++++++++++------
1 file changed, 53 insertions(+), 10 deletions(-)
diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
index 1091100a4c98..f2362f0dd80a 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,7 @@ 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);
+ vss->stopped |= !!virtsnd_pcm_msg_pending_num(vss);
vss->suspended = false;
/*
@@ -224,6 +233,12 @@ static int virtsnd_pcm_hw_params(struct snd_pcm_substream *substream,
vss->sid);
return -EBADFD;
}
+ if (vss->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),
@@ -274,6 +289,13 @@ static int virtsnd_pcm_prepare(struct snd_pcm_substream *substream)
struct virtio_device *vdev = vss->snd->vdev;
struct virtio_snd_msg *msg;
+ if (vss->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)) {
dev_err(&vdev->dev, "SID %u: invalid I/O queue state\n",
@@ -338,23 +360,32 @@ 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;
@@ -393,21 +424,32 @@ 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;
+ }
cancel_work_sync(&vss->elapsed_period);
-
- if (!vss->stopped)
+ if (!need_release)
return 0;
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ vss->stopped = true;
+
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,6 +463,7 @@ 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;
}
--
2.54.0 (Apple Git-157)
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v1 3/5] ALSA: virtio: Retain control requests through device reset
2026-10-13 5:30 [PATCH v1 0/5] ALSA: virtio: Fix request lifetime and error recovery Joonho Lee
2026-10-13 5:30 ` [PATCH v1 1/5] ALSA: virtio: Notify queued PCM messages after send errors Joonho Lee
2026-10-13 5:30 ` [PATCH v1 2/5] ALSA: virtio: Recover PCM state after a failed start Joonho Lee
@ 2026-10-13 5:30 ` Joonho Lee
2026-10-13 5:30 ` [PATCH v1 4/5] ALSA: virtio: Retain PCM DMA pages until messages are freed Joonho Lee
2026-10-13 5:30 ` [PATCH v1 5/5] ALSA: virtio: Keep control payloads in refcounted messages Joonho Lee
4 siblings, 0 replies; 6+ messages in thread
From: Joonho Lee @ 2026-10-13 5:30 UTC (permalink / raw)
To: Anton Yakovlev, Takashi Iwai
Cc: Joonho Lee, Michael S. Tsirkin, Jaroslav Kysela, virtualization,
linux-sound, linux-kernel
virtsnd_ctl_msg_cancel_all() currently completes and releases
pending messages before the device is reset. A sender can then free
a message while the virtqueue still holds its token, allowing a late
completion to access freed memory. Sending also notifies the device
after releasing the queue lock, so teardown can reset and delete the
queue between enqueue and notify.
Gate new requests and mark pending messages canceled under the queue
lock. Notify the device while still holding that lock, and retain
each queue reference until normal completion or post-reset cleanup.
Normal completion wakes a canceled sender after the device returns
ownership; cleanup wakes remaining senders only after reset and
virtqueue deletion.
Keep the control queue available through snd_card_free() so close-
time STOP and RELEASE commands can still be submitted.
Fixes: 9d45e514da88 ("ALSA: virtio: handling control messages")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
Dependency: apply with patch 5 for control payload lifetime protection.
This patch retains the message and its headers; caller-owned payloads
are addressed by patch 5.
Validation: ARM64 W=1 build and source-extracted ASan/UBSan teardown,
late-completion and cancellation tests. The final QEMU/KASAN reset test
covers the complete series, not this commit in isolation.
sound/virtio/virtio_card.c | 6 +++-
sound/virtio/virtio_card.h | 2 ++
sound/virtio/virtio_ctl_msg.c | 61 +++++++++++++++++++++++++++++++----
sound/virtio/virtio_ctl_msg.h | 4 +++
4 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
index 6f35276416fe..ed884327bb0e 100644
--- a/sound/virtio/virtio_card.c
+++ b/sound/virtio/virtio_card.c
@@ -349,13 +349,15 @@ 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);
+ virtsnd_ctl_msg_cancel_all(snd);
+
virtio_reset_device(vdev);
vdev->config->del_vqs(vdev);
+ virtsnd_ctl_msg_cleanup(snd);
for (i = 0; snd->substreams && i < snd->nsubstreams; ++i) {
struct virtio_pcm_substream *vss = &snd->substreams[i];
@@ -385,6 +387,7 @@ static int virtsnd_freeze(struct virtio_device *vdev)
virtio_reset_device(vdev);
vdev->config->del_vqs(vdev);
+ virtsnd_ctl_msg_cleanup(snd);
for (i = 0; i < snd->nsubstreams; ++i)
cancel_work_sync(&snd->substreams[i].elapsed_period);
@@ -412,6 +415,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..3ad1e1e1b205 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,32 @@ 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 virtio_reset_device() and del_vqs(), when no queue
+ * callback can still access a message.
+ *
+ * Context: Any context.
+ */
+void virtsnd_ctl_msg_cleanup(struct virtio_snd *snd)
{
struct virtio_snd_queue *queue = virtsnd_control_queue(snd);
@@ -243,6 +276,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,
--
2.54.0 (Apple Git-157)
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v1 4/5] ALSA: virtio: Retain PCM DMA pages until messages are freed
2026-10-13 5:30 [PATCH v1 0/5] ALSA: virtio: Fix request lifetime and error recovery Joonho Lee
` (2 preceding siblings ...)
2026-10-13 5:30 ` [PATCH v1 3/5] ALSA: virtio: Retain control requests through device reset Joonho Lee
@ 2026-10-13 5:30 ` Joonho Lee
2026-10-13 5:30 ` [PATCH v1 5/5] ALSA: virtio: Keep control payloads in refcounted messages Joonho Lee
4 siblings, 0 replies; 6+ messages in thread
From: Joonho Lee @ 2026-10-13 5:30 UTC (permalink / raw)
To: Anton Yakovlev, Takashi Iwai
Cc: Joonho Lee, Michael S. Tsirkin, Jaroslav Kysela, virtualization,
linux-sound, linux-kernel
PCM I/O scatterlists point directly into the ALSA runtime's vmalloc
buffer. If RELEASE or queue draining times out, ALSA still frees that
managed buffer during hw_free/close while the device may retain the
queued descriptors. Keeping only the virtio_pcm_msg objects alive does
not keep their backing pages allocated.
Take a reference to each page backing the PCM buffer before building the
I/O scatterlists, and release those references only when the messages
are freed after device ownership ends. This keeps late device DMA away
from pages that the allocator could otherwise reuse after ALSA releases
the vmalloc mapping.
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>
---
Validation: separate ARM64 W=1 module build at this commit. With the
full series, delayed capture completion passed two KASAN/QEMU runs;
the tree through patch 3 reproduced freed-page reads. Playback late
access and allocation-failure paths also have ASan/UBSan shim tests.
sound/virtio/virtio_pcm.h | 5 ++
sound/virtio/virtio_pcm_msg.c | 110 ++++++++++++++++++++++++++++++++--
sound/virtio/virtio_pcm_ops.c | 2 +
3 files changed, 112 insertions(+), 5 deletions(-)
diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h
index 8837dfc12828..56ac2769b05b 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,8 @@ 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.
* @msg_empty: Notify when msg_count is zero.
*/
struct virtio_pcm_substream {
@@ -60,6 +63,8 @@ struct virtio_pcm_substream {
struct virtio_pcm_msg **msgs;
unsigned int nmsgs;
unsigned int msg_count;
+ struct page **dma_pages;
+ unsigned int ndma_pages;
wait_queue_head_t msg_empty;
};
diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
index 5371f4985fd0..46bf4df65974 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"
@@ -75,6 +78,73 @@ static int virtsnd_pcm_sg_num(u8 *data, unsigned int length)
return num;
}
+/**
+ * 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_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;
+}
+
/**
* virtsnd_pcm_sg_from() - Build sg-list from vmalloc'ed buffer.
* @sgs: Preallocated sg-list to populate.
@@ -133,22 +203,42 @@ 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;
+ size_t buffer_bytes;
unsigned int i;
+ int rc;
+
+ if (vss->msgs || vss->nmsgs || vss->dma_pages || vss->ndma_pages)
+ return -EBUSY;
+
+ if (check_mul_overflow((size_t)periods, (size_t)period_bytes,
+ &buffer_bytes))
+ return -EOVERFLOW;
+
+ if (!runtime->dma_area || buffer_bytes > runtime->dma_bytes)
+ return -EINVAL;
+
+ rc = virtsnd_pcm_dma_pages_get(vss, runtime->dma_area, buffer_bytes);
+ if (rc)
+ return rc;
vss->msgs = kzalloc_objs(*vss->msgs, periods);
- if (!vss->msgs)
- return -ENOMEM;
+ 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;
+ u8 *data = runtime->dma_area + (size_t)period_bytes * i;
int sg_num = virtsnd_pcm_sg_num(data, period_bytes);
struct virtio_pcm_msg *msg;
msg = kzalloc_flex(*msg, sgs, sg_num + 2);
- if (!msg)
- return -ENOMEM;
+ if (!msg) {
+ rc = -ENOMEM;
+ goto err_free_msgs;
+ }
msg->substream = vss;
sg_init_one(&msg->sgs[PCM_MSG_SG_XFER], &msg->xfer,
@@ -162,6 +252,15 @@ int virtsnd_pcm_msg_alloc(struct virtio_pcm_substream *vss,
}
return 0;
+
+err_free_msgs:
+ virtsnd_pcm_msg_free(vss);
+ return rc;
+
+err_free_pages:
+ virtsnd_pcm_dma_pages_put(vss);
+
+ return rc;
}
/**
@@ -177,6 +276,7 @@ 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);
+ virtsnd_pcm_dma_pages_put(vss);
vss->msgs = NULL;
vss->nmsgs = 0;
diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
index f2362f0dd80a..82f2d2f9dd64 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 v1 5/5] ALSA: virtio: Keep control payloads in refcounted messages
2026-10-13 5:30 [PATCH v1 0/5] ALSA: virtio: Fix request lifetime and error recovery Joonho Lee
` (3 preceding siblings ...)
2026-10-13 5:30 ` [PATCH v1 4/5] ALSA: virtio: Retain PCM DMA pages until messages are freed Joonho Lee
@ 2026-10-13 5:30 ` Joonho Lee
4 siblings, 0 replies; 6+ messages in thread
From: Joonho Lee @ 2026-10-13 5:30 UTC (permalink / raw)
To: Anton Yakovlev, Takashi Iwai
Cc: Joonho Lee, Michael S. Tsirkin, Jaroslav Kysela, virtualization,
linux-sound, linux-kernel
Control requests can include scatterlists that point to caller-owned
payloads. A synchronous send can time out while the device still owns
those descriptors, so the caller can free a TLV or other payload before
a late device access.
Allocate request and response payloads with the refcounted control
message, retain separate header and payload scatterlist entries, and
copy caller data into or out of the owned payload while the caller still
holds a message reference. Check payload sizes and allocation arithmetic
before building the scatterlists.
Fixes: 9d45e514da88 ("ALSA: virtio: handling control messages")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
Dependency: patch 3 retains the queue reference until completion or
post-reset cleanup, which this payload ownership change relies on.
Validation: ARM64 W=1 build; two delayed PCM_INFO KASAN/QEMU runs
with subsequent capture/playback recovery; ASan/UBSan source-shim
tests for query, enum and TLV paths. No real TLV backend was tested.
sound/virtio/virtio_ctl_msg.c | 109 ++++++++++++++++++++++++++++------
sound/virtio/virtio_ctl_msg.h | 16 +++--
sound/virtio/virtio_kctl.c | 101 ++++++++++++++++---------------
sound/virtio/virtio_pcm_msg.c | 2 +-
4 files changed, 156 insertions(+), 72 deletions(-)
diff --git a/sound/virtio/virtio_ctl_msg.c b/sound/virtio/virtio_ctl_msg.c
index 3ad1e1e1b205..62d4c529922c 100644
--- a/sound/virtio/virtio_ctl_msg.c
+++ b/sound/virtio/virtio_ctl_msg.c
@@ -3,7 +3,9 @@
* virtio-snd: Virtio sound device
* Copyright (C) 2021 OpenSynergy GmbH
*/
+#include <linux/limits.h>
#include <linux/moduleparam.h>
+#include <linux/overflow.h>
#include <linux/virtio_config.h>
#include "virtio_card.h"
@@ -11,7 +13,11 @@
/**
* 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.
+ * @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 +25,11 @@
*/
struct virtio_snd_msg {
struct scatterlist sg_request;
+ struct scatterlist sg_request_data;
struct scatterlist sg_response;
+ struct scatterlist sg_response_data;
+ size_t request_data_size;
+ size_t response_data_size;
struct list_head list;
struct completion notify;
refcount_t ref_count;
@@ -73,10 +83,34 @@ 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_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 +119,43 @@ 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;
-
- if (!request_size || !response_size)
+ size_t total_size = sizeof(*msg);
+ u8 *data;
+
+ if (!request_size || !response_size || request_size > UINT_MAX ||
+ response_size > UINT_MAX || request_data_size > UINT_MAX ||
+ response_data_size > UINT_MAX ||
+ check_add_overflow(total_size, request_size, &total_size) ||
+ check_add_overflow(total_size, response_size, &total_size) ||
+ check_add_overflow(total_size, request_data_size, &total_size) ||
+ check_add_overflow(total_size, response_data_size, &total_size))
return NULL;
- msg = kzalloc(sizeof(*msg) + request_size + response_size, gfp);
+ msg = kzalloc(total_size, 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);
+ data = (u8 *)msg + sizeof(*msg);
+ sg_init_one(&msg->sg_request, data, (unsigned int)request_size);
+ data += request_size;
+ sg_init_one(&msg->sg_response, data, (unsigned int)response_size);
+ data += response_size;
+ msg->request_data_size = request_data_size;
+ msg->response_data_size = response_data_size;
+ if (request_data_size) {
+ sg_init_one(&msg->sg_request_data, data,
+ (unsigned int)request_data_size);
+ data += request_data_size;
+ }
+ if (response_data_size)
+ sg_init_one(&msg->sg_response_data, data,
+ (unsigned int)response_data_size);
INIT_LIST_HEAD(&msg->list);
init_completion(&msg->notify);
@@ -112,8 +169,6 @@ 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.
@@ -121,8 +176,7 @@ struct virtio_snd_msg *virtsnd_ctl_msg_alloc(size_t request_size,
* 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);
@@ -142,12 +196,12 @@ 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;
+ if (msg->response_data_size)
+ psgs[nouts + nins++] = &msg->sg_response_data;
scoped_guard(spinlock_irqsave, &queue->lock) {
if (snd->ctl_msgs_blocked)
@@ -307,10 +361,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;
@@ -320,9 +387,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..834ddeb82b90 100644
--- a/sound/virtio/virtio_ctl_msg.h
+++ b/sound/virtio/virtio_ctl_msg.h
@@ -21,11 +21,17 @@ 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.
@@ -49,7 +55,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 +69,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..44eb59d7d093 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,7 +163,8 @@ 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;
@@ -233,7 +236,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 +292,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 +321,15 @@ 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;
- }
+ 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 +338,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 +368,35 @@ 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);
+ 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 46bf4df65974..7fb945d0a36a 100644
--- a/sound/virtio/virtio_pcm_msg.c
+++ b/sound/virtio/virtio_pcm_msg.c
@@ -534,7 +534,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