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 <[email protected]>

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/[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);
> +             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);

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

Reply via email to