Re: [CI v4 09/14] drm/i915/color: Add support for 1D LUT in 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] NULL pointer dereference and list corruption when tearing down a partially initialized colorop.
- [High] Missing `drm_mode_object_unregister()` in `drm_colorop_cleanup()` leaks IDR entries and causes Use-After-Free.
--

--- Patch [9]: [CI v4 09/14] drm/i915/color: Add support for 1D LUT in SDR planes ---
commit 774f8cae80cb667cc0804c605f7a0135e70d21fa
Author: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

drm/i915/color: Add support for 1D LUT in SDR planes

Extend the SDR plane color pipeline to include pre- and post-CSC
1D LUT blocks.

SDR planes use a smaller LUT size than HDR planes and therefore
initialize the 1D LUT colorops with the appropriate hardware
capacity.

> diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> index 7457fa51cb97a..136b9378ca484 100644
> --- a/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
[ ... ]
> @@ -81,7 +86,9 @@ struct intel_colorop *intel_color_pipeline_plane_add_colorop(struct drm_plane *p
>  		ret = drm_plane_colorop_curve_1d_lut_init(dev,
>  							  &colorop->base, plane,
>  							  &intel_colorop_funcs,
> -							  PLANE_DEGAMMA_SIZE,
> +							  is_hdr ?
> +							  PLANE_DEGAMMA_SIZE :
> +							  PLANE_DEGAMMA_SIZE_SDR,
>  							  DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
>  							  DRM_COLOROP_FLAG_ALLOW_BYPASS);
>  		break;

[Severity: High]
This is a pre-existing issue, but can the error path for this initialization
lead to a null pointer dereference?

If drm_plane_colorop_curve_1d_lut_init() fails early (for example, if memory
allocation in drm_mode_object_add() fails), colorop->dev remains null and the
object is never added to the list.

The error path in this function unconditionally goes to cleanup and calls
intel_colorop_destroy(), which eventually calls drm_colorop_cleanup():

drivers/gpu/drm/drm_colorop.c:drm_colorop_cleanup() {
	struct drm_device *dev = colorop->dev;
	struct drm_mode_config *config = &dev->mode_config;

	list_del(&colorop->head);
	...
}

Will this dereference the null colorop->dev and corrupt the list by calling
list_del() on an uninitialized list head?


[Severity: High]
This is another pre-existing issue, but does the cleanup path leave a dangling
pointer in the DRM mode object IDR?

If drm_plane_colorop_curve_1d_lut_init() successfully adds the colorop to the
device's mode object IDR via drm_mode_object_add(), but a subsequent property
creation fails (like drm_property_create_enum()), the cleanup path is
triggered.

The error path calls drm_colorop_cleanup() and subsequently frees the colorop
object. However, drm_colorop_cleanup() does not seem to call
drm_mode_object_unregister():

drivers/gpu/drm/drm_colorop.c:drm_colorop_cleanup() {
	struct drm_device *dev = colorop->dev;
	struct drm_mode_config *config = &dev->mode_config;

	list_del(&colorop->head);
	config->num_colorop--;

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

Could this result in a use-after-free if the IDR continues to hold a pointer
to the freed memory?

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