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 <[email protected]> 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/[email protected]?part=13
