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

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/[email protected]/




>
> Fixes: 7f9e19f207be ("mailbox: pcc: Check before sending MCTP PCC response ACK")
> Signed-off-by: Breno Leitao <[email protected]>
> ---
>   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 <[email protected]>
>