[PATCH v1] sparc: ldc: Protect the global channel list

[Binbin Deng]

The global LDC channel list is traversed by the duplicate-ID check in
ldc_alloc(), updated by ldc_alloc(), and modified by ldc_free() without
any common lock.  Consequently, two concurrent allocations can both
observe that an ID is absent, and allocation can race removal of another
channel.

The race is reachable from process context when LDOM virtual-device
channels are configured concurrently, for example while different channel
consumers probe or remove their devices.  A racing insert and delete can
corrupt the hlist or leave a stale node for a channel that is being
freed, leading to a use-after-free during a later ID check.  The
duplicate-ID check is also ineffective because another thread can insert
the same ID between the check and publication.

The allocation path also publishes the channel before request_irq() has
succeeded.  Its current IRQ failure cleanup frees the object without
unhashing it, so keep the node uninitialized until both IRQ handlers are
registered successfully.

Protect the check/publication and removal paths with a mutex, and publish
the node only after both IRQ registrations have succeeded.  Allocation and
free are sleeping, process-context interfaces, so a mutex is appropriate.

Found during a static-analysis-assisted audit: candidate sites were
surfaced by a Coccinelle rule, and the reader/writer paths were
confirmed by LLM-assisted (GLM-5.3) code inspection.  The issue was
not observed at runtime; it is identified by code inspection only.

Fixes: e53e97ce3c71 ("[SPARC64]: Add LDOM virtual channel driver and VIO device layer.")
Assisted-by: GLM-5.3 coccinelle
Signed-off-by: Binbin Deng <18983559317@xxxxxxx>
---
 arch/sparc/kernel/ldc.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/sparc/kernel/ldc.c b/arch/sparc/kernel/ldc.c
index 826fe8ed0289..fbb3021158bd 100644
--- a/arch/sparc/kernel/ldc.c
+++ b/arch/sparc/kernel/ldc.c
@@ -7,6 +7,7 @@
 #include <linux/kernel.h>
 #include <linux/export.h>
 #include <linux/slab.h>
+#include <linux/mutex.h>
 #include <linux/spinlock.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
@@ -952,13 +953,8 @@ static irqreturn_t ldc_tx(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-/* XXX ldc_alloc() and ldc_free() needs to run under a mutex so
- * XXX that addition and removal from the ldc_channel_list has
- * XXX atomicity, otherwise the __ldc_channel_exists() check is
- * XXX totally pointless as another thread can slip into ldc_alloc()
- * XXX and add a channel with the same ID.  There also needs to be
- * XXX a spinlock for ldc_channel_list.
- */
+/* Serializes ldc_channel_list and the channel-ID uniqueness check. */
+static DEFINE_MUTEX(ldc_channel_lock);
 static HLIST_HEAD(ldc_channel_list);
 
 static int __ldc_channel_exists(unsigned long id)
@@ -1165,16 +1161,17 @@ struct ldc_channel *ldc_alloc(unsigned long id,
 	if (hv_err == HV_ECHANNEL)
 		goto out_err;
 
+	mutex_lock(&ldc_channel_lock);
 	err = -EEXIST;
 	if (__ldc_channel_exists(id))
-		goto out_err;
+		goto out_err_locked;
 
 	mssbuf = NULL;
 
 	lp = kzalloc_obj(*lp);
 	err = -ENOMEM;
 	if (!lp)
-		goto out_err;
+		goto out_err_locked;
 
 	spin_lock_init(&lp->lock);
 
@@ -1223,9 +1220,6 @@ struct ldc_channel *ldc_alloc(unsigned long id,
 	lp->hs_state = LDC_HS_CLOSED;
 	ldc_set_state(lp, LDC_STATE_INIT);
 
-	INIT_HLIST_NODE(&lp->list);
-	hlist_add_head(&lp->list, &ldc_channel_list);
-
 	INIT_HLIST_HEAD(&lp->mh_list);
 
 	snprintf(lp->rx_irq_name, LDC_IRQ_NAME_MAX, "%s RX", name);
@@ -1243,6 +1237,11 @@ struct ldc_channel *ldc_alloc(unsigned long id,
 		goto out_free_txq;
 	}
 
+	INIT_HLIST_NODE(&lp->list);
+	hlist_add_head(&lp->list, &ldc_channel_list);
+
+	mutex_unlock(&ldc_channel_lock);
+
 	return lp;
 
 out_free_txq:
@@ -1257,6 +1256,9 @@ struct ldc_channel *ldc_alloc(unsigned long id,
 out_free_ldc:
 	kfree(lp);
 
+out_err_locked:
+	mutex_unlock(&ldc_channel_lock);
+
 out_err:
 	return ERR_PTR(err);
 }
-- 
2.43.0