Re: [CI 8/9] drm/i915/color: Add color pipeline support for SDR planes

[sashiko-bot@kernel.org]

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

New issues:
- [High] Exposing the color pipeline for SDR planes to user-space causes visual corruption and hardware state mismatches because the underlying hardware programming functions still artificially restrict operations to HDR planes.

Pre-existing issues:
- [Critical] Dangling DRM object IDR pointer when colorop initialization fails (or on cleanup) leads to a Use-After-Free.
- [High] Missing NULL check after `kasprintf()` in `_intel_color_pipeline_plane_init()` leads to a NULL pointer dereference when string allocation fails.
--

commit 4a507c298b13a8efe368f825fd9f9b9e49485cef
Author: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

drm/i915/color: Add color pipeline support for SDR planes

Now that everything is in place expose the SDR plane color pipeline
to user-space.

> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 6cf8080ee8000..4ea1f34dfb10e 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -155,17 +155,11 @@ int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct drm_prop_en
>  
>  int intel_color_pipeline_plane_init(struct drm_plane *plane, enum pipe pipe)
>  {
> -	struct drm_device *dev = plane->dev;
> -	struct intel_display *display = to_intel_display(dev);
>  	struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES] = {};
>  	int len = 0;
>  	int ret = 0;
>  	int i;
>  
> -	/* Currently expose pipeline only for HDR planes */
> -	if (!icl_is_hdr_plane(display, to_intel_plane(plane)->id))
> -		return 0;
> -

[Severity: High]
Does removing this check cause unprogrammed hardware blocks to be activated?

By exposing the color pipeline to user-space for SDR planes, user-space can
configure the pipeline. If configured, glk_plane_color_ctl() sets the enable
bits (like PLANE_COLOR_PRE_CSC_GAMMA_ENABLE). 

However, functions that program the LUT and CSC registers still appear to
skip SDR planes:

drivers/gpu/drm/i915/display/intel_color.c:xelpd_program_plane_pre_csc_lut() {
    if (!icl_is_hdr_plane(display, plane))
        return;
    ...
}

This early return pattern also exists in xelpd_load_plane_csc_matrix() and
xelpd_program_plane_post_csc_lut(). 

Could this regression lead to visual corruption or hardware state mismatches
if the pipeline is enabled but the registers are never programmed?

>  	/* Add pipeline consisting of transfer functions */

[Severity: High]
This is a pre-existing issue, but does _intel_color_pipeline_plane_init() 
need to check the return value of kasprintf()?

In _intel_color_pipeline_plane_init():

    list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ...);
    return 0;

If kasprintf() fails, list->name is NULL. When this is passed down during 
property registration, drm_property_add_enum() calls strlen(name):

drivers/gpu/drm/drm_property.c:drm_property_add_enum() {
    if (WARN_ON(strlen(name) >= DRM_PROP_NAME_LEN))
        return -EINVAL;
    ...
}

Can this lead to a NULL pointer dereference under memory pressure?

>  	ret = _intel_color_pipeline_plane_init(plane, &pipelines[len], pipe);
>  	if (ret)

[Severity: Critical]
This is a pre-existing issue, but can a failure during colorop 
initialization or destruction lead to a use-after-free?

When drm_plane_colorop_init() is called, it successfully registers the 
colorop with the DRM core mode object IDR:

drivers/gpu/drm/drm_colorop.c:drm_plane_colorop_init() {
    ret = drm_mode_object_add(dev, &colorop->base, DRM_MODE_OBJECT_COLOROP);
    ...
}

However, if an error occurs and intel_colorop_destroy() is called (such
as on the error path of _intel_color_pipeline_plane_init()), or when 
drm_colorop_cleanup() is executed, the structure memory is freed but 
drm_mode_object_unregister() is never called to remove the object from 
the IDR.

Does this leave a dangling pointer in the mode object IDR that could be 
accessed via the DRM_IOCTL_MODE_OBJ_GETPROPERTIES ioctl?

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