Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Switching to the generic PAS API exposes the IPA driver to a Time-of-Check to Time-of-Use (TOCTOU) race condition and Use-After-Free due to lockless access to the global `ops_ptr`. --
commit dccc0e82eb171fead953bff95c97cce60f8a1717 Author: Sumit Garg <[email protected]> net: ipa: Switch to generic PAS TZ APIs This commit switches the ipa client driver to use the generic PAS TZ APIs instead of the scm-specific implementation. This allows supporting multiple TZ implementation backends. > diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c > index 788dd99af2a4d..3cd9e44680e9f 100644 > --- a/drivers/net/ipa/ipa_main.c > +++ b/drivers/net/ipa/ipa_main.c [ ... ] > @@ -624,10 +624,13 @@ static int ipa_firmware_load(struct device *dev) > } > > ret = qcom_mdt_load(dev, fw, path, IPA_PAS_ID, virt, phys, size, NULL); > - if (ret) > + if (ret) { > dev_err(dev, "error %d loading \"%s\"\n", ret, path); > - else if ((ret = qcom_scm_pas_auth_and_reset(IPA_PAS_ID))) > - dev_err(dev, "error %d authenticating \"%s\"\n", ret, path); > + } else { > + ret = qcom_pas_auth_and_reset(IPA_PAS_ID); [Severity: High] Does switching to qcom_pas_auth_and_reset() expose the driver to a race condition if the PAS backend is unbound concurrently? Looking at the underlying qcom_pas_auth_and_reset() implementation in drivers/firmware/qcom/qcom_pas.c, it performs a lockless check: if (!ops_ptr) return -ENODEV; return ops_ptr->auth_and_reset(ops_ptr->dev, pas_id); If a privileged user unbinds the PAS backend or unloads its module after ops_ptr is checked but before it is dereferenced, ops_ptr could be set to NULL or the backend module memory could be freed. Since qcom_pas.c doesn't appear to use synchronization or module reference counting (like try_module_get()) to protect ops_ptr, could this result in a NULL pointer dereference or a use-after-free by executing freed module text when the dereference occurs? > + if (ret) > + dev_err(dev, "error %d authenticating \"%s\"\n", ret, > path); > + } > > memunmap(virt); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
