Re: [PATCH v10 02/69] drm/connector: hdmi: Handle reset() state allocation failure

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

Pre-existing issues:
- [High] Custom connector reset callbacks and helpers crash on allocation failure before the newly added NULL check can execute.
--

--- Patch [2]: [PATCH v10 02/69] drm/connector: hdmi: Handle reset() state allocation failure ---
commit 4612d8e6373873b0631394a087aa684911dafc2f
Author: Cristian Ciocaltea <[email protected]>

drm/connector: hdmi: Handle reset() state allocation failure

This commit adds a check for memory allocation failure during the connector
reset callback in drmm_connector_hdmi_init(). If connector->state remains
NULL after reset(), it bails out with -ENOMEM to avoid a subsequent NULL
pointer dereference.

Link: https://lore.kernel.org/all/[email protected]/

> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 8b4baed060f3a..0e39431e85b20 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -628,6 +628,8 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
>  		connector->state = state;
>  	} else if (connector->funcs->reset) {
>  		connector->funcs->reset(connector);

[Severity: High]
This is a pre-existing issue, but does this patch fully prevent the NULL
pointer dereference?

If memory allocation fails inside the reset callback, connector->state
becomes NULL. However, custom reset callbacks like vc4_hdmi_connector_reset()
and sun4i_hdmi_connector_reset() pass this state directly to
__drm_atomic_helper_connector_hdmi_state_init():

drivers/gpu/drm/vc4/vc4_hdmi.c:vc4_hdmi_connector_reset() {
    ...
    __drm_atomic_helper_connector_hdmi_state_init(connector, connector->state);
    ...
}

That helper unconditionally dereferences new_conn_state:

drivers/gpu/drm/display/drm_hdmi_state_helper.c:__drm_atomic_helper_connector_hdmi_state_init() {
    ...
    new_conn_state->max_bpc = max_bpc;
    ...
}

Could this trigger a kernel panic inside the custom reset callback before
it even returns here to be caught by the new check?

> +		if (!connector->state)
> +			return -ENOMEM;
>  	}
>  
>  	drm_connector_attach_max_bpc_property(connector, 8, max_bpc);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2