[PATCH] mailbox: pcc: Synchronize channel IRQ before unmapping shared memory

[PATCH] mailbox: pcc: Synchronize channel IRQ before unmapping shared memory

[Christian Loehle]

pcc_mbox_free_channel() unmaps the PCC shared-memory region before
mbox_free_channel() invokes the controller shutdown callback. For
interrupt-capable extended subspaces, an in-flight handler may
consequently access the mapping after it has been invalidated.

Release the mailbox channel first so its IRQ is disabled and synchronized
before unmapping the shared-memory region. Serialize PCC channel
acquisition and release across this sequence: once mbox_free_channel()
makes the channel available, another client must not replace the
shared-memory mapping until the old one has been unmapped.

Fixes: 7f9e19f207be ("mailbox: pcc: Check before sending MCTP PCC response ACK")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/mailbox/pcc.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 9888dab64639..4654b028df9f 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -47,12 +47,14 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/list.h>
 #include <linux/log2.h>
+#include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/mailbox_controller.h>
 #include <linux/mailbox_client.h>
@@ -113,6 +115,8 @@ struct pcc_chan_info {
 #define to_pcc_chan_info(c) container_of(c, struct pcc_chan_info, chan)
 static struct pcc_chan_info *chan_info;
 static int pcc_chan_count;
+/* Serializes PCC channel ownership changes with shared-memory teardown. */
+static DEFINE_MUTEX(pcc_mbox_lock);
 
 /*
  * PCC can be used with perf critical drivers such as CPPC
@@ -392,18 +396,23 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
 	if (subspace_id < 0 || subspace_id >= pcc_chan_count)
 		return ERR_PTR(-ENOENT);
 
+	mutex_lock(&pcc_mbox_lock);
+
 	pchan = chan_info + subspace_id;
 	chan = pchan->chan.mchan;
 	if (IS_ERR(chan) || chan->cl) {
 		pr_err("Channel not found for idx: %d\n", subspace_id);
-		return ERR_PTR(-EBUSY);
+		rc = -EBUSY;
+		goto err_unlock;
 	}
 
 	pcc_mchan = &pchan->chan;
 	pcc_mchan->shmem = acpi_os_ioremap(pcc_mchan->shmem_base_addr,
 					   pcc_mchan->shmem_size);
-	if (!pcc_mchan->shmem)
-		return ERR_PTR(-ENXIO);
+	if (!pcc_mchan->shmem) {
+		rc = -ENXIO;
+		goto err_unlock;
+	}
 
 	rc = pcc_mbox_validate_signature(pcc_mchan, subspace_id);
 	if (rc)
@@ -413,11 +422,14 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
 	if (rc)
 		goto err_unmap_shmem;
 
+	mutex_unlock(&pcc_mbox_lock);
 	return pcc_mchan;
 
 err_unmap_shmem:
 	iounmap(pcc_mchan->shmem);
 	pcc_mchan->shmem = NULL;
+err_unlock:
+	mutex_unlock(&pcc_mbox_lock);
 	return ERR_PTR(rc);
 }
 EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
@@ -436,14 +448,18 @@ void pcc_mbox_free_channel(struct pcc_mbox_chan *pchan)
 
 	if (!chan || !chan->cl)
 		return;
+	guard(mutex)(&pcc_mbox_lock);
+
 	pchan_info = chan->con_priv;
 	pcc_mbox_chan = &pchan_info->chan;
+
+	/* Disable and synchronize the channel IRQ before unmapping its data. */
+	mbox_free_channel(chan);
+
 	if (pcc_mbox_chan->shmem) {
 		iounmap(pcc_mbox_chan->shmem);
 		pcc_mbox_chan->shmem = NULL;
 	}
-
-	mbox_free_channel(chan);
 }
 EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
 
-- 
2.34.1

Re: [PATCH] mailbox: pcc: Synchronize channel IRQ before unmapping shared memory

[Sudeep Holla]

On Fri, Aug 28, 2026 at 05:10:33PM +0100, Christian Loehle wrote:
> pcc_mbox_free_channel() unmaps the PCC shared-memory region before
> mbox_free_channel() invokes the controller shutdown callback. For
> interrupt-capable extended subspaces, an in-flight handler may
> consequently access the mapping after it has been invalidated.
> 
> Release the mailbox channel first so its IRQ is disabled and synchronized
> before unmapping the shared-memory region. Serialize PCC channel
> acquisition and release across this sequence: once mbox_free_channel()
> makes the channel available, another client must not replace the
> shared-memory mapping until the old one has been unmapped.
> 

Breno Leitao has already posted the fix for the unmapping before freeing
the channel. You just need the mutex guards.

> Fixes: 7f9e19f207be ("mailbox: pcc: Check before sending MCTP PCC response ACK")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
>  drivers/mailbox/pcc.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 

I believe after Breno's patch, it can be as simple as below:

Regards,
Sudeep

-->8

diff --git i/drivers/mailbox/pcc.c w/drivers/mailbox/pcc.c
index 9888dab64639..e5e8caa54cf2 100644
--- i/drivers/mailbox/pcc.c
+++ w/drivers/mailbox/pcc.c
@@ -53,6 +53,7 @@
 #include <linux/interrupt.h>
 #include <linux/list.h>
 #include <linux/log2.h>
