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