Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Removing the `if (!ctx->ptr)` check from the metadata release path causes a kernel crash when called with a `NULL` pointer on error paths or during teardown. --
commit e2ee3fe2c5f5a59a30d27512b70afb597cba4bec Author: Sumit Garg <[email protected]> firmware: qcom_scm: Migrate to generic PAS service With the availability of generic PAS service, this patch adds SCM calls as a backend to keep supporting legacy QTEE interfaces. The exported qcom_scm* wrappers will get dropped once all the client drivers get migrated as part of future patches. > diff --git a/drivers/firmware/qcom/qcom_scm.c > b/drivers/firmware/qcom/qcom_scm.c > index 6b601a4b89dbf..7933e55803dc2 100644 > --- a/drivers/firmware/qcom/qcom_scm.c > +++ b/drivers/firmware/qcom/qcom_scm.c [ ... ] > -/** > - * qcom_scm_pas_metadata_release() - release metadata context > - * @ctx: pas context > - */ > -void qcom_scm_pas_metadata_release(struct qcom_scm_pas_context *ctx) > +static void __qcom_scm_pas_metadata_release(struct device *dev, > + struct qcom_pas_context *ctx) > { > - if (!ctx->ptr) > - return; > + if (ctx->use_tzmem) > + qcom_tzmem_free(ctx->ptr); > else > - dma_free_coherent(__scm->dev, ctx->size, ctx->ptr, ctx->phys); > + dma_free_coherent(dev, ctx->size, ctx->ptr, ctx->phys); [Severity: High] Will this cause a kernel crash by passing a NULL pointer to dma_free_coherent() on error paths? If a client driver calls qcom_scm_pas_init_image() and it fails before setting ctx->ptr (for example, if dma_alloc_coherent() fails), the client's error handler will then call qcom_scm_pas_metadata_release(). Since the !ctx->ptr check was removed, this would execute dma_free_coherent() with a NULL pointer. I note that a later commit in the series (commit 823d5bd22444cb98ee7ba) removes these SCM PAS wrappers entirely after clients are migrated. However, does removing this safety check here break bisection for existing client drivers in the meantime? > > ctx->ptr = NULL; > } > + > +void qcom_scm_pas_metadata_release(struct qcom_scm_pas_context *ctx) > +{ > + __qcom_scm_pas_metadata_release(__scm->dev, > + (struct qcom_pas_context *)ctx); > +} > EXPORT_SYMBOL_GPL(qcom_scm_pas_metadata_release); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
