Re: [PATCH v3 4/7] drm/amd/display: truly bypass plane colorop 3x4 matrix and hdr mult

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Dereference of `blob->length` when `blob` is NULL in `__set_dm_plane_colorop_3x4_matrix`.
--

commit e38d376afd75d8310bd23096e9f3d253c0d99c70
Author: Melissa Wen <mwen@igalia.com>

drm/amd/display: truly bypass plane colorop 3x4 matrix and hdr mult

This commit addresses a latent issue where the driver skips programming the
3x4 matrix and hdr multiplier blocks on bypass. It updates the driver to
explicitly reset these blocks to their default values if the bypass
property is set.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index 86086d10c5435..13ad7ac1e35e4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -1579,7 +1579,13 @@ __set_dm_plane_colorop_3x4_matrix(struct drm_plane_state *plane_state,
>  		}
>  	}
>  
> -	if (colorop_state && !colorop_state->bypass && colorop->type == DRM_COLOROP_CTM_3X4) {
> +	if (colorop_state && colorop->type == DRM_COLOROP_CTM_3X4) {
> +		if (colorop_state->bypass) {
> +			dc_plane_state->gamut_remap_matrix.enable_remap = false;
> +			dc_plane_state->input_csc_color_matrix.enable_adjustment = false;
> +			return 0;
> +		}
> +
>  		drm_dbg(dev, "3x4 matrix colorop with ID: %d\n", colorop->base.id);
>  		blob = colorop_state->data;
>  		if (blob->length == sizeof(struct drm_color_ctm_3x4)) {

[Severity: High]
This isn't a bug introduced by this patch, but can blob be NULL here when
colorop_state->bypass is false?

Looking at __set_dm_plane_colorop_3x4_matrix(), the DRM API allows userspace
to unset the data property (for instance, by passing 0 for the blob ID).
This would result in colorop_state->data being NULL. 

If userspace sets bypass to false while providing no data blob, would the
unconditional dereference of blob->length lead to a NULL pointer dereference
and a kernel crash?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609121230.1358786-1-mwen@igalia.com?part=4