+#include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/mailbox_controller.h>
 #include <linux/mailbox_client.h>
@@ -113,6 +114,7 @@ struct pcc_chan_info {
 #define to_pcc_chan_info(c) container_of(c, struct pcc_chan_info, chan)
 static struct pcc_chan_info *chan_info;
 static int pcc_chan_count;
+static DEFINE_MUTEX(pcc_chan_mutex);

 /*
  * PCC can be used with perf critical drivers such as CPPC
@@ -392,6 +394,8 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
 	if (subspace_id < 0 || subspace_id >= pcc_chan_count)
 		return ERR_PTR(-ENOENT);

+	guard(mutex)(&pcc_chan_mutex);
+
 	pchan = chan_info + subspace_id;
 	chan = pchan->chan.mchan;
 	if (IS_ERR(chan) || chan->cl) {
@@ -434,6 +438,8 @@ void pcc_mbox_free_channel(struct pcc_mbox_chan *pchan)
 	struct pcc_chan_info *pchan_info;
 	struct pcc_mbox_chan *pcc_mbox_chan;

+	guard(mutex)(&pcc_chan_mutex);
+
 	if (!chan || !chan->cl)
 		return;
 	pchan_info = chan->con_priv;

Re: [PATCH] mailbox: pcc: Synchronize channel IRQ before unmapping shared memory

[Sudeep Holla]

On Thu, Sep 03, 2026 at 10:03:38AM +0100, Sudeep Holla wrote:
> On Fri, Aug 28, 2026 at 05:10:33PM +0100, Christian Loehle wrote:
> > pcc_mbox_free_channel() unmaps the PCC shared-memory region before
> > mbox_free_channel() invokes the controller shutdown callback. For
> > interrupt-capable extended subspaces, an in-flight handler may
> > consequently access the mapping after it has been invalidated.
> > 
> > Release the mailbox channel first so its IRQ is disabled and synchronized
> > before unmapping the shared-memory region. Serialize PCC channel
> > acquisition and release across this sequence: once mbox_free_channel()
> > makes the channel available, another client must not replace the
> > shared-memory mapping until the old one has been unmapped.
> > 
> 
> Breno Leitao has already posted the fix for the unmapping before freeing
> the channel. You just need the mutex guards.
>

Failed to add the reference to it [1] earlier.

-- 
Regards,
Sudeep

[1] https://lore.kernel.org/all/20260812-pcc-v1-1-5a94d2cc26cc@debian.org

Re: [PATCH] mailbox: pcc: Synchronize channel IRQ before unmapping shared memory

[Christian Loehle]

On 9/3/26 10:10, Sudeep Holla wrote:
> On Thu, Sep 03, 2026 at 10:03:38AM +0100, Sudeep Holla wrote:
>> On Fri, Aug 28, 2026 at 05:10:33PM +0100, Christian Loehle wrote:
>>> pcc_mbox_free_channel() unmaps the PCC shared-memory region before
>>> mbox_free_channel() invokes the controller shutdown callback. For
>>> interrupt-capable extended subspaces, an in-flight handler may
>>> consequently access the mapping after it has been invalidated.
>>>
>>> Release the mailbox channel first so its IRQ is disabled and synchronized
>>> before unmapping the shared-memory region. Serialize PCC channel
>>> acquisition and release across this sequence: once mbox_free_channel()
>>> makes the channel available, another client must not replace the
>>> shared-memory mapping until the old one has been unmapped.
>>>
>>
>> Breno Leitao has already posted the fix for the unmapping before freeing
>> the channel. You just need the mutex guards.
>>
> 
> Failed to add the reference to it [1] earlier.
> 

Thanks, I didn't see that.
I guess I'll wait until Breno's patch hits linux-next
or linus' tree and rebase?

Re: [PATCH] mailbox: pcc: Synchronize channel IRQ before unmapping shared memory

[Sudeep Holla]

On Thu, Sep 03, 2026 at 10:51:28AM +0100, Christian Loehle wrote:
> On 9/3/26 10:10, Sudeep Holla wrote:
> > On Thu, Sep 03, 2026 at 10:03:38AM +0100, Sudeep Holla wrote:
> >> On Fri, Aug 28, 2026 at 05:10:33PM +0100, Christian Loehle wrote:
> >>> pcc_mbox_free_channel() unmaps the PCC shared-memory region before
> >>> mbox_free_channel() invokes the controller shutdown callback. For
> >>> interrupt-capable extended subspaces, an in-flight handler may
> >>> consequently access the mapping after it has been invalidated.
> >>>
> >>> Release the mailbox channel first so its IRQ is disabled and synchronized
> >>> before unmapping the shared-memory region. Serialize PCC channel
> >>> acquisition and release across this sequence: once mbox_free_channel()
> >>> makes the channel available, another client must not replace the
> >>> shared-memory mapping until the old one has been unmapped.
> >>>
> >>
> >> Breno Leitao has already posted the fix for the unmapping before freeing
> >> the channel. You just need the mutex guards.
> >>
> > 
> > Failed to add the reference to it [1] earlier.
> > 
> 
> Thanks, I didn't see that.
> I guess I'll wait until Breno's patch hits linux-next
> or linus' tree and rebase?
> 

No just post it with a dependency mention. Both can go together IMO.

-- 
Regards,
Sudeep