Re: [PATCH v4 03/19] firmware: qcom: scm: Fix missing smp_load_acquire()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The waitqueue interrupt is requested and enabled before the global __scm pointer is published, leading to a potential NULL pointer dereference in the IRQ handler path.
--

commit 181d68fe28239c2e3b0bbf1762f175bcdf577648
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

firmware: qcom: scm: Fix missing smp_load_acquire()

This commit fixes a potential ordering violation in
qcom_scm_set_download_mode() by using smp_load_acquire() to safely read
the __scm pointer published by qcom_scm_probe().

> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 9b06a69d3a6d7..c8a84836887f0 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
[ ... ]
> @@ -2841,7 +2844,7 @@ static int qcom_scm_probe(struct platform_device *pdev)
>  	 * will cause the boot stages to enter download mode, unless
>  	 * disabled below by a clean shutdown/reboot.
>  	 */
> -	qcom_scm_set_download_mode(download_mode);
> +	qcom_scm_set_download_mode(scm, download_mode);

[Severity: High]
This is a pre-existing issue, but since we are looking at the initialization
ordering in qcom_scm_probe() in drivers/firmware/qcom/qcom_scm.c, could the
IRQ registration order cause a NULL pointer dereference?

Earlier in qcom_scm_probe(), the waitqueue interrupt is enabled before the
global __scm pointer is published:

	ret = devm_request_threaded_irq(scm->dev, irq, NULL, qcom_scm_irq_handler,
					IRQF_ONESHOT, "qcom-scm", scm);
	...
	smp_store_release(&__scm, scm);

If a pending hardware interrupt fires before __scm is published, the IRQ
handler will execute. The handler calls qcom_scm_waitq_wakeup(), which invokes
qcom_scm_get_completion().

Could qcom_scm_get_completion() crash here, since it ignores the valid scm
context passed via the IRQ data and unconditionally dereferences __scm?

drivers/firmware/qcom/qcom_scm.c:qcom_scm_get_completion() {
	if (WARN_ON_ONCE(wq_ctx >= __scm->wq_cnt))
		return ERR_PTR(-EINVAL);
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624190830.3131112-1-mukesh.ojha@oss.qualcomm.com?part=3