Re: [PATCH 3/3] drm/vc4: Dynamically size the initial tile-alloc block

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing bounds check on user-controlled `bin_tiles_x` and `bin_tiles_y` causes an integer underflow when calculating the `tile alloc size`, leading to massive out-of-bounds writes in GPU memory.
- [Critical] Off-by-one validation check on `max_x_tile` allows out-of-bounds tile rendering, leading to an out-of-bounds physical offset computation.
- [High] Unconditional dereference of `vc4->bin_bo` allows local attackers to trigger a NULL pointer dereference by bypassing the BO allocation.
--

--- Patch [3]: [PATCH 3/3] drm/vc4: Dynamically size the initial tile-alloc block ---
commit 70ba8b264b3becc24aff870ad390492995036867
Author: Maíra Canal <mcanal@igalia.com>

drm/vc4: Dynamically size the initial tile-alloc block

This patch dynamically adjusts the initial tile-allocation block size to fix
hardware hangs caused by unreliable continuation-block chain linking. It
calculates the largest block size that fits within the binner slot for the
job's tile count, and programs the RCL branch stride to match.

Link: https://github.com/raspberrypi/linux/issues/3221
Link: https://github.com/raspberrypi/linux/issues/5780

> diff --git a/drivers/gpu/drm/vc4/vc4_render_cl.c b/drivers/gpu/drm/vc4/vc4_render_cl.c
> index c264d21bc3fe3..be150bad676f6 100644
> --- a/drivers/gpu/drm/vc4/vc4_render_cl.c
> +++ b/drivers/gpu/drm/vc4/vc4_render_cl.c
> @@ -184,7 +184,8 @@ static void emit_tile(struct vc4_exec_info *exec,
>  	if (has_bin) {
>  		rcl_u8(setup, VC4_PACKET_BRANCH_TO_SUB_LIST);
>  		rcl_u32(setup, (exec->tile_alloc_offset +
> -				(y * exec->bin_tiles_x + x) * 32));
> +				(y * exec->bin_tiles_x + x) *
> +				exec->tile_alloc_stride));

[Severity: Critical]
This is a pre-existing issue, but can this out-of-bounds computation occur
here?

If a user specifies args->max_x_tile equal to exec->bin_tiles_x in the render
job ioctl, the check (args->max_x_tile > exec->bin_tiles_x) in vc4_get_rcl()
will pass since it uses > instead of >= for bounds checking.

Would this cause the driver to iterate an extra tile and emit a branch
instruction pointing to an out-of-bounds tile index in the initial tile-alloc
block array, bypassing the command stream validation sandbox?

> diff --git a/drivers/gpu/drm/vc4/vc4_validate.c b/drivers/gpu/drm/vc4/vc4_validate.c
> index 7f2fadfde7a8e..3961cf38fc2a5 100644
> --- a/drivers/gpu/drm/vc4/vc4_validate.c
> +++ b/drivers/gpu/drm/vc4/vc4_validate.c
[ ... ]
> @@ -406,13 +407,35 @@ validate_tile_binning_config(VALIDATE_ARGS)
>  	tile_state_size = 48 * tile_count;
>  
>  	/* Since the tile alloc array will follow us, align. */
> -	exec->tile_alloc_offset = bin_addr + roundup(tile_state_size, 4096);
> +	tile_state_aligned = roundup(tile_state_size, 4096);
> +	exec->tile_alloc_offset = bin_addr + tile_state_aligned;

[Severity: Critical]
This isn't a bug introduced by this patch, but does the lack of bounds
validation on user-controlled bin_tiles_x and bin_tiles_y cause an integer
underflow later when calculating the tile alloc size?

If bin_tiles_x and bin_tiles_y are set to 255, tile_state_aligned evaluates
to ~3.1MB, which is greater than vc4->bin_alloc_size (512KB). The computation
of (vc4->bin_alloc_size - exec->tile_alloc_offset) would then underflow,
writing a massive positive size to the validated command packet. This could
allow the GPU's binner to overwrite arbitrary physical memory out of bounds.

[Severity: High]
This isn't a bug introduced by this patch, but could the unconditional
dereference of vc4->bin_bo in validate_tile_binning_config() trigger a NULL
pointer dereference?

If an attacker allocates a dumb buffer and submits a command list containing
VC4_PACKET_TILE_BINNING_MODE_CONFIG, vc4_v3d_bin_bo_get() isn't called yet,
and the validator might blindly access vc4->bin_bo->base.dma_addr before
it is guaranteed to be allocated.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718-vc4-bin-oom-fixes-v1-0-8cba09558c33@igalia.com?part=3