zephyr: sof_dma: validate syscalls against kernel-only DMA state

src/lib/dma.c

@@ -17,6 +17,7 @@
 #include <sof/lib/dma.h>
 #include <sof/lib/memory.h>
 #include <sof/lib/uuid.h>
+#include <sof/schedule/ll_schedule_domain.h> /* zephyr_ll_user_heap_verify() */
 #include <rtos/spinlock.h>
 #include <sof/trace/trace.h>
 #include <ipc/topology.h>
@@ -37,6 +38,38 @@ DECLARE_TR_CTX(dma_tr, SOF_UUID(dma_uuid), LOG_LEVEL_INFO);
 #if CONFIG_ZEPHYR_NATIVE_DRIVERS
 static int dma_init(struct sof_dma *dma);
 
+#ifdef CONFIG_SOF_USERSPACE_INTERFACE_DMA
+/*
+ * dma[] is writable by user threads, so the reference count, the lock and
+ * the channel array to free are kept in the kernel-only struct
+ * sof_dma_kinfo. dma->sref is still updated for diagnostics, but never
+ * trusted.
+ */
+static int *dma_sref(struct sof_dma *dma)
+{
+	return &sof_dma_kinfo_get(dma)->sref;
+}
+
+/*
+ * dma->lock is user-writable too: a user thread could release it while the
+ * kernel holds it, letting two cores update the reference count at once.
+ */
+static struct k_spinlock *dma_lock(struct sof_dma *dma)
+{
+	return &sof_dma_kinfo_get(dma)->lock;
+}
+#else
+static int *dma_sref(struct sof_dma *dma)
+{
+	return &dma->sref;
+}
+
+static struct k_spinlock *dma_lock(struct sof_dma *dma)
+{
+	return &dma->lock;
+}
+#endif
+
 __cold struct sof_dma *z_impl_sof_dma_get(uint32_t dir, uint32_t cap, uint32_t dev, uint32_t flags)
 {
 	const struct dma_info *info = dma_info_get();
@@ -111,9 +144,9 @@ __cold struct sof_dma *z_impl_sof_dma_get(uint32_t dir, uint32_t cap, uint32_t d
 	 * may be requested many times, let the probe()
 	 * do on-first-use initialization.
 	 */
-	key = k_spin_lock(&dmin->lock);
+	key = k_spin_lock(dma_lock(dmin));
 
-	if (!dmin->sref) {
+	if (!*dma_sref(dmin)) {
 		ret = dma_init(dmin);
 		if (ret < 0) {
 			tr_err(&dma_tr, "dma_get(): dma-probe failed id = %d, ret = %d",
@@ -122,13 +155,15 @@ __cold struct sof_dma *z_impl_sof_dma_get(uint32_t dir, uint32_t cap, uint32_t d
 		}
 	}
 
-	dmin->sref++;
+	++*dma_sref(dmin);
+	/* user-visible copy for diagnostics and DMAC selection, never trusted */
+	dmin->sref = *dma_sref(dmin);
 
 	tr_info(&dma_tr, "dma_get() ID %d sref = %d busy channels %ld",
 		dmin->plat_data.id, dmin->sref,
 		atomic_read(&dmin->num_channels_busy));
 out:
-	k_spin_unlock(&dmin->lock, key);
+	k_spin_unlock(dma_lock(dmin), key);
 	return !ret ? dmin : NULL;
 }
 
@@ -138,43 +173,84 @@ __cold void z_impl_sof_dma_put(struct sof_dma *dma)
 
 	assert_can_be_cold();
 
-	key = k_spin_lock(&dma->lock);
-	if (--dma->sref == 0) {
+	key = k_spin_lock(dma_lock(dma));
+	if (*dma_sref(dma) <= 0) {
+		tr_err(&dma_tr, "dma_put(), dma = %p not in use", dma);
+		k_spin_unlock(dma_lock(dma), key);
+		return;
+	}
+
+	--*dma_sref(dma);
+	dma->sref = *dma_sref(dma);
+	if (!*dma_sref(dma)) {
+#ifdef CONFIG_SOF_USERSPACE_INTERFACE_DMA
+		struct sof_dma_kinfo *kinfo = sof_dma_kinfo_get(dma);
+
+		sof_heap_free(kinfo->heap, kinfo->chan);
+		kinfo->chan = NULL;
+#else
 		sof_heap_free(dma->heap, dma->chan);
+#endif
 		dma->chan = NULL;
 	}
 
 	tr_info(&dma_tr, "dma_put(), dma = %p, sref = %d",
 		dma, dma->sref);
-	k_spin_unlock(&dma->lock, key);
+	k_spin_unlock(dma_lock(dma), key);
 }
 
 __cold static int dma_init(struct sof_dma *dma)
 {
-	struct dma_chan_data *chan;
+	struct dma_chan_data *chans, *chan;
 	struct k_heap *heap = sof_sys_user_heap_get();
-	int i;
+#ifdef CONFIG_SOF_USERSPACE_INTERFACE_DMA
+	/* dma->plat_data is user-writable, use the kernel snapshot */
+	const uint32_t channels = sof_dma_kinfo_get(dma)->channels;
+#else
+	const uint32_t channels = dma->plat_data.channels;
+#endif
+	uint32_t i;
 
 	assert_can_be_cold();
 
-	/* allocate dma channels */
-	dma->chan = sof_heap_alloc(heap, SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT,
-				   sizeof(struct dma_chan_data) * dma->plat_data.channels, 0);
-	if (!dma->chan) {
+#if CONFIG_SOF_USERSPACE_INTERFACE_DMA && CONFIG_SOF_USERSPACE_LL
+	/*
+	 * Called in supervisor mode from the sof_dma_get() system call. The
+	 * heap pointer comes from user-writable memory, so check that it is
+	 * the LL user heap before running the allocator on it.
+	 */
+	if (!zephyr_ll_user_heap_verify(heap)) {
+		tr_err(&dma_tr, "dma %d: invalid channel heap %p", dma->plat_data.id, heap);
+		return -EINVAL;
+	}
+#endif
+
+	/*
+	 * allocate dma channels, use a local pointer until the array is
+	 * initialized: dma->chan is user-writable and must not be read back
+	 */
+	chans = sof_heap_alloc(heap, SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT,
+			       sizeof(struct dma_chan_data) * channels, 0);
+	if (!chans) {
 		tr_err(&dma_tr, "dma %d allocation of channels failed",
 		       dma->plat_data.id);
 		return -ENOMEM;
 	}
 
-	dma->heap = heap;
-	memset(dma->chan, 0, sizeof(struct dma_chan_data) * dma->plat_data.channels);
+	memset(chans, 0, sizeof(struct dma_chan_data) * channels);
 	/* init work */
-	for (i = 0, chan = dma->chan; i < dma->plat_data.channels;
-	     i++, chan++) {
+	for (i = 0, chan = chans; i < channels; i++, chan++) {
 		chan->dma = dma;
 		chan->index = i;
 	}
 
+#ifdef CONFIG_SOF_USERSPACE_INTERFACE_DMA
+	sof_dma_kinfo_get(dma)->heap = heap;
+	sof_dma_kinfo_get(dma)->chan = chans;
+#endif
+	dma->heap = heap;
+	dma->chan = chans;
+
 	return 0;
 }
 #else

zephyr/include/sof/lib/sof_dma.h

@@ -111,6 +111,42 @@ static inline int z_impl_sof_dma_resume(struct sof_dma *dma, uint32_t channel)
 
 #ifdef CONFIG_SOF_USERSPACE_INTERFACE_DMA
 
+struct k_heap;
+struct dma_chan_data;
+
+/**
+ * Kernel-only state of one struct sof_dma entry.
+ *
+ * This is the authoritative state for the kernel. The dma[] array is shared
+ * with user threads (APP_SYSUSER_DATA), so none of its fields can be trusted
+ * in supervisor mode: there, the DMA system call validators and the get/put
+ * reference counting use this structure. The dma[] fields stay as the
+ * user-visible view. The kernel may still read them for non-security
+ * purposes, such as picking a controller in sof_dma_get() or logging, but
+ * every decision that matters is taken from here. The constant fields are
+ * copied from dma[] in dmac_init(), before user threads run.
+ */
+struct sof_dma_kinfo {
+	const struct device *z_dev;	/**< Zephyr DMA device */
+	uint32_t dir;			/**< SOF_DMA_DIR_* supported directions */
+	uint32_t caps;			/**< SOF_DMA_CAP_* capabilities */
+	uint32_t channels;		/**< size of the channel array */
+	struct k_spinlock lock;		/**< protects sref, heap and chan */
+	int sref;			/**< reference count, under lock */
+	struct k_heap *heap;		/**< heap the channel array came from */
+	struct dma_chan_data *chan;	/**< channel array to free on last put */
+};
+
+/**
+ * Return the kernel-only state of @dma.
+ *
+ * @dma is compared against the dma[] array bounds only, it is not
+ * dereferenced, so an untrusted pointer may be passed.
+ *
+ * @return kernel-only state, or NULL if @dma is not an element of dma[]
+ */
+struct sof_dma_kinfo *sof_dma_kinfo_get(const struct sof_dma *dma);
+
 /* include definitions from generated file */
 #include <zephyr/syscalls/sof_dma.h>
 

zephyr/lib/dma.c

@@ -304,6 +304,23 @@ const struct dma_info lib_dma = {
 	.num_dmas = ARRAY_SIZE(dma)
 };
 
+#ifdef CONFIG_SOF_USERSPACE_INTERFACE_DMA
+/* kernel-only, see struct sof_dma_kinfo */
+static struct sof_dma_kinfo dma_kinfo[ARRAY_SIZE(dma)];
+
+struct sof_dma_kinfo *sof_dma_kinfo_get(const struct sof_dma *d)
+{
+	/* use the array symbol, not sof->dma_info, and never dereference d */
+	uintptr_t offset = (uintptr_t)d - (uintptr_t)dma;
+
+	if ((uintptr_t)d < (uintptr_t)dma || offset >= sizeof(dma) ||
+	    offset % sizeof(dma[0]))
+		return NULL;
+
+	return &dma_kinfo[offset / sizeof(dma[0])];
+}
+#endif
+
 /* Initialize all platform DMAC's */
 __cold int dmac_init(struct sof *sof)
 {
@@ -317,5 +334,15 @@ __cold int dmac_init(struct sof *sof)
 	for (i = 0; i < sof->dma_info->num_dmas; i++)
 		k_spinlock_init(&sof->dma_info->dma_array[i].lock);
 
+#ifdef CONFIG_SOF_USERSPACE_INTERFACE_DMA
+	/* snapshot the constant fields before any user thread can modify dma[] */
+	for (i = 0; i < ARRAY_SIZE(dma); i++) {
+		dma_kinfo[i].z_dev = dma[i].z_dev;
+		dma_kinfo[i].dir = dma[i].plat_data.dir;
+		dma_kinfo[i].caps = dma[i].plat_data.caps;
+		dma_kinfo[i].channels = dma[i].plat_data.channels;
+	}
+#endif
+
 	return 0;
 }