Re: [PATCH 02/37] drm/connector: add drm_connector_hdmi_dynamic_init()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional dereference of connector->state if the reset callback fails to allocate memory.
--

commit 7cc84b3b3fd13ae7b12a391a46d924249c29ae3d
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>

drm/connector: add drm_connector_hdmi_dynamic_init()

This commit introduces a dynamic initialization variant for HDMI connectors
to support hotpluggable bridges.

> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 37ed73300a181..5e7a3a438e5db 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c

[ ... ]

> @@ -658,6 +658,56 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
> +int drm_connector_hdmi_dynamic_init(struct drm_device *dev,
> +				    struct drm_connector *connector,
> +				    const char *vendor, const char *product,
> +				    const struct drm_connector_funcs *funcs,
> +				    const struct drm_connector_hdmi_funcs *hdmi_funcs,
> +				    int connector_type,
> +				    struct i2c_adapter *ddc,
> +				    unsigned long supported_formats,
> +				    unsigned int max_bpc)
> +{
> +	int ret;
> +
> +	ret = drm_connector_hdmi_sanity_checks(connector, vendor, product, hdmi_funcs,
> +					       connector_type, supported_formats, max_bpc);
> +	if (ret)
> +		return ret;
> +
> +	ret = drm_connector_dynamic_init(dev, connector, funcs, connector_type, ddc);
> +	if (ret)
> +		return ret;
> +
> +	drm_connector_hdmi_init(connector, vendor, product,
> +				hdmi_funcs, supported_formats, max_bpc);

Could this lead to a NULL pointer dereference under memory pressure? I
realize this behavior pre-exists in drmm_connector_hdmi_init(), but the new
dynamic variant exposes the same pattern.

Looking at drm_connector_hdmi_init():

static void drm_connector_hdmi_init(...)
{
	...
	if (connector->funcs->reset)
		connector->funcs->reset(connector);

	drm_connector_attach_max_bpc_property(connector, 8, max_bpc);
}

For atomic drivers, the reset callback allocates connector->state, but since
it returns void, allocation failures are silent and connector->state remains
NULL.

The subsequent call to drm_connector_attach_max_bpc_property() then
unconditionally dereferences it:

	connector->state->max_requested_bpc = max;

Would it be safer to verify connector->state is allocated after the reset
callback before continuing?

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_hdmi_dynamic_init);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=2