git: de5fd56985c3 - releng/14.3 - sound: Fix software buffer lifetime issues
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 09 Jun 2026 19:18:37 UTC
The branch releng/14.3 has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=de5fd56985c380ec617a10e480a27eb192b1b074
commit de5fd56985c380ec617a10e480a27eb192b1b074
Author: Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2026-06-01 21:57:40 +0000
Commit: Mark Johnston <markj@FreeBSD.org>
CommitDate: 2026-06-07 17:48:04 +0000
sound: Fix software buffer lifetime issues
The channel buffer mapped by dsp_mmap_single() may be freed when the
device handle is closed, but the mapping persists beyond that, allowing
userspace to read or write memory owned by a different consumer.
Fix the problem by adding a reference counter to the sound buffer.
Define pager ops for the VM object returned by dsp_mmap_single() and use
them to manage the extra reference.
Add a regression test.
Approved by: so
Security: FreeBSD-SA-26:27.sound
Security: CVE-2026-49417
Reported by: Lexpl0it, 75Acol, Liyw979, Rob1n
Reviewed by kib
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D57393
---
sys/dev/sound/pcm/buffer.c | 38 ++++++++++++++++++++--
sys/dev/sound/pcm/buffer.h | 4 +++
sys/dev/sound/pcm/dsp.c | 80 ++++++++++++++++++++++++++++++++++++++--------
tests/sys/sound/mmap.c | 59 ++++++++++++++++++++++++++++++++++
4 files changed, 165 insertions(+), 16 deletions(-)
diff --git a/sys/dev/sound/pcm/buffer.c b/sys/dev/sound/pcm/buffer.c
index de535ec2dcba..ddcf14fbc1c2 100644
--- a/sys/dev/sound/pcm/buffer.c
+++ b/sys/dev/sound/pcm/buffer.c
@@ -32,6 +32,7 @@
#include "opt_snd.h"
#endif
+#include <sys/refcount.h>
#include <dev/sound/pcm/sound.h>
#include "feeder_if.h"
@@ -46,6 +47,7 @@ sndbuf_create(device_t dev, char *drv, char *desc, struct pcm_channel *channel)
struct snd_dbuf *b;
b = malloc(sizeof(*b), M_DEVBUF, M_WAITOK | M_ZERO);
+ refcount_init(&b->refcount, 1);
snprintf(b->name, SNDBUF_NAMELEN, "%s:%s", drv, desc);
b->dev = dev;
b->channel = channel;
@@ -56,8 +58,30 @@ sndbuf_create(device_t dev, char *drv, char *desc, struct pcm_channel *channel)
void
sndbuf_destroy(struct snd_dbuf *b)
{
- sndbuf_free(b);
- free(b, M_DEVBUF);
+ b->flags |= SNDBUF_F_DETACHED;
+ sndbuf_rele(b);
+}
+
+void
+sndbuf_ref(struct snd_dbuf *b)
+{
+ unsigned int count __diagused;
+
+ CHN_LOCK(b->channel);
+ count = refcount_acquire(&b->refcount);
+ KASSERT(count > 0, ("sndbuf %p refcount 0", b));
+ CHN_UNLOCK(b->channel);
+}
+
+void
+sndbuf_rele(struct snd_dbuf *b)
+{
+ if (refcount_release(&b->refcount)) {
+ sndbuf_free(b);
+ KASSERT(refcount_load(&b->refcount) == 0,
+ ("sndbuf %p still referenced", b));
+ free(b, M_DEVBUF);
+ }
}
bus_addr_t
@@ -183,6 +207,11 @@ sndbuf_resize(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz)
if (bufsize > b->allocsize ||
bufsize < (b->allocsize >> SNDBUF_CACHE_SHIFT)) {
+ if (refcount_load(&b->refcount) > 1 ||
+ (b->flags & SNDBUF_F_DETACHED) != 0) {
+ CHN_UNLOCK(b->channel);
+ return (EBUSY);
+ }
allocsize = round_page(bufsize);
CHN_UNLOCK(b->channel);
tmpbuf = malloc(allocsize, M_DEVBUF, M_WAITOK);
@@ -218,10 +247,15 @@ sndbuf_remalloc(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz)
if (blkcnt < 2 || blksz < 16)
return EINVAL;
+ CHN_LOCKASSERT(b->channel);
+
bufsize = blksz * blkcnt;
if (bufsize > b->allocsize ||
bufsize < (b->allocsize >> SNDBUF_CACHE_SHIFT)) {
+ if (refcount_load(&b->refcount) > 1 ||
+ (b->flags & SNDBUF_F_DETACHED) != 0)
+ return (EBUSY);
allocsize = round_page(bufsize);
CHN_UNLOCK(b->channel);
buf = malloc(allocsize, M_DEVBUF, M_WAITOK);
diff --git a/sys/dev/sound/pcm/buffer.h b/sys/dev/sound/pcm/buffer.h
index ddf4083ec19f..99bc6c0611d2 100644
--- a/sys/dev/sound/pcm/buffer.h
+++ b/sys/dev/sound/pcm/buffer.h
@@ -27,6 +27,7 @@
*/
#define SNDBUF_F_MANAGED 0x00000008
+#define SNDBUF_F_DETACHED 0x00000010
#define SNDBUF_NAMELEN 48
@@ -50,6 +51,7 @@ struct snd_dbuf {
bus_dma_tag_t dmatag;
bus_addr_t buf_addr;
int dmaflags;
+ unsigned int refcount;
struct selinfo sel;
struct pcm_channel *channel;
char name[SNDBUF_NAMELEN];
@@ -57,6 +59,8 @@ struct snd_dbuf {
struct snd_dbuf *sndbuf_create(device_t dev, char *drv, char *desc, struct pcm_channel *channel);
void sndbuf_destroy(struct snd_dbuf *b);
+void sndbuf_ref(struct snd_dbuf *b);
+void sndbuf_rele(struct snd_dbuf *b);
void sndbuf_dump(struct snd_dbuf *b, char *s, u_int32_t what);
diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c
index f2254f925940..21d1407ac882 100644
--- a/sys/dev/sound/pcm/dsp.c
+++ b/sys/dev/sound/pcm/dsp.c
@@ -74,7 +74,6 @@ static d_read_t dsp_read;
static d_write_t dsp_write;
static d_ioctl_t dsp_ioctl;
static d_poll_t dsp_poll;
-static d_mmap_t dsp_mmap;
static d_mmap_single_t dsp_mmap_single;
struct cdevsw dsp_cdevsw = {
@@ -84,7 +83,6 @@ struct cdevsw dsp_cdevsw = {
.d_write = dsp_write,
.d_ioctl = dsp_ioctl,
.d_poll = dsp_poll,
- .d_mmap = dsp_mmap,
.d_mmap_single = dsp_mmap_single,
.d_name = "dsp",
};
@@ -1821,23 +1819,72 @@ dsp_poll(struct cdev *i_dev, int events, struct thread *td)
return (ret);
}
+struct dsp_mmap_handle {
+ struct cdev *cdev;
+ struct snd_dbuf *buf;
+};
+
static int
-dsp_mmap(struct cdev *i_dev, vm_ooffset_t offset, vm_paddr_t *paddr,
- int nprot, vm_memattr_t *memattr)
+dsp_dev_pager_ctor(void *handle, vm_ooffset_t size, vm_prot_t prot,
+ vm_ooffset_t foff, struct ucred *cred, u_short *color)
{
+ struct dsp_mmap_handle *h = handle;
- /*
- * offset is in range due to checks in dsp_mmap_single().
- * XXX memattr is not honored.
- */
- *paddr = vtophys(offset);
+ dev_ref(h->cdev);
+ sndbuf_ref(h->buf);
return (0);
}
+static void
+dsp_dev_pager_dtor(void *handle)
+{
+ struct dsp_mmap_handle *h = handle;
+
+ sndbuf_rele(h->buf);
+ dev_rel(h->cdev);
+ free(h, M_DEVBUF);
+}
+
static int
-dsp_mmap_single(struct cdev *i_dev, vm_ooffset_t *offset,
+dsp_dev_pager_fault(vm_object_t object, vm_ooffset_t offset, int prot,
+ vm_page_t *mres)
+{
+ struct dsp_mmap_handle *h = object->handle;
+ vm_page_t m;
+ uintptr_t addr;
+ vm_paddr_t paddr;
+
+ addr = (uintptr_t)offset;
+ if (addr < (uintptr_t)h->buf->buf ||
+ addr >= (uintptr_t)h->buf->buf + h->buf->allocsize)
+ return (VM_PAGER_ERROR);
+ paddr = vtophys((void *)addr);
+
+ if (((*mres)->flags & PG_FICTITIOUS) != 0) {
+ m = *mres;
+ vm_page_updatefake(m, paddr, object->memattr);
+ } else {
+ VM_OBJECT_WUNLOCK(object);
+ m = vm_page_getfake(paddr, object->memattr);
+ VM_OBJECT_WLOCK(object);
+ vm_page_replace(m, object, (*mres)->pindex, *mres);
+ *mres = m;
+ }
+ m->valid = VM_PAGE_BITS_ALL;
+ return (VM_PAGER_OK);
+}
+
+static const struct cdev_pager_ops dsp_dev_pager_ops = {
+ .cdev_pg_ctor = dsp_dev_pager_ctor,
+ .cdev_pg_dtor = dsp_dev_pager_dtor,
+ .cdev_pg_fault = dsp_dev_pager_fault,
+};
+
+static int
+dsp_mmap_single(struct cdev *cdev, vm_ooffset_t *offset,
vm_size_t size, struct vm_object **object, int nprot)
{
+ struct dsp_mmap_handle *handle;
struct dsp_cdevpriv *priv;
struct snddev_info *d;
struct pcm_channel *wrch, *rdch, *c;
@@ -1900,13 +1947,18 @@ dsp_mmap_single(struct cdev *i_dev, vm_ooffset_t *offset,
*offset = (uintptr_t)sndbuf_getbufofs(c->bufsoft, *offset);
dsp_unlock_chans(priv, SD_F_PRIO_RD | SD_F_PRIO_WR);
- *object = vm_pager_allocate(OBJT_DEVICE, i_dev,
- size, nprot, *offset, curthread->td_ucred);
+ handle = malloc(sizeof(*handle), M_DEVBUF, M_WAITOK);
+ handle->cdev = cdev;
+ handle->buf = c->bufsoft;
+ *object = cdev_pager_allocate(handle, OBJT_DEVICE, &dsp_dev_pager_ops,
+ size, nprot, *offset, curthread->td_ucred);
PCM_GIANT_LEAVE(d);
+ if (*object == NULL) {
+ free(handle, M_DEVBUF);
+ return (EINVAL);
+ }
- if (*object == NULL)
- return (EINVAL);
return (0);
}
diff --git a/tests/sys/sound/mmap.c b/tests/sys/sound/mmap.c
index 33440529eb10..ee35986831c2 100644
--- a/tests/sys/sound/mmap.c
+++ b/tests/sys/sound/mmap.c
@@ -4,12 +4,14 @@
* Copyright (c) 2026 The FreeBSD Foundation
*/
+#include <sys/param.h>
#include <sys/mman.h>
#include <sys/soundcard.h>
#include <atf-c.h>
#include <errno.h>
#include <fcntl.h>
+#include <string.h>
#include <unistd.h>
#define FMT_ERR(s) s ": %s", strerror(errno)
@@ -42,9 +44,66 @@ ATF_TC_BODY(mmap_offset_overflow, tc)
close(fd);
}
+/*
+ * Verify that a MAP_SHARED mapping of a DSP device's software buffer remains
+ * valid after the file descriptor is closed.
+ */
+ATF_TC(mmap_buffer_lifetime);
+ATF_TC_HEAD(mmap_buffer_lifetime, tc)
+{
+ atf_tc_set_md_var(tc, "descr", "mmap data survives close()");
+}
+ATF_TC_BODY(mmap_buffer_lifetime, tc)
+{
+ audio_buf_info abi;
+ uint8_t *buf;
+ size_t len;
+ int fd, arg;
+
+ fd = open("/dev/dsp0", O_RDWR);
+ ATF_REQUIRE_MSG(fd >= 0, FMT_ERR("open"));
+
+ arg = (2 << 16) | 14; /* 2*16KB */
+ ATF_REQUIRE_MSG(ioctl(fd, SNDCTL_DSP_SETFRAGMENT, &arg) == 0,
+ FMT_ERR("SNDCTL_DSP_SETFRAGMENT"));
+ ATF_REQUIRE_MSG(ioctl(fd, SNDCTL_DSP_GETOSPACE, &abi) == 0,
+ FMT_ERR("SNDCTL_DSP_GETOSPACE"));
+
+ len = abi.bytes;
+ ATF_REQUIRE_MSG(len >= PAGE_SIZE, "buffer too small: %zu", len);
+
+ buf = mmap(NULL, len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+ ATF_REQUIRE_MSG(buf != MAP_FAILED, FMT_ERR("mmap"));
+
+ for (size_t i = 0; i < len; i++) {
+ ATF_REQUIRE_MSG(buf[i] == 0,
+ "mmap data corrupted at offset %zu: want 0 got 0x%02x",
+ i, buf[i]);
+ }
+
+ memset(buf, 0xa5, len);
+ for (size_t i = 0; i < len; i++) {
+ ATF_REQUIRE_MSG(buf[i] == 0xa5,
+ "mmap data corrupted at offset %zu: want 0xa5 got 0x%02x",
+ i, buf[i]);
+ }
+
+ ATF_REQUIRE(close(fd) == 0);
+
+ /* Closing the device causes the buffer to be reset. */
+ for (size_t i = 0; i < len; i++) {
+ ATF_REQUIRE_MSG(buf[i] == 0 || buf[i] == 0xa5,
+ "mmap data corrupted at offset %zu: got 0x%02x", i, buf[i]);
+ }
+ memset(buf, 0xa5, len);
+
+ ATF_REQUIRE(munmap(buf, len) == 0);
+}
+
ATF_TP_ADD_TCS(tp)
{
ATF_TP_ADD_TC(tp, mmap_offset_overflow);
+ ATF_TP_ADD_TC(tp, mmap_buffer_lifetime);
return (atf_no_error());
}