Re: [PATCH v6 07/10] drm/amd/display: use drm backlight

[sashiko-bot@kernel.org]

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

New issues:
- [High] Redundant call to drm_connector_unregister() in the connector destroy hook triggers a kernel WARN_ON during driver unload.
- [Medium] LVDS connectors are missing the DRM backlight property allocation, leaving them without the new DRM-level brightness control capabilities.

Pre-existing issues:
- [High] Pre-existing error path Use-After-Free, Double Free, and memory leak when device initialization fails.
--

commit 1adb86b656e02ccab8cb2193136ef89ff549cddf
Author: Mario Limonciello (AMD) <superm1@kernel.org>
Subject: drm/amd/display: use drm backlight

This commit converts the AMD display driver to use the new DRM backlight
infrastructure to allow brightness control via DRM connector properties.
It also includes fixes for backlight resource lifecycle management.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

[ ... ]

> @@ -8152,6 +8163,10 @@ static void amdgpu_dm_connector_destroy(struct drm_connector *connector)
>  	aconnector->dc_sink = NULL;
>  
>  	drm_dp_cec_unregister_connector(&aconnector->dm_dp_aux.aux);
> +	/* DRM core will handle drm_backlight cleanup via:
> +	 * drm_connector_unregister() -> drm_backlight_unregister()
> +	 * drm_connector_cleanup() -> drm_backlight_free()
> +	 */
>  	drm_connector_unregister(connector);
>  	drm_connector_cleanup(connector);

[Severity: High]
Does this redundant unregister trigger a WARN_ON during driver unload?

When the driver is removed, drm_dev_unregister() automatically calls
drm_connector_unregister_all(), which unregisters the connector and its
newly added backlight object via drm_backlight_unregister().

Later, when the DRM device is cleaned up and the connector reference drops
to zero, amdgpu_dm_connector_destroy() is called. Because
drm_connector_unregister() calls drm_backlight_unregister() before checking
if the connector is already unregistered, it will attempt to unregister the
backlight a second time. This asserts a warning because the backlight was
already removed from the list.

[ ... ]

> @@ -9402,6 +9417,12 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
>  	aconnector->base.state->max_bpc = 16;
>  	aconnector->base.state->max_requested_bpc = aconnector->base.state->max_bpc;
>  
> +	if (connector_type == DRM_MODE_CONNECTOR_eDP) {
> +		int r = drm_backlight_alloc(&aconnector->base);
> +
> +		if (r)
> +			drm_err(dm->ddev, "Failed to allocate backlight: %d\n", r);
> +	}

[Severity: Medium]
Are LVDS connectors meant to be excluded from the new backlight allocation?

The AMD display driver also supports backlight control for legacy LVDS
panels, but drm_backlight_alloc() is explicitly restricted to eDP here.
Since it is skipped for LVDS, the connector's backlight pointer remains
NULL, and LVDS panels will not expose the new DRM luminance properties.

[ ... ]

While reviewing these changes, I also noticed an issue in the error handling
path of amdgpu_dm_initialize_drm_device() in the same file:

> fail:
> 	kfree(aencoder);
> 	kfree(aconnector);
>
> 	return -EINVAL;

[Severity: High]
This is a pre-existing issue, but can this error path lead to a
use-after-free and double-free?

If a late initialization step fails after a connector is successfully
created and registered into the global drm_device connector list, the
code jumps to the fail label and frees aconnector.

This leaves a dangling pointer in the global DRM connector list. During
the subsequent error rollback via drm_mode_config_cleanup(), the DRM core
will iterate over the connector list and invoke
amdgpu_dm_connector_destroy(), which accesses the freed memory and
attempts to free it again. Additionally, the newly added backlight
allocation would be leaked here.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624165751.2014759-1-mario.limonciello@amd.com?part=7