Re: [PATCH v9 13/14] firmware: qcom_scm: Remove SCM PAS wrappers

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Time-of-check to time-of-use (TOCTOU) race condition in PAS API wrappers leading to potential NULL pointer dereference.
--

commit 979ccc647befac000888b4c5f41818fc1037d4ff
Author: Sumit Garg <sumit.garg@oss.qualcomm.com>

firmware: qcom_scm: Remove SCM PAS wrappers

Now since all the Qcom SCM client drivers have been migrated over to
generic PAS TZ service, let's drop the exported SCM PAS wrappers.

> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 7933e55803dc2..1deee6aea3874 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the new
generic PAS TZ service usage, I noticed a potential race condition in the
API wrappers in drivers/firmware/qcom/qcom_pas.c:

drivers/firmware/qcom/qcom_pas.c:qcom_pas_init_image() {
    ...
	if (!ops_ptr)
		return -ENODEV;

	return ops_ptr->init_image(ops_ptr->dev, pas_id, metadata, size, ctx);
}

Since ops_ptr is a global pointer and isn't accessed with READ_ONCE() or
protected by a lock, could the compiler generate multiple loads?

If a PAS backend is unbound and qcom_pas_ops_unregister() sets ops_ptr to
NULL concurrently, can this lead to a NULL pointer dereference when
accessing ops_ptr->init_image or ops_ptr->dev?

The same pattern appears in other wrappers as well:

drivers/firmware/qcom/qcom_pas.c:qcom_pas_metadata_release() {
    ...
	if (!ops_ptr || !ctx || !ctx->ptr)
		return;

	ops_ptr->metadata_release(ops_ptr->dev, ctx);
}

Should these wrappers use READ_ONCE() or some form of synchronization to
ensure safe dereferencing of ops_ptr?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702115835.167602-1-sumit.garg@kernel.org?part=13