* Re: [PATCH RFC] gpu: host1x: validate the firewall source mapping
2026-09-11 6:09 [PATCH RFC] gpu: host1x: validate the firewall source mapping Slavin Liu
@ 2026-09-11 6:24 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-09-11 6:24 UTC (permalink / raw)
To: Slavin Liu; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing IS_ERR() check for host1x_bo_mmap() return values across multiple functions.
- [High] BO reference and mapping leak on failure paths in pin_job()
- [High] Passing ERR_PTR to drm_syncobj_replace_fence() after host1x_fence_create() fails.
--
commit 1a11db6d4b6dd9dc137819f2fb17f9728ae00690
Author: Slavin Liu <bolin.liu@seu.edu.cn>
gpu: host1x: validate the firewall source mapping
This commit adds missing validation for the firewall source mapping in
copy_gathers(). It correctly rejects an ERR_PTR or NULL source mapping
before copying gather commands, fixing a potential out-of-bounds dereference.
> diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
> --- a/drivers/gpu/host1x/job.c
> +++ b/drivers/gpu/host1x/job.c
> @@ -575,6 +575,10 @@ static inline int copy_gathers(struct device *host, struct host1x_job *job,
>
> /* Copy the gather */
> gather = host1x_bo_mmap(g->bo);
> + if (IS_ERR(gather))
> + return PTR_ERR(gather);
> + if (!gather)
> + return -ENOMEM;
[Severity: High]
This isn't a bug introduced by this patch, but while this adds the proper
validation here, there are a few other pre-existing locations that only
check for NULL when calling host1x_bo_mmap().
For instance, in do_relocs() in the same file:
cmdbuf_addr = host1x_bo_mmap(cmdbuf);
if (unlikely(!cmdbuf_addr)) {
Could this also evaluate as true for an ERR_PTR and cause an out-of-bounds
pointer dereference when patching relocations?
Similarly, trace_write_gather() in drivers/gpu/host1x/hw/channel_hw.c:
mem = host1x_bo_mmap(bo);
if (mem) {
And show_channel_gathers() in drivers/gpu/host1x/hw/debug_hw.c:
mapped = host1x_bo_mmap(g->bo);
if (!mapped) {
host1x_debug_output(o, "[could not mmap]\n");
continue;
}
Do these paths also need IS_ERR() checks to prevent error pointers from
falling through to be dereferenced?
[Severity: High]
This is another pre-existing issue in the same file, but looking at
pin_job(), do the failure paths leak BO references or mappings?
When iterating over job->relocs, if host1x_bo_pin() fails:
map = host1x_bo_pin(dev, bo, direction, NULL);
if (IS_ERR(map)) {
err = PTR_ERR(map);
goto unpin;
}
Since the jump to the unpin label calls host1x_job_unpin(job), which only
processes items already added to the job->unpins array, does this skip
releasing the BO reference acquired by the host1x_bo_get() just above it?
Also in pin_job(), within the gathers loop, if alloc_iova() fails:
alloc = alloc_iova(&host->iova, gather_size >> shift,
host->iova_end >> shift, true);
if (!alloc) {
err = -ENOMEM;
goto put;
}
Does jumping to the put label leave the successfully created map orphaned
without calling host1x_bo_unpin(map)?
[Severity: High]
This is also a pre-existing issue, but I noticed a potential problem with
fence creation error handling in drivers/gpu/drm/tegra/submit.c.
In tegra_drm_ioctl_channel_submit(), if host1x_fence_create() fails:
if (IS_ERR(fence)) {
err = PTR_ERR(fence);
SUBMIT_ERR(context, "failed to create postfence: %d", err);
}
drm_syncobj_replace_fence(syncobj, fence);
Does execution fall through after the failure and pass the error pointer
directly to drm_syncobj_replace_fence()? It appears
drm_syncobj_replace_fence() evaluates the pointer as true and calls
dma_fence_get(fence), which will dereference the error pointer during
kref_get().
> memcpy(job->gather_copy_mapped + offset, gather + g->offset,
> g->words * sizeof(u32));
> host1x_bo_munmap(g->bo, gather);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911060933.94619-1-bolin.liu@seu.edu.cn?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread