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=GSky4iMT9prYVzYV3oq883QY6JqOtHXZFN0bQUtUObEGE72zTh8AkgdMncGRaMAt4Cfk 9tTz5LTxa60kTMYrfDIt/csE205oDi+L0rAJjUr3D6XTZS9vmgjHph6tm6CiW0Zv9mNGl 1ts5kr7O/QXckprdCjP2H828N5QDtm7DsLFtJrRovWWNWDAuuR5JJ4GmRcnvub4xO/dra Jtp35s3nXm3DVL10gV7F6dQvIwfbGjgzXoln52wXiq4PIabUJNtGm3zSFdWscPnsu/vAM HLmIVzG2ZAXMdpXk/16QWNHQcV0gy/S6vGwZr5Cmo6TpktzBTurg6M7Psud0l0c7d1w== 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=l7pG4I9HWZxgzyNL+lIFZytdgZqlrpZ01vJ/gPbnvkX4N05RgAuIbwxHqLT8y0toceQu Xqrmw3wZbEYxt4OAeOoD+0fnGEcccHEl6H8oD1PGubkmtE8UFLJ43uvCclwuAi09ZjIcp 4hn0fBEuHxCwBxnmBhc2hjJiloo1yG2zlKMV+NuwahY5rIMKyIkgCS8EbdkzpTcNrUgjV UqEdkcS/Tr5ZqHhL4msjVgkMXC3yhnB2twdP2pJeCa0C+RysGjl0tktebuSm8OBKQ9YLB y6YVUTfoYvxYhtPQmi3Vzxqqtjk5kn2uznC7A6OO3voz+PeZC8mM8EMZPZSCdBctmWA== 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=oaYfv+sw; 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 7836A606A323 for ; Mon, 9 Nov 2026 05:16:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.2 smtp.subspace.kernel.org 7836A606A323 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=oaYfv+sw Received: by mail-dl1-f47.google.com with SMTP id a92af1059eb24-15dd124bbe7so941802c88.1 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=vger.kernel.org; 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=oaYfv+swqPaadOncwnqQ+H2S+OysxrMXZIbWB9lVAk+glsculLkdhaEiS91Ilc2DgE Aw+FyVrkRLPsQVUjOl2o66VxtHamCflDbdw0Sxg49oXLdNFkdEJo13vdvqPGE+cU0gX2 SktEuLOg/+Qv7/6rMda7uFlyMoipkiy7mvu3KKwrmmMa/UxXpuNCq6YbkLQuTFBfkdUI +K27yb2IJq5uxsh3BI2dvTFbB/WMxoqaXwRnM9iGqMaeGzoo8HjMz9uBGvgH/Rd79RU8 j5DCUHxELKX0olSRLNksui1OEVIQxjqyj0etto6y3fuC9fniODF75NcY+rKNuwyIm3dS tiWw== 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=OibeoQjiDzVAEMuS4CU+0/cQacf8v1tAzC3jv5t67xabe9yRJVfQTc3rnNeTyw+lRZ Iri0xFY5dl9IPouTqQJkAII+eNiTDQ/OEtQHLFtQ5oYfbCwNHHaRL+rrbnt0aNvdY8zW cO2DtwjEJRPYazMnJkJggCUGLE5HFy6b5NEPXrGcjVVBmj5tE35ntQbBEZlX/+/gPIgT rLYbGtttnReHT65hcb323gfPiWHTLdcPzm67Rio1RuQAAcHpQK5GKScBqncjtCzUVt2w RChXS2QcTaz7iTReMhgUjURNfq9JmcGjRje26iWTcQdBz+NQlY34ZzFIHWvXJN4vMW08 W8Tg== X-Forwarded-Encrypted: i=1; AL+hErERy7XKGJas6BSDhqTnrgtLPB+owqOERuCN6fzwc4GK+VkVPZhAaK1IkSwg4j9FSSHo9vsQwMZ02HEPQ9A=@vger.kernel.org X-Gm-Message-State: APuU4Z/WvepT/t5xXt5pixMEOZOtgengpHKyfXJAVry9C0MNa6m8FmZk 8LVRHiz3vHGObWtjCR7JtBUxPFBjlxtorcCsKBMbIYli6M0atmHHS7yr X-Gm-Gg: AYBFou0u7UEK2aO2ul6wFNmVuotsoZjYK7G3Cc9fjem6VoMysTAYqS0AF3tIPP225CC 4TWyUu7XRfvunoc2ETToofH98ChrFKugrlH21h3orKzQH2ypBHlHQ82khNmDTviYOtWmszzPzh1 IKqkhhbd1nb09PlIUv7DjfnS63fHFII2gZlUth/yFBOE/Dkv6MWIRP6QiDNUHCLRhMHxkhU8TGf QVp5TrhUYyh4Y+tIJxEbs4uHU4Jzun0K9mZE8ka12leUFB6AOxVdGrRRGWS4+L38TVp9blO+QBe JxcPTuUXhNKRDVzvqYzk78F89t2Y1Vkl/wf2ZaZ/maJmGOE2GJdYeuj4dRzNE6FFK+cPu1DIdKK oRdeQdQHRtNgqtEoZcd/4uo/rKqJQBs9J9wmU+ktxlDuWZCkggReO3nyQt4oyuuCJYGuJMX238k C//8170SkOosrx6P8b3MIajYN1Y+kEKpuT1VgAOqF9JXfQiplsgEIrKuTz/Aj/M7uiMP6gIaeCf j+3PtFm1wvGkBVFivK+IMSGC+xpb+9tiJlnmK5LRXmAvkoccQ769kml7QIIUUVi+HnT035/IBZa X9A9QPKPxVPDtFldumc5Z60Nn/f9yTskxvr7MWc68tW4h31VtPJng6BnsKDHhhAo+Q+DjgICuMJ d4zyh6YJ76/4xKy2RYDJSdRVM9TcB7KEoHu/Nv1ckka2SoSw= 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: linux-kernel@vger.kernel.org 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