[PATCH] mailbox: Serialize mbox_free_channel() to prevent double shutdown

[Anup Vishwakarma]

mbox_free_channel() checks chan->cl without holding con_mutex, while
mbox_controller_unregister() frees the same channels under it. A client
releasing its channel can therefore run concurrently with unregister.

During concurrent driver unbind both paths can observe a non-NULL
chan->cl for the same channel and both tear it down. This calls
ops->shutdown() twice: in arm_mhu_db.c, mhu_db_shutdown() frees
chan->con_priv and dereferences it without a NULL check, so the second
call panics. Both paths also reach mbox_clean_and_put_channel(), issuing
two module_put() for one try_module_get().

Fix by splitting mbox_free_channel() into a locked public wrapper and an
unlocked __mbox_free_channel() helper. mbox_controller_unregister()
already holds con_mutex and calls the helper directly. chan->lock can't
be used instead, as ops->shutdown() may sleep (free_irq() in arm_mhu.c).

Fixes: 2b6d83e2b8b7d ("mailbox: Introduce framework for mailbox")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Anup Vishwakarma <anup.vishwakarma@xxxxxxxxxxxxxxxx>
---
 drivers/mailbox/mailbox.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index efacd24a085d..2cc655dad9e2 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -504,12 +504,7 @@ struct mbox_chan *mbox_request_channel_byname(struct mbox_client *cl,
 }
 EXPORT_SYMBOL_GPL(mbox_request_channel_byname);
 
-/**
- * mbox_free_channel - The client relinquishes control of a mailbox
- *			channel by this call.
- * @chan: The mailbox channel to be freed.
- */
-void mbox_free_channel(struct mbox_chan *chan)
+static void __mbox_free_channel(struct mbox_chan *chan)
 {
 	if (!chan || !chan->cl)
 		return;
@@ -519,6 +514,17 @@ void mbox_free_channel(struct mbox_chan *chan)
 
 	mbox_clean_and_put_channel(chan);
 }
+
+/**
+ * mbox_free_channel - The client relinquishes control of a mailbox
+ *			channel by this call.
+ * @chan: The mailbox channel to be freed.
+ */
+void mbox_free_channel(struct mbox_chan *chan)
+{
+	guard(mutex)(&con_mutex);
+	__mbox_free_channel(chan);
+}
 EXPORT_SYMBOL_GPL(mbox_free_channel);
 
 static struct mbox_chan *fw_mbox_index_xlate(struct mbox_controller *mbox,
@@ -596,7 +602,7 @@ void mbox_controller_unregister(struct mbox_controller *mbox)
 		list_del(&mbox->node);
 
 		for (i = 0; i < mbox->num_chans; i++)
-			mbox_free_channel(&mbox->chans[i]);
+			__mbox_free_channel(&mbox->chans[i]);
 
 		if (mbox->txdone_poll)
 			hrtimer_cancel(&mbox->poll_hrt);

---
base-commit: 20a33c6b6cbe7e4805dfb6cd68816937443aa148
change-id: 20261026-b4-mbox_free_channel_lock_upstream-fb263a88853a

Best regards,
--  
Anup Vishwakarma <anup.vishwakarma@xxxxxxxxxxxxxxxx>