firmware: arm_scmi: Publish channel state before callbacks

Transport setup can enable callbacks before the setup routine returns.
mailbox_chan_setup() registers the mailbox client with
mbox_request_channel(), and the mailbox controller startup path can enable
interrupt delivery before SCMI mailbox channel state has been published.
Similarly, smc_chan_setup() requests the optional A2P completion IRQ before
the SMC transport has made its cinfo pointer visible.

If a pending or spurious callback fires in those windows, the transport RX
callback can dereference a NULL transport cinfo pointer. Publishing only
the transport-private pointer is not sufficient either: an early callback
can enter the SCMI core before scmi_chan_setup() has assigned
cinfo->handle.
The core derives scmi_info from cinfo->handle in the RX path, so a NULL
handle can still fault even when the transport-private cinfo is valid.

Assign cinfo->handle before invoking the transport setup callback. Publish
the mailbox and SMC transport-private channel state before requesting the
mailbox channels or IRQ, and clear the early-published pointers again on
setup failure. Also unwind mailbox setup devres resources on failure so an
optional RX setup error that is ignored by the core does not leave stale
transport state behind.

Fixes: 5c8a47a5a9 ("firmware: arm_scmi: Make scmi core independent of the transport type")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://patch.msgid.link/20260714-scmi_core_fixes-v6-1-3afe499d46e3@kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
This commit is contained in:
Sudeep Holla 2026-07-14 13:56:20 +01:00
parent 9c62bb72a5
commit 0314900dcd
3 changed files with 23 additions and 12 deletions

View File

@ -2782,6 +2782,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
cinfo->id = prot_id;
cinfo->dev = &tdev->dev;
cinfo->handle = &info->handle;
ret = info->desc->ops->chan_setup(cinfo, info->dev, tx);
if (ret) {
of_node_put(of_node);
@ -2814,7 +2815,6 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
return ret;
}
cinfo->handle = &info->handle;
return 0;
}

View File

@ -211,13 +211,18 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
cl->tx_block = false;
cl->knows_txdone = tx;
cinfo->transport_info = smbox;
smbox->cinfo = cinfo;
mutex_init(&smbox->chan_lock);
smbox->chan = mbox_request_channel(cl, tx ? 0 : p2a_chan);
if (IS_ERR(smbox->chan)) {
ret = PTR_ERR(smbox->chan);
smbox->chan = NULL;
if (ret != -EPROBE_DEFER)
dev_err(cdev,
"failed to request SCMI %s mailbox\n", desc);
return ret;
goto err_clear_cinfo;
}
/* Additional unidirectional channel for TX if needed */
@ -241,11 +246,14 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
}
}
cinfo->transport_info = smbox;
smbox->cinfo = cinfo;
mutex_init(&smbox->chan_lock);
return 0;
err_clear_cinfo:
cinfo->transport_info = NULL;
smbox->cinfo = NULL;
devm_iounmap(dev, smbox->shmem);
devm_kfree(dev, smbox);
return ret;
}
static int mailbox_chan_free(int id, void *p, void *data)

View File

@ -172,6 +172,13 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
scmi_info->param_page = SHMEM_PAGE(res.start);
scmi_info->param_offset = SHMEM_OFFSET(res.start);
}
scmi_info->func_id = func_id;
scmi_info->cap_id = cap_id;
scmi_info->cinfo = cinfo;
smc_channel_lock_init(scmi_info);
cinfo->transport_info = scmi_info;
/*
* If there is an interrupt named "a2p", then the service and
* completion of a message is signaled by an interrupt rather than by
@ -183,18 +190,14 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
IRQF_NO_SUSPEND, dev_name(dev), scmi_info);
if (ret) {
dev_err(dev, "failed to setup SCMI smc irq\n");
cinfo->transport_info = NULL;
scmi_info->cinfo = NULL;
return ret;
}
} else {
cinfo->no_completion_irq = true;
}
scmi_info->func_id = func_id;
scmi_info->cap_id = cap_id;
scmi_info->cinfo = cinfo;
smc_channel_lock_init(scmi_info);
cinfo->transport_info = scmi_info;
return 0;
}