Re: [PATCH v2] drm/imx: Fix ipu_plane_duplicate_state() OOM handling

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The PRE resource allocation in `ipu_planes_assign_pre()` does not track global state, allowing resource overcommit across multiple CRTCs and leading to silent hardware configuration failures.
--

--- Patch [1]: [PATCH v2] drm/imx: Fix ipu_plane_duplicate_state() OOM handling ---
commit e059dd54b800a823912e85c46d6931922398d1c7
Author: Osama Abdelkader <osama.abdelkader@gmail.com>

drm/imx: Fix ipu_plane_duplicate_state() OOM handling

This patch fixes a potential NULL pointer dereference in
ipu_plane_duplicate_state(). When kmalloc_obj() fails, the function now
returns NULL properly instead of unconditionally returning &state->base.

> diff --git a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
> index c7ec09e557c19..67f2da7f2b65e 100644
> --- a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
> @@ -323,9 +323,10 @@ ipu_plane_duplicate_state(struct drm_plane *plane)
>  		return NULL;
>  
>  	state = kmalloc_obj(*state);
> -	if (state)
> -		__drm_atomic_helper_plane_duplicate_state(plane, &state->base);
> +	if (!state)
> +		return NULL;
>  
> +	__drm_atomic_helper_plane_duplicate_state(plane, &state->base);
>  	return &state->base;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this file I
noticed a pre-existing issue with PRE resource allocation.

In ipu_planes_assign_pre(), the available_pres counter is reset to the system
maximum on every atomic commit:

drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c:ipu_planes_assign_pre() {
...
	int available_pres = ipu_prg_max_active_channels();
...
	for_each_new_plane_in_state(state, plane, plane_state, i) {
...
		ipu_state->use_pre = true;
		available_pres--;
...
}

Does this allow resource overcommit across multiple CRTCs if they are
configured independently via separate atomic commits, since it ignores globally
active PREs from other CRTCs?

Additionally, in ipu_plane_atomic_update(), the return value of
ipu_prg_channel_configure() is ignored:

drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c:ipu_plane_atomic_update() {
...
		axi_id = ipu_chan_assign_axi_id(ipu_plane->dma);
		ipu_prg_channel_configure(ipu_plane->ipu_ch, axi_id, width,
					  height, fb->pitches[0],
					  fb->format->format, fb->modifier,
					  &eba);
...
}

If a PRE acquisition fails, will this silently drop the error and program the
hardware with invalid state? Could this cause the IPU to read raw, potentially
tiled memory directly if the expected SRAM buffer address is not mapped?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708202124.12560-1-osama.abdelkader@gmail.com?part=1