Re: [v5 07/14] drm/i915/color: Add YCbCr limited-to-full range color block support

[sashiko-bot@kernel.org]

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

New issues:
- [High] YUV range correction is incorrectly enabled by default for all color pipelines, corrupting both RGB and YUV surfaces.
- [Medium] The INTEL_PLANE_CB_YUV_RANGE_CORRECT color block is never added to the plane color pipeline arrays, making the feature unreachable.

Pre-existing issues:
- [Critical] Use-After-Free via leaked DRM IDR pointer during colorop cleanup.
- [High] NULL Pointer Dereference on colorop initialization error path.
--

--- Patch [7]: [v5 07/14] drm/i915/color: Add YCbCr limited-to-full range color block support ---
commit 63b36f46939531b2add35ef940340e77707f342a
Author: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

drm/i915/color: Add YCbCr limited-to-full range color block support

Add support for color block which performs YCbCr limited-to-full
range expansion in the plane color pipeline. The color block is
represented using DRM_COLOROP_FIXED_MATRIX colorop.

> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 2ef42a133a987..7d25f4fbfa9b7 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -108,6 +108,12 @@ struct intel_colorop *intel_color_pipeline_plane_add_colorop(struct drm_plane *p
>  							  intel_plane_supported_csc_ff,
>  							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  		break;
> +	case INTEL_PLANE_CB_YUV_RANGE_CORRECT:
> +		ret = drm_plane_colorop_fixed_matrix_init(dev, &colorop->base, plane,
> +							  &intel_colorop_funcs,
> +							  BIT(DRM_COLOROP_FM_YCBCR_LIMITED_FULL),
> +							  DRM_COLOROP_FLAG_ALLOW_BYPASS);

[Severity: Medium]
Is INTEL_PLANE_CB_YUV_RANGE_CORRECT actually reachable? It appears this color
block is omitted from the pipeline definition arrays like sdr_plane_pipeline,
which means it will never be added to the pipeline.

[Severity: High]
This is a pre-existing issue, but does drm_colorop_cleanup() dereference a
NULL pointer if drm_plane_colorop_fixed_matrix_init() fails?

If drm_mode_object_add() fails during initialization, it returns an error
before colorop->dev is set. The cleanup path calls drm_colorop_cleanup(),
which unconditionally dereferences colorop->dev:

void drm_colorop_cleanup(struct drm_colorop *colorop)
{
	struct drm_device *dev = colorop->dev;
	struct drm_mode_config *config = &dev->mode_config;

This would result in a crash during device probe.

[Severity: Critical]
This is also a pre-existing issue, but does drm_colorop_cleanup() leak the
DRM IDR pointer?

drm_plane_colorop_fixed_matrix_init() registers the colorop with the DRM IDR
via drm_mode_object_add(). However, drm_colorop_cleanup() fails to call
drm_mode_object_unregister(), leaving a dangling pointer in the IDR:

void drm_colorop_cleanup(struct drm_colorop *colorop)
{
    ...
	list_del(&colorop->head);
	config->num_colorop--;

	if (colorop->state)
		drm_colorop_atomic_destroy_state(colorop, colorop->state);
}

Could this lead to a Use-After-Free if userspace invokes DRM IOCTLs (like
DRM_IOCTL_MODE_OBJ_GETPROPERTIES) using the leaked ID?

> +		break;
>  	default:
>  		drm_err(plane->dev, "Invalid colorop id [%d]", id);
>  		ret = -EINVAL;

[ ... ]

> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 9893b061da5fa..88df4a76a0ab8 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1295,6 +1295,8 @@ static u32 glk_plane_color_ctl_input_csc(const struct intel_plane_state *plane_s
>  	} else if (!icl_is_hdr_plane(display, plane->id)) {
>  		if (plane_state->hw.csc_ff_enable)
>  			ctl |= intel_csc_ff_type_to_csc_mode(plane_state->hw.csc_ff_type);
> +		if (plane_state->hw.yuv_range_correction_disable)
> +			ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;

[Severity: High]
Does this logic inadvertently enable YUV range correction by default for all
color pipelines, corrupting RGB and YUV surfaces?

When a color pipeline is active, the early return for non-YUV surfaces is
skipped in glk_plane_color_ctl_input_csc():

    if (!fb->format->is_yuv)
        return 0;

Since INTEL_PLANE_CB_YUV_RANGE_CORRECT is missing from the pipeline array,
yuv_range_correction_disable defaults to false. This leaves the
PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE bit unset (meaning enabled).

Furthermore, HDR planes skip this bit assignment entirely due to the
!icl_is_hdr_plane() check, meaning range correction is unconditionally enabled
for them as well.

>  	}
>  
>  	return ctl;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720045131.971650-1-chaitanya.kumar.borah@intel.com?part=7