diff mbox series

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

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

Commit Message

Cristian Ciocaltea Sept. 1, 2026, 6:50 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/
Link: https://lore.kernel.org/all/20260723015306.C94241F000E9@smtp.kernel.org/
Reviewed-by: Maxime Ripard <mripard@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 Sept. 1, 2026, 7:07 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The newly added NULL check for `connector->state` in `drmm_connector_hdmi_init()` fails to prevent crashes on memory allocation failure for main HDMI drivers (e.g., `vc4`, `sun4i`) and the testing infrastructure, because their `reset` hooks unconditionally dereference the state before returning.
--

commit 424f1b968bca06b1916973328efd3feb5a119e0e
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

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

This commit adds a check for connector->state after calling the reset
callback in drmm_connector_hdmi_init() to avoid a NULL pointer dereference
if memory allocation fails.

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);
> +		if (!connector->state)
> +			return -ENOMEM;

[Severity: High]
This isn't a bug introduced by this patch, but doesn't the system crash
before it can even reach this new check on memory allocation failure?

If drm_atomic_helper_connector_reset() fails to allocate memory, it sets
connector->state to NULL. Several driver-specific reset hooks then
immediately dereference it by unconditionally passing it to
__drm_atomic_helper_connector_hdmi_state_init().

For example, in drivers/gpu/drm/vc4/vc4_hdmi.c:vc4_hdmi_connector_reset():

	drm_atomic_helper_connector_reset(connector);
	__drm_atomic_helper_connector_hdmi_state_init(connector, connector->state);

The sun4i driver has the exact same pattern in sun4i_hdmi_connector_reset(),
and the testing infrastructure will also panic the KUnit framework in
dummy_hdmi_connector_reset() from
drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c under the same conditions.

Could these driver reset hooks be updated to handle the NULL state gracefully,
so that this new check can successfully prevent the panic?

>  	}
>  
>  	drm_connector_attach_max_bpc_property(connector, 8, max_bpc);
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);