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 <[email protected]> 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/[email protected]?part=7
