Re: [PATCH v8 05/12] media: iris: Enable Secure PAS support with IOMMU managed by Linux

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Failure of `devm_qcom_scm_pas_context_alloc()` leaves an `ERR_PTR` in `core->pas_ctx`, causing a kernel panic on subsequent initialization attempts.
- [High] Missing `qcom_scm_pas_metadata_release()` causes a persistent DMA/tzmem memory leak.
--

commit 1775c14d01e8bdf8905d074121532c742ef2030a
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

media: iris: Enable Secure PAS support with IOMMU managed by Linux

This commit transitions the iris driver to support Secure Peripheral
Authentication Service (PAS) on platforms where Linux manages the IOMMU. It
creates a firmware context device, replaces qcom_mdt_load() with
qcom_mdt_pas_load(), and maps the IOMMU when a firmware device SID is present.

Link: https://lore.kernel.org/lkml/20260105-kvmrprocv10-v10-0-022e96815380@oss.qualcomm.com/

> diff --git a/drivers/media/platform/qcom/iris/iris_firmware.c b/drivers/media/platform/qcom/iris/iris_firmware.c
> --- a/drivers/media/platform/qcom/iris/iris_firmware.c
> +++ b/drivers/media/platform/qcom/iris/iris_firmware.c
[ ... ]
> @@ -30,14 +33,21 @@ static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
>  	if (strlen(fw_name) >= MAX_FIRMWARE_NAME_SIZE - 4)
>  		return -EINVAL;
>  
> -	ret = of_reserved_mem_region_to_resource(dev->of_node, 0, &res);
> +	ret = of_reserved_mem_region_to_resource(core->dev->of_node, 0, &res);
>  	if (ret)
>  		return ret;
>  
>  	mem_phys = res.start;
>  	res_size = resource_size(&res);
>  
> -	ret = request_firmware(&firmware, fw_name, dev);
> +	if (!core->pas_ctx) {
> +		core->pas_ctx = devm_qcom_scm_pas_context_alloc(core->dev, IRIS_PAS_ID,
> +								mem_phys, res_size);
> +		if (IS_ERR(core->pas_ctx))
> +			return PTR_ERR(core->pas_ctx);
> +	}
> +

[Severity: High]
If devm_qcom_scm_pas_context_alloc() fails, it returns an error pointer which
is directly assigned to core->pas_ctx. Since the context pointer is not
cleared back to NULL on failure, will subsequent initialization attempts
bypass the (!core->pas_ctx) check and illegally dereference the error pointer
further down in this function?

> +	ret = request_firmware(&firmware, fw_name, fw_dev);
>  	if (ret)
>  		return ret;
>  
[ ... ]
> @@ -53,9 +63,23 @@ static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
>  		goto err_release_fw;
>  	}
>  
> -	ret = qcom_mdt_load(dev, firmware, fw_name,
> -			    IRIS_PAS_ID, mem_virt, mem_phys, res_size, NULL);
> +	core->pas_ctx->use_tzmem = !!core->fw_dev;
> +	ret = qcom_mdt_pas_load(core->pas_ctx, firmware, fw_name, mem_virt, NULL);

[Severity: High]
This patch transitions to using qcom_mdt_pas_load(), which allocates a
metadata buffer and stores it in core->pas_ctx->ptr. Does this code leak
the metadata buffer by omitting the required qcom_scm_pas_metadata_release()
call in the error paths of this function, as well as during the teardown
sequence?

> +	if (ret)
> +		goto err_mem_unmap;
> +
[ ... ]
> @@ -100,7 +148,11 @@ int iris_fw_unload(struct iris_core *core)
>  {
> -	return qcom_scm_pas_shutdown(IRIS_PAS_ID);
> +	int ret;
> +
> +	ret = qcom_scm_pas_shutdown(core->pas_ctx->pas_id);
> +	iris_fw_iommu_unmap(core);
> +
> +	return ret;
>  }

[Severity: High]
Can memory exhaustion occur from repeated initializations and teardowns since
qcom_scm_pas_metadata_release() is missing from the normal iris_fw_unload()
cleanup path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610-glymur-v8-0-1c79b9d51fc0@oss.qualcomm.com?part=5