Re: [CI v4 14/14] drm/i915/color: Add color pipeline support for SDR planes

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing NULL check for kasprintf() return value in _intel_color_pipeline_plane_init() leads to a deterministic NULL pointer dereference.
- [High] If a color operation initialization fails early, the cleanup path unconditionally calls drm_colorop_cleanup on a partially initialized object, triggering a NULL pointer dereference.
--

--- Patch [14]: [CI v4 14/14] drm/i915/color: Add color pipeline support for SDR planes ---
commit 6fb2b458fe3e54b8772eb125265bd378c4f8ec9e
Author: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
drm/i915/color: Add color pipeline support for SDR planes

This commit exposes the SDR plane color pipeline to user-space by
removing the HDR plane restriction during initialization.

> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 136b9378ca484..a99b6eb204488 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -188,17 +188,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;
> -
>  	/* Add pipeline consisting of transfer functions */
>  	ret = _intel_color_pipeline_plane_init(plane, &pipelines[len], pipe);

[Severity: High]
This is a pre-existing issue, but does _intel_color_pipeline_plane_init() 
lack a check for kasprintf() allocation failures?

If memory allocation fails during plane initialization, kasprintf() 
returns a NULL pointer:

_intel_color_pipeline_plane_init()
    list->name = kasprintf(GFP_KERNEL, ...);
    return 0;

The function then returns success (0) even though list->name is NULL.

This NULL pointer is propagated up through
drm_plane_create_color_pipeline_property(), and eventually passed to
drm_property_add_enum(), which does an unconditional strlen():

drm_property_add_enum()
    if (WARN_ON(strlen(name) >= DRM_PROP_NAME_LEN))

Would it be safer to check for a NULL return from kasprintf() and return
an error code like -ENOMEM?

[Severity: High]
This is also a pre-existing issue, but if a colorop fails to initialize
during the pipeline plane setup, can it cause a NULL pointer dereference 
during cleanup?

If an error occurs early in intel_color_pipeline_plane_add_colorop() 
(for example, if drm_mode_object_add() fails to allocate an ID), the 
cleanup path unconditionally invokes intel_colorop_destroy():

intel_color_pipeline_plane_add_colorop()
    ...
    cleanup:
        intel_colorop_destroy(&colorop->base);

This calls into drm_colorop_cleanup() with a partially initialized object 
where colorop->dev is still NULL, leading to a dereference:

drm_colorop_cleanup()
    struct drm_device *dev = colorop->dev;
    struct drm_mode_config *config = &dev->mode_config; 
    ...
    list_del(&colorop->head);

Since colorop->dev is NULL, &dev->mode_config will trigger a dereference, 
and list_del() will be called on a zeroed list_head. Could this error path 
be reached if there is resource exhaustion during plane initialization?

>  	if (ret)

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