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