Re: [PATCH RESEND] drm/connector: check if destroy function exist

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Manual invocation of `funcs->destroy()` causes a NULL pointer dereference on the error path in `msm_hdmi_modeset_init`.
--

commit aa404d4746f85dae926593fae989fac53f064b44
Author: Andreas Kemnade <andreas@kemnade.info>

drm/connector: check if destroy function exist

Connectors might have not resources needed to be manually freed.
E.g. the drm_bridge_connector does not have such a callback.

> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 8b4baed060f3a..288d9d6c75a8d 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -198,7 +198,9 @@ static void drm_connector_free(struct kref *kref)
>  	struct drm_device *dev = connector->dev;
>  
>  	drm_mode_object_unregister(dev, &connector->base);
> -	connector->funcs->destroy(connector);
> +
> +	if (connector->funcs->destroy)
> +		connector->funcs->destroy(connector);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but could the same NULL pointer
dereference happen in the error path of msm_hdmi_modeset_init() when it
manually cleans up the connector?

If devm_request_irq() fails after the drm_bridge_connector is successfully
initialized, it seems the failure path blindly calls the destroy callback:

drivers/gpu/drm/msm/hdmi/hdmi.c:msm_hdmi_modeset_init() {
	...
	hdmi->connector = drm_bridge_connector_init(hdmi->dev, encoder);
	if (IS_ERR(hdmi->connector)) {
		...
	}

	ret = devm_request_irq(dev->dev, hdmi->irq, ...);
	if (ret < 0) {
		DRM_DEV_ERROR(dev->dev, "failed to request IRQ%u: %d\n",
				hdmi->irq, ret);
		goto fail;
	}

	return 0;

fail:
	if (hdmi->connector) {
		hdmi->connector->funcs->destroy(hdmi->connector);
		hdmi->connector = NULL;
	}

	return ret;
}

Should this driver-specific cleanup path also be updated to check if
funcs->destroy is present, similar to the fix applied here in
drm_connector_free?

>  
>  void drm_connector_free_work_fn(struct work_struct *work)
[ ... ]
> @@ -216,7 +218,9 @@ void drm_connector_free_work_fn(struct work_struct *work)
>  
>  	llist_for_each_entry_safe(connector, n, freed, free_node) {
>  		drm_mode_object_unregister(dev, &connector->base);
> -		connector->funcs->destroy(connector);
> +
> +		if (connector->funcs->destroy)
> +			connector->funcs->destroy(connector);
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702191453.611070-1-andreas@kemnade.info?part=1