Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Multiple resource leaks (buffer object references and DMA mappings) on error paths in pin_job(). -- commit 6076ffab64411b3ea0b629afb57e0a62e382c1ba Author: WenTao Liang <[email protected]> fix: drm/tegra: tegra_dc_pin: chunks>1 error path leaks current mapping from host1x_bo_pin This commit fixes a memory leak in the tegra_dc_pin() function where an error triggered by map->chunks > 1 causes the function to jump to the unpin label before storing the current mapping. The unpin loop only cleans up previously pinned planes, leaving the current mapping unreleased. Cc: [email protected] > diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c > index ffe5f06b770dc..e0b146a0926e7 100644 > --- a/drivers/gpu/drm/tegra/plane.c > +++ b/drivers/gpu/drm/tegra/plane.c > @@ -161,6 +161,7 @@ static int tegra_dc_pin(struct tegra_dc *dc, struct > tegra_plane_state *state) > */ > if (map->chunks > 1) { > err = -EINVAL; > + host1x_bo_unpin(map); > goto unpin; > } > [Severity: High] This isn't a bug introduced by this patch, but I noticed similar pre-existing issues in drivers/gpu/host1x/job.c:pin_job() while looking at this fix. In the relocs loop of pin_job(): map = host1x_bo_pin(dev, bo, direction, NULL); if (IS_ERR(map)) { err = PTR_ERR(map); goto unpin; } if (map->chunks > 1) { err = -EINVAL; goto unpin; } If map->chunks > 1, does this jump to unpin and leak the newly created map since it hasn't been added to job->unpins yet? Additionally, do both of these error paths leak the bo reference acquired earlier via host1x_bo_get() since host1x_job_unpin() won't release it? Similarly, in the gathers loop of pin_job(): map = host1x_bo_pin(host->dev, g->bo, DMA_TO_DEVICE, NULL); if (IS_ERR(map)) { err = PTR_ERR(map); goto unpin; } Does jumping to unpin here bypass the put: label, failing to release the g->bo reference? And a little further down in the gathers loop: alloc = alloc_iova(&host->iova, gather_size >> shift, host->iova_end >> shift, true); if (!alloc) { err = -ENOMEM; goto put; } err = iommu_map_sgtable(host->domain, iova_dma_addr(&host->iova, alloc), map->sgt, IOMMU_READ); if (err == 0) { __free_iova(&host->iova, alloc); err = -EINVAL; goto put; } When these error paths jump to put:, do they correctly release g->bo but fail to unpin the map created just above? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
