drivers/mailbox/pcc.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-)
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.1On 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;
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
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?
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
© 2016 - 2026 Red Hat, Inc.