Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] media: venus: devm_ allocation in venus_load_fw() causes leak on error recovery --
commit 8d4223f26ac66cddad9e1923272c5c9297949631 Author: Mukesh Ojha <[email protected]> media: venus: migrate firmware loading to qcom_mdt_pas_load() and qcom_pas_prepare_and_auth_reset() This commit migrates the firmware loading for the TrustZone path to use the new PAS context allocation and loading functions. It threads the PAS context from venus_load_fw() through to venus_boot() where authentication occurs. > diff --git a/drivers/media/platform/qcom/venus/firmware.c > b/drivers/media/platform/qcom/venus/firmware.c > index 3a38ff985822f..4ea4c7769ac3d 100644 > --- a/drivers/media/platform/qcom/venus/firmware.c > +++ b/drivers/media/platform/qcom/venus/firmware.c [ ... ] > @@ -117,21 +116,19 @@ static int venus_load_fw(struct venus_core *core, const > char *fwname, > goto err_release_fw; > } > > - mem_va = memremap(*mem_phys, *mem_size, MEMREMAP_WC); > - if (!mem_va) { > - dev_err(dev, "unable to map memory region %pa size %#zx\n", > mem_phys, *mem_size); > - ret = -ENOMEM; > - goto err_release_fw; > - } > - > - if (core->use_tz) > - ret = qcom_mdt_load(dev, mdt, fwname, VENUS_PAS_ID, > - mem_va, *mem_phys, *mem_size, NULL); > - else > - ret = qcom_mdt_load_no_init(dev, mdt, fwname, mem_va, > + if (core->use_tz) { > + core->fw.pas_ctx = devm_qcom_pas_context_alloc(dev, > VENUS_PAS_ID, > + *mem_phys, > *mem_size); [Severity: High] Does this devm allocation leak memory during firmware error recovery? Looking at the firmware recovery path, when a firmware crash occurs, venus_sys_error_handler() repeatedly invokes venus_boot(), which then calls venus_load_fw(). Because devm_qcom_pas_context_alloc() attaches the allocation to the device's lifecycle, it is only freed when the device is unbound. Each recovery cycle will allocate a new PAS context and overwrite core->fw.pas_ctx without freeing the previous one. This could lead to unbounded memory consumption and an OOM crash if an attacker or stream continuously triggers firmware crashes. > + if (IS_ERR(core->fw.pas_ctx)) { > + ret = PTR_ERR(core->fw.pas_ctx); > + core->fw.pas_ctx = NULL; > + goto err_release_fw; > + } > + ret = qcom_mdt_pas_load(core->fw.pas_ctx, mdt, fwname, NULL); > + } else { -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
