[PATCH] mailbox: pcc: Free the channel before unmapping the shared memory

[PATCH] mailbox: pcc: Free the channel before unmapping the shared memory

[Breno Leitao]

I am seeing a crash on PCC that is related to a an shared memory being
unmapped before the IRQ is disabled, and the IRQ kicks in and hits the
unmapped (NULL) address. This is a summary of what I see on my box:

	scmi_protocol scmi_dev.1: Message for 1 type 0 is not expected!
	Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
	 __handle_irq_event_percpu+0x1c4/0x9e0
	 handle_irq_event+0x98/0x218
	 handle_fasteoi_irq+0x230/0x750
	 generic_handle_domain_irq+0xac/0x138
	 gic_handle_irq+0x344/0x740
	 call_on_irq_stack+0x30/0x48

The trapping store is iowrite32(SCMI_SHMEM_FLAG_INTR_ENABLED,
&shmem->header.flags), a write of 1 at offset 4 of a NULL base.

But, back to the problem, pcc_mbox_free_channel() unmaps the shared
memory and clears pchan->chan.shmem *before* freeing the IRQ (aka
calling mbox_free_channel()).

The interrupt is still live when the mapping goes away.

Free the channel first, before the memory unmap. mbox_free_channel()
calls pcc_shutdown(), which frees the platform interrupt, and then unmap
shared memory.

Fixes: 7f9e19f207be ("mailbox: pcc: Check before sending MCTP PCC response ACK")
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/mailbox/pcc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 636879ae1db76..d32f170141de7 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -408,12 +408,13 @@ void pcc_mbox_free_channel(struct pcc_mbox_chan *pchan)
 		return;
 	pchan_info = chan->con_priv;
 	pcc_mbox_chan = &pchan_info->chan;
+
+	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);
 

---
base-commit: 5e6de6a2b522f659defacb1551d0465ba6ce13cf
change-id: 20260812-pcc-3a7413982a5e

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH] mailbox: pcc: Free the channel before unmapping the shared memory

[Sudeep Holla]

On Wed, Aug 12, 2026 at 05:36:03AM -0700, Breno Leitao wrote:
> I am seeing a crash on PCC that is related to a an shared memory being
> unmapped before the IRQ is disabled, and the IRQ kicks in and hits the
> unmapped (NULL) address. This is a summary of what I see on my box:
> 
> 	scmi_protocol scmi_dev.1: Message for 1 type 0 is not expected!
> 	Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
> 	 __handle_irq_event_percpu+0x1c4/0x9e0
> 	 handle_irq_event+0x98/0x218
> 	 handle_fasteoi_irq+0x230/0x750
> 	 generic_handle_domain_irq+0xac/0x138
> 	 gic_handle_irq+0x344/0x740
> 	 call_on_irq_stack+0x30/0x48
> 

Just curious as how are you testing ACPI PCC + SCMI using upstream and
on which platform ?

-- 
Regards,
Sudeep

Re: [PATCH] mailbox: pcc: Free the channel before unmapping the shared memory

[Breno Leitao]

Hello Sudeep,

On Thu, Aug 13, 2026 at 03:57:29AM +0100, Sudeep Holla wrote:
> On Wed, Aug 12, 2026 at 05:36:03AM -0700, Breno Leitao wrote:
> > I am seeing a crash on PCC that is related to a an shared memory being
> > unmapped before the IRQ is disabled, and the IRQ kicks in and hits the
> > unmapped (NULL) address. This is a summary of what I see on my box:
> > 
> > 	scmi_protocol scmi_dev.1: Message for 1 type 0 is not expected!
> > 	Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
> > 	 __handle_irq_event_percpu+0x1c4/0x9e0
> > 	 handle_irq_event+0x98/0x218
> > 	 handle_fasteoi_irq+0x230/0x750
> > 	 generic_handle_domain_irq+0xac/0x138
> > 	 gic_handle_irq+0x344/0x740
> > 	 call_on_irq_stack+0x30/0x48
> > 
> 
> Just curious as how are you testing ACPI PCC + SCMI using upstream and
> on which platform ?

I am using Arm AGI CPU with the latest SCMI patches that are being under
discussion in the mailing list.

--breno

Re: [PATCH] mailbox: pcc: Free the channel before unmapping the shared memory

[Sudeep Holla]

On Thu, Aug 13, 2026 at 01:18:58AM -0700, Breno Leitao wrote:
> Hello Sudeep,
> 
> On Thu, Aug 13, 2026 at 03:57:29AM +0100, Sudeep Holla wrote:
> > On Wed, Aug 12, 2026 at 05:36:03AM -0700, Breno Leitao wrote:
> > > I am seeing a crash on PCC that is related to a an shared memory being
> > > unmapped before the IRQ is disabled, and the IRQ kicks in and hits the
> > > unmapped (NULL) address. This is a summary of what I see on my box:
> > > 
> > > 	scmi_protocol scmi_dev.1: Message for 1 type 0 is not expected!
> > > 	Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
> > > 	 __handle_irq_event_percpu+0x1c4/0x9e0
> > > 	 handle_irq_event+0x98/0x218
> > > 	 handle_fasteoi_irq+0x230/0x750
> > > 	 generic_handle_domain_irq+0xac/0x138
> > > 	 gic_handle_irq+0x344/0x740
> > > 	 call_on_irq_stack+0x30/0x48
> > > 
> > 
> > Just curious as how are you testing ACPI PCC + SCMI using upstream and
> > on which platform ?
> 
> I am using Arm AGI CPU with the latest SCMI patches that are being under
> discussion in the mailing list.
> 

