[PATCH 04/17] dmaengine: idxd: list a new channel only once it is complete

[Daniel Byrne]

idxd_register_dma_channel() links a new channel into the dma_device's
channel list before it registers the channel, and sets the channel's
work queue only after that. The dma_device is registered when the
device is enabled, before any of its work queues, and a channel
request walks the channel list of every registered device under
dma_list_mutex and may hand out any listed channel without a client.
A request that runs while a work queue is being enabled can therefore
take the half-built channel, and the allocation callback dereferences
its NULL work queue with dma_list_mutex held, after which every
channel request and release hangs. The link runs without that mutex
as well, so it races the walkers.

When the registration fails, the channel is freed but left on the
list, where the next request, the next channel's link and the device's
unregistration all reach it. Registration can fail: the core frees a
channel's id before it removes the channel's sysfs device. The channel
of a work queue enabled while another queue of the same device is
being disabled can therefore be handed an id whose sysfs name still
exists, and its registration then fails with -EEXIST.

Add dma_async_device_channel_add(), which registers a channel and only
then links it into its device's list, under dma_list_mutex, and use
it. The driver sets the channel's work queue and initializes its
descriptors before the call, so a request finds the channel complete
or not at all, and a channel that failed to register is never listed.
An enable that hits the -EEXIST race still fails, but leaves nothing
on the list.

A new helper rather than a change to dma_async_device_channel_register():
that function's contract requires the caller to have linked the channel
into the device's list already, which virt-dma's vchan_init() does for
stm32-dma3, the only other caller. That driver has the same window
between the link and the registration and is left for a follow-up.

The helper serves DMA_PRIVATE devices such as idxd's only, since the
channel list of a public device is also walked under RCU alone and its
channels hold references for the dmaengine_get() users, which the
helper does not take. The unbind still unlinks the channel without the
mutex. A later change, which reworks the unbind for channels that
clients still hold, unlinks it under the mutex.

The hang is reachable since the dma_device registration moved to the
device enable in v5.13, so this is for stable too.

Fixes: 8f47d1a5e545 ("dmaengine: idxd: connect idxd to dmaengine subsystem")
Fixes: 397862855619 ("dmaengine: idxd: fix dma device lifetime")
Cc: stable@xxxxxxxxxxxxxxx # 5.13+
Assisted-by: Claude:claude-fable-5-1
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Daniel Byrne <daniel1.byrne@xxxxxxxxx>
---
 drivers/dma/dmaengine.c   | 41 +++++++++++++++++++++++++++++++++++++++
 drivers/dma/idxd/dma.c    | 12 +++++++++---
 include/linux/dmaengine.h |  2 ++
 3 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index c71763047126..ced78c985d1c 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -1155,6 +1155,47 @@ int dma_async_device_channel_register(struct dma_device *device,
 }
 EXPORT_SYMBOL_GPL(dma_async_device_channel_register);
 
+/**
+ * dma_async_device_channel_add - register a channel and make it available
+ * @device: the dmaengine device, DMA_PRIVATE and registered already
+ * @chan: the channel, not on @device's channel list
+ * @name: the channel's name, or NULL for the default one
+ *
+ * Channel requests walk the channel list of every registered device under
+ * dma_list_mutex and may hand out any listed channel that has no client.
+ * A channel added to a registered device is therefore linked to the list
+ * only once it is registered, and under that mutex, so that a request
+ * never finds it half initialized. The driver's own state behind the
+ * channel must be complete before the call. A channel that fails to
+ * register is not linked.
+ *
+ * Only a DMA_PRIVATE device may add channels this way. The channel list
+ * of a public device is also walked under RCU alone, and its channels
+ * hold references for the dmaengine_get() users, which this function
+ * does not take.
+ *
+ * Return: 0 on success or a negative errno.
+ */
+int dma_async_device_channel_add(struct dma_device *device,
+				 struct dma_chan *chan, const char *name)
+{
+	int rc;
+
+	if (WARN_ON_ONCE(!dma_has_cap(DMA_PRIVATE, device->cap_mask)))
+		return -EINVAL;
+
+	rc = __dma_async_device_channel_register(device, chan, name);
+	if (rc < 0)
+		return rc;
+
+	mutex_lock(&dma_list_mutex);
+	list_add_tail(&chan->device_node, &device->channels);
+	dma_channel_rebalance();
+	mutex_unlock(&dma_list_mutex);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dma_async_device_channel_add);
+
 static void __dma_async_device_channel_unregister(struct dma_device *device,
 						  struct dma_chan *chan)
 {
diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
index c116416e10e5..defd1af74c0a 100644
--- a/drivers/dma/idxd/dma.c
+++ b/drivers/dma/idxd/dma.c
@@ -279,7 +279,7 @@ static int idxd_register_dma_channel(struct idxd_wq *wq)
 
 	chan = &idxd_chan->chan;
 	chan->device = dma;
-	list_add_tail(&chan->device_node, &dma->channels);
+	idxd_chan->wq = wq;
 
 	for (i = 0; i < wq->num_descs; i++) {
 		struct idxd_desc *desc = wq->descs[i];
@@ -288,14 +288,20 @@ static int idxd_register_dma_channel(struct idxd_wq *wq)
 		desc->txd.tx_submit = idxd_dma_tx_submit;
 	}
 
-	rc = dma_async_device_channel_register(dma, chan, NULL);
+	/*
+	 * The dma_device is registered already, so a channel request may
+	 * take the channel as soon as it is on the device's list. The core
+	 * links it only once it is registered, and everything a client
+	 * reaches through it is set up by then. What follows serves the
+	 * unbind, which cannot run before this probe returns.
+	 */
+	rc = dma_async_device_channel_add(dma, chan, NULL);
 	if (rc < 0) {
 		kfree(idxd_chan);
 		return rc;
 	}
 
 	wq->idxd_chan = idxd_chan;
-	idxd_chan->wq = wq;
 	get_device(wq_confdev(wq));
 
 	return 0;
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index fe33a20abc61..4178248611d9 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1751,6 +1751,8 @@ static inline int dmaengine_desc_free(struct dma_async_tx_descriptor *desc)
 int dma_async_device_register(struct dma_device *device);
 int dmaenginem_async_device_register(struct dma_device *device);
 void dma_async_device_unregister(struct dma_device *device);
+int dma_async_device_channel_add(struct dma_device *device,
+				 struct dma_chan *chan, const char *name);
 int dma_async_device_channel_register(struct dma_device *device,
 				      struct dma_chan *chan,
 				      const char *name);
-- 
2.43.0