Re: [RFC PATCH 4/8] drm/amd/colorop: Update to new pipeline API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing NULL check for kasprintf() when setting the color pipeline name leads to a potential NULL pointer dereference in strlen().
- [High] Calling list_del on an uninitialized list head during error cleanup causes a kernel panic.
--

commit 3a2ecacea83aea676a95a30af6588c8a29a29749
Author: John Harrison <John.Harrison@Igalia.com>

drm/amd/colorop: Update to new pipeline API

Switch to using the new method for building colorop pipelines.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> index 48f5c431eaf91..6b5741d9d98b0 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
[ ... ]
> @@ -210,7 +212,7 @@ amdgpu_dm_build_default_pipeline(struct drm_device *dev, struct drm_plane *plane
>  	if (ret)
>  		goto cleanup;
>  
> -	drm_colorop_set_next_property(ops[i-1], ops[i]);
> +	drm_colorop_add_to_pipeline(ops[0], ops[i]);
>  
>  	list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[0]->base.id);

[Severity: High]
This isn't a bug introduced by this patch, but does this kasprintf() need a
NULL check?

If the memory allocation in kasprintf() fails, it returns a NULL pointer.
This NULL pointer is blindly assigned to list->name and then passed down to
drm_plane_create_color_pipeline_property() and eventually to
drm_property_add_enum(), which attempts to compute its length:

drivers/gpu/drm/drm_property.c:drm_property_add_enum() {
	...
	if (WARN_ON(strlen(name) >= DRM_PROP_NAME_LEN))
		return -EINVAL;
	...
}

Calling strlen(NULL) causes an immediate kernel panic during device
initialization. (This same pattern also appears in the Intel and VKMS
drivers' color pipeline initialization).

[Severity: High]
This is another pre-existing issue, but can calling list_del() on an
uninitialized list head during error cleanup cause a kernel panic?

When an error occurs early in color pipeline initialization (e.g.,
drm_mode_object_add() fails), the struct drm_colorop is zero-allocated but
never added to the global colorop_list. The error handling path in the Intel
driver unconditionally calls intel_colorop_destroy(), which delegates to
drm_colorop_cleanup():

drivers/gpu/drm/drm_colorop.c:drm_colorop_cleanup() {
	...
	list_del(&colorop->head);
	...
}

Because the list_head was never initialized with INIT_LIST_HEAD nor linked,
its prev pointer is NULL. The internal __list_del macro dereferences this
NULL pointer, causing an immediate kernel panic on driver probe failure. Is
this expected behavior?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609171941.169104-1-John.Harrison@Igalia.com?part=4