Thanks, I just wanted to rule out any hidden issues in the ACPI PCC driver
I wrote which hasn't got much review 😉.

-- 
Regards,
Sudeep

Re: [PATCH] mailbox: pcc: Free the channel before unmapping the shared memory

[Sudeep Holla]

On Wed, Aug 12, 2026 at 05:36:03AM -0700, Breno Leitao wrote:
> I am seeing a crash on PCC that is related to a an shared memory being
> unmapped before the IRQ is disabled, and the IRQ kicks in and hits the
> unmapped (NULL) address. This is a summary of what I see on my box:
> 
> 	scmi_protocol scmi_dev.1: Message for 1 type 0 is not expected!
> 	Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
> 	 __handle_irq_event_percpu+0x1c4/0x9e0
> 	 handle_irq_event+0x98/0x218
> 	 handle_fasteoi_irq+0x230/0x750
> 	 generic_handle_domain_irq+0xac/0x138
> 	 gic_handle_irq+0x344/0x740
> 	 call_on_irq_stack+0x30/0x48
> 
> The trapping store is iowrite32(SCMI_SHMEM_FLAG_INTR_ENABLED,
> &shmem->header.flags), a write of 1 at offset 4 of a NULL base.
> 
> But, back to the problem, pcc_mbox_free_channel() unmaps the shared
> memory and clears pchan->chan.shmem *before* freeing the IRQ (aka
> calling mbox_free_channel()).
> 
> The interrupt is still live when the mapping goes away.
> 
> Free the channel first, before the memory unmap. mbox_free_channel()
> calls pcc_shutdown(), which frees the platform interrupt, and then unmap
> shared memory.
> 
> Fixes: 7f9e19f207be ("mailbox: pcc: Check before sending MCTP PCC response ACK")

Reviewed-by: Sudeep Holla <sudeep.holla@kernel.org>

Digging my git repo, I found I had this fix initially as part of
Commit 9f3bbbb72ad0 ("mailbox: pcc: Initialize SHMEM before binding the
channel with the client") when I encountered that issue but then removed
to post it as separate fix which I never did 🙁.

Thanks for catching this, much appreciated!

-- 
Regards,
Sudeep

Re: [PATCH] mailbox: pcc: Free the channel before unmapping the shared memory

[Adam Young]

On 8/12/26 08:36, Breno Leitao wrote:
> I am seeing a crash on PCC that is related to a an shared memory being
> unmapped before the IRQ is disabled, and the IRQ kicks in and hits the
> unmapped (NULL) address. This is a summary of what I see on my box:
>
> 	scmi_protocol scmi_dev.1: Message for 1 type 0 is not expected!
> 	Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
> 	 __handle_irq_event_percpu+0x1c4/0x9e0
> 	 handle_irq_event+0x98/0x218
> 	 handle_fasteoi_irq+0x230/0x750
> 	 generic_handle_domain_irq+0xac/0x138
> 	 gic_handle_irq+0x344/0x740
> 	 call_on_irq_stack+0x30/0x48
>
> The trapping store is iowrite32(SCMI_SHMEM_FLAG_INTR_ENABLED,
> &shmem->header.flags), a write of 1 at offset 4 of a NULL base.
>
> But, back to the problem, pcc_mbox_free_channel() unmaps the shared
> memory and clears pchan->chan.shmem *before* freeing the IRQ (aka
> calling mbox_free_channel()).
>
> The interrupt is still live when the mapping goes away.
>
> Free the channel first, before the memory unmap. mbox_free_channel()
> calls pcc_shutdown(), which frees the platform interrupt, and then unmap
> shared memory.

I posted a related fix undere the MCTP PCC Driver changes.

This fix is necessary but not sufficient to deal with the race 
conditions.  Take a look at the series of patches under here:

https://lore.kernel.org/all/20260721175258.87600-1-admiyo@os.amperecomputing.com/




>
> Fixes: 7f9e19f207be ("mailbox: pcc: Check before sending MCTP PCC response ACK")
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>   drivers/mailbox/pcc.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 636879ae1db76..d32f170141de7 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -408,12 +408,13 @@ void pcc_mbox_free_channel(struct pcc_mbox_chan *pchan)
>   		return;
>   	pchan_info = chan->con_priv;
>   	pcc_mbox_chan = &pchan_info->chan;
> +
> +	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);
>   
>
> ---
> base-commit: 5e6de6a2b522f659defacb1551d0465ba6ce13cf
> change-id: 20260812-pcc-3a7413982a5e
>
> Best regards,
> --
> Breno Leitao <leitao@debian.org>
>

Re: [PATCH] mailbox: pcc: Free the channel before unmapping the shared memory

[Breno Leitao]

Hello Adam,

On Sat, Aug 15, 2026 at 11:58:16PM -0400, Adam Young wrote:
> On 8/12/26 08:36, Breno Leitao wrote:
> > Free the channel first, before the memory unmap. mbox_free_channel()
> > calls pcc_shutdown(), which frees the platform interrupt, and then unmap
> > shared memory.
>
> I posted a related fix undere the MCTP PCC Driver changes.
>
> This fix is necessary but not sufficient to deal with the race conditions.
> Take a look at the series of patches under here:
>
> https://lore.kernel.org/all/20260721175258.87600-1-admiyo@os.amperecomputing.com/

That looks like a much bigger series, with new features, open discussions,
Nacks, and already 45 revisions.

This curent fix is only meant to address a real problem (user after
free) I am hitting in production systems.

My suggestion is that we get this real issue unblocked, even if not
complete, while you keep working on your larger series.

Thanks,
--breno