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