From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=74.125.82.47 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1794201411; b=fJbBsEdrw1baHs6jQrj1prtKcrdS4nmqcQ0dOPUCPIAhhxRusSMUe6hQf+y6KPmcxEa7 auJiW/CbNLoaWZIEh+Q/FhN6THruPFRdCeq0QpQGxcYUwVa+5Yg50DZCavTpAsu63s606 ZZLEDUvNI6kOgGWcNL6fTQolgZ6mCCe4V5w5Xp/UZlQYWWA7p2c6zYiPTHLmK82lPG+h+ 6sLPcf8BiBsiV+P0eSQzH2XI54NuF2SKTknBF88w0k1dd9f7YSEXRSqndu9ApydpZpq04 Y1uCmDfBpMBNZ7Y2vu+BMKaUT1zwS+kso3vUkepmpLDJPQTFe07rMPPdO4WN2Xg/ixQ== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1794201411; h=DMARC-Filter:Received:DKIM-Signature:X-Google-DKIM-Signature: X-Forwarded-Encrypted:X-Gm-Message-State:X-Gm-Gg:X-Received:Received: From:To:Cc:Subject:Date:Message-ID:X-Mailer:In-Reply-To:References: MIME-Version:Content-Transfer-Encoding; bh=F9VadDTyIvxtwTesOFYe162uSd4ddwueP7W1xi3nolk=; b=irZoJ46KZneIxWwzjx8MW/n8IrIa3qr2NMRgRdR/UP+fLjCIvZrpByf3cjypGBzbtW8e AfTXhRE8dXmhj+bpBWn1rWOVXTQ1dV9efdmpMkc/o8yzg4qW3lFq1VkKPPAzvMvgdHmNi le8ltEQeF/ms63jInlslNe8hDFMf0P/8Q6aaQmjy7j7He8/OCjI5UlnTXaCzERgfUCGVa HIE9XDw4bEr6BMT3Cg//3GPso+3OfXUAXUjYy0nSW1gzFwRJVx0XIN1AnHDvTAHypd8N/ lWXVo4kOauVUc9LMswWIhDxBlB4WKSpTey/ETCatJkVC4JGgPXbr0SLVKiPALpr/K3Q== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20260707 header.b=mFAPGnWC; arc=none smtp.remote-ip=74.125.82.47 Received: from mail-dl1-f47.google.com (mail-dl1-f47.google.com [74.125.82.47]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9BC80606A331 for ; Mon, 9 Nov 2026 05:16:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.2 smtp.subspace.kernel.org 9BC80606A331 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20260707 header.b=mFAPGnWC Received: by mail-dl1-f47.google.com with SMTP id a92af1059eb24-1a2761c69fbso1229015c88.0 for ; Sun, 08 Nov 2026 21:16:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20260707; t=1794201410; x=1794806210; darn=lists.linux.dev; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to:content-type; bh=F9VadDTyIvxtwTesOFYe162uSd4ddwueP7W1xi3nolk=; b=mFAPGnWCShu/KhRoN2Krk9Lic+PPS02MGy9ApJUDEk5uYJ1X3AAicq/N8pV/vRYivU xur8gT+vqaivsYipQy4Ytep5nSRWlg20GRi1PO9FKJAp1vtq94ff4DnlfmishfOJdtQv yf42loh2d9z+QW+5uK/gRzBvL/iYDWjy4Mb9DGUSjntyCRJPeq790RUT+Qn+Nk0r5Cxa Lqc4MbjIzbVQcR2uvHtNk8bvlkATv+aepE5mTBCvnO6pbJGgAGy0xqlmyqBsHRsqBzpm uKO6JUApm2HcC5enQ88IYrU6qYOZ2z3DswnaQz05rorhz4C08bChX6V1xroFbWCVZp9B 306w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1794201410; x=1794806210; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to:content-type; bh=F9VadDTyIvxtwTesOFYe162uSd4ddwueP7W1xi3nolk=; b=MsZ0okVna3FupkkF29d5cDf1mBmH8Yo8SQ0e494tPCBjQqWD7OXIByXuDW30ML0Zwz I/mOKAkW8T3W0lpFGyYoZtqcdK6FtUJ9YMlwJQgtGBUycn9JnUTX6OoojZ0LAREhSWQZ kBSYalk4qdfXG/G8JYWhWYb7QsBHlT89vSxB68YuhMhYByHkLBD4ORb71k2Rs/+3LeoC gqEwZnOylQsWF7kXpIqAVkCLzKWkA4YpNeJs/ZsCiOlnzTQa9W4NQmriH5Ni/V0lh86I mFlWlsU7ZouFaDiqbj31FM06X0+29Nyh7MUEFZDy4B0Hb+5ZVy7MOkg13WYlwVBq0yLG 4RxA== X-Forwarded-Encrypted: i=1; AL+hErEdqpEVnzZGKPAmOdYChctrrIAEGczHxxNnjEdeGLFfOlUf+h2+X2kzt/aAQ8of4CNguUmqRBAtCToOhEF+XA==@lists.linux.dev X-Gm-Message-State: APuU4Z/alPfTkT/ZBFjuQyW7WAjL+HdTkYLcQ+qAdnlDPSaySv2WLhb9 k0YKF2dnENkrWnxt6+6Q64dXbMG9RI73DOxnftVGu7KFSsp4mSBaDr7J X-Gm-Gg: AYBFou3JEYtIOPxJ/sRkPFEaCnZLrPKUz81ePk012TW0J9OH3GHo5WMjohunL6CxkiL 7fBlFrPcBHMuGTyKeuRgPope4SUqevOR2gQ5S9uXX1yQgRSDhoG9HoGagbonSFi39Ae+kVadEf+ 5pTL4JPWA4UZ+LZAjIar4oLkEatBRzs0z/F6zGel0Q7zqz53lDTS0pznPCsuL4UTSnGtdtuYZX9 E4HzA9MvbQxGo25bdQq/k7hJGbXSIS17mCGudGLYVwyqwtFGlc/Q2wN6OHqlEiNZoOptb+1VI7E TrgvL4YW4OdwmHu0nnic9f0Vn3DqQ4BIkgLyW3wzCmb7hTSHvdxwEQkanIGo7cbK0kZaT9kqHC9 nrw1TAAbqJKQxdj/LbDeFwE5emEbVYexvMbIcYTGR25vfPierazma1i7FDiStivf0asxcd4BSGK z5H1ZLq6Pv83k7SxH/BguavH/p6RZoR/jpsR9d5FGSh9Ntd2EUVrjyNC85sr/bpt9Jni3Z1bJeD Ms9/8UhVqEWZaqGK7j60O2ryrMMYAqTPaz66E6H5yuYWUjK4jsyEdRsdvmRkx5VuLSy/8XnrOfG QHkiqKvAFe6iJhtVz5NDcC3DK/3qS5mDczIDZG2qjpCDL9+h/DTv4ko4Prcc7CUI1pwkDGF1vJP aVbmJojwvkwxB7YMhjzeu7jtUnhkK5fLfim70BbTdRJIxEl0= X-Received: by 2002:a05:7023:d09:b0:185:1640:5536 with SMTP id a92af1059eb24-1b39fe9aa24mr5828037c88.19.1794201409500; Sun, 08 Nov 2026 21:16:49 -0800 (PST) Received: from localhost.localdomain ([183.96.175.225]) by smtp.gmail.com with ESMTPSA id a92af1059eb24-1b37314826asm11395635c88.3.2026.11.08.21.16.46 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Sun, 08 Nov 2026 21:16:48 -0800 (PST) From: Joonho Lee To: linux-sound@vger.kernel.org Cc: tiwai@suse.com, perex@perex.cz, anton.yakovlev@opensynergy.com, mst@redhat.com, virtualization@lists.linux.dev, linux-kernel@vger.kernel.org, corbet@lwn.net, skhan@linuxfoundation.org, rdunlap@infradead.org, linux-doc@vger.kernel.org, Joonho Lee Subject: [PATCH v22 08/13] ALSA: virtio: validate indirect PCM ACK accounting Date: Mon, 9 Nov 2026 14:16:13 +0900 Message-ID: <20261109051618.22506-9-mynameisjune111@gmail.com> X-Mailer: git-send-email 2.54.0 In-Reply-To: <20261109051618.22506-1-mynameisjune111@gmail.com> References: <20261108235457.12688-1-mynameisjune111@gmail.com> <20261109051618.22506-1-mynameisjune111@gmail.com> X-Mailing-List: virtualization@lists.linux.dev List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Validate playback appl_ptr accounting before the indirect transfer helper can mutate its cursor or queue data. Normalize boundary wrap, reject frame deltas outside runtime->min_align before byte conversion, and check frame-to-byte geometry before the helper's signed multiply. Validate ring indices and signed ready counters, then bound hw_ready + sw_ready + new bytes by the PCM buffer capacity. The playback pointer has already deducted device completions from hw_ready when ACK runs. Retain ordinary indirect capture's existing checks against unread completed RX bytes and its pre-START no-data behavior. An invalid paused playback ACK returns -EBADFD without starting a linked XRUN. Assisted-by: LLM Signed-off-by: Joonho Lee --- sound/virtio/virtio_pcm.h | 13 ++ sound/virtio/virtio_pcm_msg.c | 303 +++++++++++++++++++++++++++++++++++------ sound/virtio/virtio_pcm_ops.c | 211 +++++++++++++++++++++++++++-- 3 files changed, 477 insertions(+), 50 deletions(-) diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h --- 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. @@ -29,6 +30,7 @@ * substream operators, including @stopped. * @buffer_bytes: Current buffer size in bytes. * @hw_ptr: Substream hardware pointer value in bytes [0 ... buffer_bytes). + * @capture_rx_seeded: Ordinary capture ring was queued since prepare. * @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 @@ -42,6 +44,10 @@ * @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 { @@ -57,6 +63,7 @@ spinlock_t lock; size_t buffer_bytes; size_t hw_ptr; + bool capture_rx_seeded; bool xfer_enabled; bool xfer_xrun; bool stopped; @@ -68,6 +75,12 @@ 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 --- 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 +#include +#include #include #include "virtio_card.h" @@ -13,6 +16,7 @@ * @xfer: Request header payload. * @status: Response header payload. * @length: Data length in bytes. + * @data_offset: Offset of the fixed PCM ring slice for ordinary I/O. * @sgs: Payload scatter-gather table. */ struct virtio_pcm_msg { @@ -20,6 +24,7 @@ struct virtio_snd_pcm_xfer xfer; struct virtio_snd_pcm_status status; size_t length; + size_t data_offset; struct scatterlist sgs[]; }; @@ -37,9 +42,8 @@ }; /** - * 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,14 +51,18 @@ */ 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) @@ -75,11 +83,80 @@ return num; } -/** - * virtsnd_pcm_sg_from() - Build sg-list from vmalloc'ed buffer. +#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_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 +209,97 @@ 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; + + 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) - 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; - 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)); + msg->data_offset = (size_t)period_bytes * i; 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 +315,13 @@ 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; @@ -199,6 +344,7 @@ guard(spinlock_irqsave)(&vss->lock); for (i = 0; vss->msgs && i < vss->nmsgs; ++i) vss->msgs[i]->length = 0; + vss->capture_rx_seeded = false; } /** @@ -228,8 +374,15 @@ bool notify = false; int rc; + if (!bytes) + return 0; + if (!period_bytes || !vss->msgs || offset >= vss->buffer_bytes || + bytes > vss->buffer_bytes - offset) + return -EINVAL; start = offset / period_bytes; end = (offset + bytes - 1) / period_bytes; + if (end >= vss->nmsgs) + return -EINVAL; for (i = start; i <= end; i++) { struct virtio_pcm_msg *msg = vss->msgs[i]; @@ -248,6 +401,33 @@ if (msg->length == period_bytes) { 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, @@ -356,42 +536,81 @@ size_t written_bytes) { struct virtio_pcm_substream *vss = msg->substream; + bool capture_invalid = false; /* - * hw_ptr always indicates the buffer position of the first I/O message - * in the virtqueue. Therefore, on each completion of an I/O message, - * the hw_ptr value is unconditionally advanced. + * Completed requests can return captured data after STOP while the PCM + * buffer remains prepared. Once preparation ends, completions only retire + * queue ownership. */ guard(spinlock)(&vss->lock); - /* - * If the capture substream returned an incorrect status, then just - * increase the hw_ptr by the message size. - */ - if (vss->direction == SNDRV_PCM_STREAM_PLAYBACK || - written_bytes <= sizeof(msg->status)) + if (vss->direction == SNDRV_PCM_STREAM_CAPTURE && + (!vss->prepared || vss->xfer_xrun || + (vss->stopped && written_bytes <= sizeof(msg->status)))) { + msg->length = 0; + vss->msg_count--; + if (!vss->msg_count) + wake_up_all(&vss->msg_empty); + return; + } + + /* Only the next complete ring slice can extend captured data. */ + if (vss->direction == SNDRV_PCM_STREAM_CAPTURE && + (msg->data_offset != vss->hw_ptr || + written_bytes < sizeof(msg->status) || + le32_to_cpu(msg->status.status) != VIRTIO_SND_S_OK || + written_bytes - sizeof(msg->status) != msg->length)) { + dev_err_ratelimited(&vss->snd->vdev->dev, + "SID %u: invalid capture position, status or length\n", + vss->sid); + vss->xfer_xrun = true; + capture_invalid = true; + } +#ifndef CONFIG_MMU + if (vss->direction == SNDRV_PCM_STREAM_CAPTURE && + vss->prepared && !capture_invalid && + written_bytes > sizeof(msg->status)) { + struct snd_pcm_runtime *runtime = + vss->substream ? vss->substream->runtime : NULL; + + if (!runtime || !runtime->dma_area || !vss->owned_dma_area || + msg->data_offset > runtime->dma_bytes || + msg->length > runtime->dma_bytes - msg->data_offset || + msg->data_offset > vss->owned_dma_bytes || + msg->length > 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); + vss->xfer_xrun = true; + capture_invalid = true; + } else { + memcpy(runtime->dma_area + msg->data_offset, + vss->owned_dma_area + msg->data_offset, msg->length); + } + } +#endif + /* Publish pointer progress only for a complete TX or valid RX request. */ + if (!capture_invalid) { vss->hw_ptr += msg->length; - else - vss->hw_ptr += written_bytes - sizeof(msg->status); - - if (vss->hw_ptr >= vss->buffer_bytes) - vss->hw_ptr -= vss->buffer_bytes; + if (vss->hw_ptr >= vss->buffer_bytes) + vss->hw_ptr -= vss->buffer_bytes; + } msg->length = 0; - - vss->xfer_xrun = false; vss->msg_count--; if (vss->xfer_enabled) { - struct snd_pcm_runtime *runtime = vss->substream->runtime; - - runtime->delay = - bytes_to_frames(runtime, - le32_to_cpu(msg->status.latency_bytes)); - + if (!capture_invalid) { + struct snd_pcm_runtime *runtime = vss->substream->runtime; + + runtime->delay = + bytes_to_frames(runtime, + le32_to_cpu(msg->status.latency_bytes)); + } schedule_work(&vss->elapsed_period); - } else if (!vss->msg_count) { + } + 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 --- 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: * @@ -260,10 +262,21 @@ struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream); struct virtio_device *vdev = vss->snd->vdev; unsigned int msg_count; + size_t period_buffer_bytes; bool stopped; int rc; virtsnd_pcm_close_ack(substream, false); + + /* Fixed descriptors require whole-byte periods covering the buffer. */ + if (check_mul_overflow((size_t)params_periods(hw_params), + (size_t)params_period_bytes(hw_params), + &period_buffer_bytes) || + period_buffer_bytes != params_buffer_bytes(hw_params) || + (params_period_size(hw_params) * + (u64)snd_pcm_format_physical_width(params_format(hw_params)) * + params_channels(hw_params)) % 8) + return -EINVAL; scoped_guard(spinlock_irqsave, &vss->lock) { msg_count = vss->msg_count; @@ -440,8 +453,15 @@ substream->runtime->stop_operating = true; return -EBADFD; } - if (vss->direction == SNDRV_PCM_STREAM_CAPTURE) + if (vss->direction == SNDRV_PCM_STREAM_CAPTURE && + !vss->capture_rx_seeded) { rc = virtsnd_pcm_msg_send(vss, 0, vss->buffer_bytes); + if (!rc) { + /* The full RX ring is reserved until userspace reads it. */ + vss->pcm_indirect.sw_ready = vss->buffer_bytes; + vss->capture_rx_seeded = true; + } + } if (rc) { vss->stopped = true; substream->runtime->stop_operating = true; @@ -606,6 +626,163 @@ int error; }; +/* Caller holds @vss->lock and the PCM stream lock. */ +static int virtsnd_pcm_playback_ack_validate(struct virtio_pcm_substream *vss, + struct snd_pcm_runtime *runtime, + const struct snd_pcm_indirect *rec) +{ + snd_pcm_uframes_t appl_ptr; + snd_pcm_uframes_t appl_diff; + snd_pcm_uframes_t signed_max; + snd_pcm_uframes_t back; + u64 buffer_bits; + u64 appl_bits; + size_t appl_bytes; + s64 queued_bytes; + s64 sw_ready_total; + s64 total_bytes; + s64 int_max; + unsigned int hw_queue_size; + + if (!runtime->control || !runtime->boundary || + runtime->control->appl_ptr >= runtime->boundary || + rec->appl_ptr >= runtime->boundary) + return -EPIPE; + + /* Keep the signed frame conversion used by frames_to_bytes() in range. */ + signed_max = (~(snd_pcm_uframes_t)0) >> 1; + if (runtime->boundary > signed_max || + runtime->buffer_size > signed_max || !runtime->frame_bits || + check_mul_overflow((u64)runtime->buffer_size, + (u64)runtime->frame_bits, &buffer_bits) || + buffer_bits > signed_max || buffer_bits % 8 || + buffer_bits / 8 != vss->buffer_bytes) + return -EPIPE; + + /* Match the indirect helper's appl_ptr boundary-wrap calculation. */ + appl_ptr = runtime->control->appl_ptr; + if (appl_ptr >= rec->appl_ptr) { + appl_diff = appl_ptr - rec->appl_ptr; + } else { + back = rec->appl_ptr - appl_ptr; + if (back <= runtime->boundary / 2) + return -EPIPE; + appl_diff = runtime->boundary - back; + } + + if (appl_diff > runtime->buffer_size) + return -EPIPE; + /* The indirect helper advances appl_ptr even if frames_to_bytes truncates. */ + if (!runtime->min_align || appl_diff % runtime->min_align) + return -EBADFD; + if (check_mul_overflow((u64)appl_diff, + (u64)runtime->frame_bits, &appl_bits) || + appl_bits % 8) + return -EPIPE; + + if (!rec->hw_buffer_size || !rec->sw_buffer_size || + rec->hw_buffer_size != vss->buffer_bytes || + rec->sw_buffer_size != vss->buffer_bytes || rec->hw_ready < 0 || + rec->sw_ready < 0 || rec->hw_data >= rec->hw_buffer_size || + rec->hw_io >= rec->hw_buffer_size || + rec->sw_data >= rec->sw_buffer_size || + rec->sw_io >= rec->sw_buffer_size || + (unsigned int)rec->sw_ready > rec->sw_buffer_size) + return -EPIPE; + + hw_queue_size = rec->hw_queue_size ? rec->hw_queue_size : + rec->hw_buffer_size; + if (!hw_queue_size || hw_queue_size > rec->hw_buffer_size || + (unsigned int)rec->hw_ready > hw_queue_size) + return -EPIPE; + + int_max = (int)(~0U >> 1); + if (hw_queue_size > int_max || appl_bits / 8 > int_max) + return -EPIPE; + appl_bytes = frames_to_bytes(runtime, (snd_pcm_sframes_t)appl_diff); + if (appl_bytes != (size_t)(appl_bits / 8) || + check_add_overflow((s64)rec->sw_ready, (s64)appl_bytes, + &sw_ready_total) || + sw_ready_total > int_max) + return -EPIPE; + + /* hw_ready already excludes device completions accounted by the pointer. */ + if (check_add_overflow((s64)rec->hw_ready, (s64)rec->sw_ready, + &queued_bytes) || + check_add_overflow(queued_bytes, (s64)appl_bytes, &total_bytes) || + total_bytes > (s64)vss->buffer_bytes) + return -EPIPE; + + return 0; +} + +/* Caller holds @vss->lock and the PCM stream lock. */ +static int virtsnd_pcm_capture_ack_validate(struct virtio_pcm_substream *vss, + struct snd_pcm_runtime *runtime, + bool *skip_transfer) +{ + const struct snd_pcm_indirect *rec = &vss->pcm_indirect; + snd_pcm_sframes_t appl_diff; + s64 unread_bytes; + size_t appl_bytes; + unsigned int hw_queue_size; + + *skip_transfer = false; + if (!runtime->control || !runtime->boundary || + runtime->control->appl_ptr >= runtime->boundary || + rec->appl_ptr >= runtime->boundary) + return -EPIPE; + + /* Match the indirect helper's appl_ptr boundary-wrap calculation. */ + appl_diff = runtime->control->appl_ptr - rec->appl_ptr; + if (appl_diff < -(snd_pcm_sframes_t)(runtime->boundary / 2)) + appl_diff += runtime->boundary; + + /* Before START there is no RX data to consume or requeue. */ + if (!vss->capture_rx_seeded) { + if (appl_diff) + return -EBADFD; + *skip_transfer = true; + return 0; + } + + if (appl_diff < 0 || (u64)appl_diff > runtime->buffer_size) + return -EPIPE; + /* The indirect helper advances appl_ptr even if frames_to_bytes truncates. */ + if (!runtime->min_align || + (snd_pcm_uframes_t)appl_diff % runtime->min_align) + return -EBADFD; + + if (!rec->hw_buffer_size || !rec->sw_buffer_size || + rec->hw_buffer_size != vss->buffer_bytes || + rec->sw_buffer_size != vss->buffer_bytes || rec->hw_ready < 0 || + rec->sw_ready < 0 || rec->hw_data >= rec->hw_buffer_size || + rec->hw_io >= rec->hw_buffer_size || + rec->sw_data >= rec->sw_buffer_size || + rec->sw_io >= rec->sw_buffer_size || + (unsigned int)rec->sw_ready > rec->sw_buffer_size) + return -EPIPE; + + hw_queue_size = rec->hw_queue_size ? rec->hw_queue_size : + rec->hw_buffer_size; + if (!hw_queue_size || hw_queue_size > rec->hw_buffer_size || + (unsigned int)rec->hw_ready > hw_queue_size) + return -EPIPE; + + /* The RX ring reserves its full size; only the excess is unread data. */ + unread_bytes = (s64)rec->hw_ready + (s64)rec->sw_ready - + (s64)rec->sw_buffer_size; + if (unread_bytes < 0 || unread_bytes > (s64)hw_queue_size) + return -EPIPE; + + appl_bytes = frames_to_bytes(runtime, (snd_pcm_uframes_t)appl_diff); + if ((u64)appl_bytes > (u64)unread_bytes) + return -EPIPE; + + return 0; +} + + static void virtsnd_pcm_trans_copy(struct snd_pcm_substream *substream, struct snd_pcm_indirect *rec, size_t bytes) { @@ -668,7 +845,15 @@ rc = -EPIPE; } else { transfer.indirect = vss->pcm_indirect; - rc = snd_pcm_indirect_playback_transfer(substream, + rc = virtsnd_pcm_playback_ack_validate(vss, + substream->runtime, + &transfer.indirect); + if (rc == -EBADFD || + (rc && substream->runtime->state == + SNDRV_PCM_STATE_PAUSED)) + return -EBADFD; + if (!rc) + rc = snd_pcm_indirect_playback_transfer(substream, &transfer.indirect, virtsnd_pcm_trans_copy); if (!rc) @@ -694,6 +879,7 @@ struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream); struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss); struct virtsnd_pcm_transfer transfer = {}; + bool skip_transfer = false; int rc; rc = virtsnd_pcm_ack_state(substream); @@ -712,13 +898,22 @@ if (vss->xfer_xrun) { rc = -EPIPE; } else { - transfer.indirect = vss->pcm_indirect; - rc = snd_pcm_indirect_capture_transfer(substream, - &transfer.indirect, - virtsnd_pcm_trans_copy); - if (!rc) - rc = transfer.error; + rc = virtsnd_pcm_capture_ack_validate(vss, + substream->runtime, + &skip_transfer); + if (rc == -EBADFD) + return rc; + if (!rc && skip_transfer) + return 0; + if (!rc) { + transfer.indirect = vss->pcm_indirect; + 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