Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

firmware: arm_scmi: Fix chan_free cleanup on SMC

SCMI transport based on SMC can optionally use an additional IRQ to
signal message completion. The associated interrupt handler is currently
allocated using devres but on shutdown the core SCMI stack will call
.chan_free() well before any managed cleanup is invoked by devres.
As a consequence, the arrival of a late reply to an in-flight pending
transaction could still trigger the interrupt handler well after the
SCMI core has cleaned up the channels, with unpleasant results.

Inhibit further message processing on the IRQ path by explicitly freeing
the IRQ inside .chan_free() callback itself.

Fixes: dd820ee21d5e ("firmware: arm_scmi: Augment SMC/HVC to allow optional interrupt")
Reported-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
Link: https://lore.kernel.org/r/20230719173533.2739319-1-cristian.marussi@arm.com
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

authored by

Cristian Marussi and committed by
Sudeep Holla
d1ff11d7 da042eb4

+11 -6
+11 -6
drivers/firmware/arm_scmi/smc.c
··· 40 40 /** 41 41 * struct scmi_smc - Structure representing a SCMI smc transport 42 42 * 43 + * @irq: An optional IRQ for completion 43 44 * @cinfo: SCMI channel info 44 45 * @shmem: Transmit/Receive shared memory area 45 46 * @shmem_lock: Lock to protect access to Tx/Rx shared memory area. ··· 53 52 */ 54 53 55 54 struct scmi_smc { 55 + int irq; 56 56 struct scmi_chan_info *cinfo; 57 57 struct scmi_shared_mem __iomem *shmem; 58 58 /* Protect access to shmem area */ ··· 129 127 struct resource res; 130 128 struct device_node *np; 131 129 u32 func_id; 132 - int ret, irq; 130 + int ret; 133 131 134 132 if (!tx) 135 133 return -ENODEV; ··· 171 169 * completion of a message is signaled by an interrupt rather than by 172 170 * the return of the SMC call. 173 171 */ 174 - irq = of_irq_get_byname(cdev->of_node, "a2p"); 175 - if (irq > 0) { 176 - ret = devm_request_irq(dev, irq, smc_msg_done_isr, 177 - IRQF_NO_SUSPEND, 178 - dev_name(dev), scmi_info); 172 + scmi_info->irq = of_irq_get_byname(cdev->of_node, "a2p"); 173 + if (scmi_info->irq > 0) { 174 + ret = request_irq(scmi_info->irq, smc_msg_done_isr, 175 + IRQF_NO_SUSPEND, dev_name(dev), scmi_info); 179 176 if (ret) { 180 177 dev_err(dev, "failed to setup SCMI smc irq\n"); 181 178 return ret; ··· 195 194 { 196 195 struct scmi_chan_info *cinfo = p; 197 196 struct scmi_smc *scmi_info = cinfo->transport_info; 197 + 198 + /* Ignore any possible further reception on the IRQ path */ 199 + if (scmi_info->irq > 0) 200 + free_irq(scmi_info->irq, scmi_info); 198 201 199 202 cinfo->transport_info = NULL; 200 203 scmi_info->cinfo = NULL;