diff mbox series

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

Message ID 20260731-dw-hdmi-qp-scramb-v10-2-294364b2cf15@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea July 31, 2026, 4:19 p.m. UTC 
In drmm_connector_hdmi_init(), if the reset callback fails to allocate
memory, e.g. inside drm_atomic_helper_connector_reset(),
connector->state will remain NULL.  This is unconditionally passed to
drm_connector_attach_max_bpc_property(), leading to a NULL pointer
dereference.

Bail out with -ENOMEM if the reset callback leaves connector->state
unset.

Fixes: aadb3e16b8f3 ("drm/connector: hdmi: Add output BPC to the connector state")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260723015306.C94241F000E9@smtp.kernel.org/
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/drm_connector.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

sashiko-bot@kernel.org July 31, 2026, 4:37 p.m. UTC | #1 
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 <cristian.ciocaltea@collabora.com>

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/20260723015306.C94241F000E9@smtp.kernel.org/

> 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);
Maxime Ripard Aug. 13, 2026, 1:45 p.m. UTC | #2 
On Fri, 31 Jul 2026 19:19:09 +0300, Cristian Ciocaltea wrote:
> In drmm_connector_hdmi_init(), if the reset callback fails to allocate
> memory, e.g. inside drm_atomic_helper_connector_reset(),
> connector->state will remain NULL.  This is unconditionally passed to
> drm_connector_attach_max_bpc_property(), leading to a NULL pointer
> dereference.
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 8b4baed060f3..0e39431e85b2 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);
+		if (!connector->state)
+			return -ENOMEM;
 	}
 
 	drm_connector_attach_max_bpc_property(connector, 8, max_bpc);