Re: [PATCH 3/5] drm/bridge: chrontel-ch7033: replace deprecated DRM_ERROR macro

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing cleanup on error paths in `ch7033_bridge_attach()` causes a resource leak and use-after-free.
- [High] `ch7033_bridge_detach()` incorrectly cleans up uninitialized resources if the bridge was attached with `DRM_BRIDGE_ATTACH_NO_CONNECTOR`.
- [High] HPD is enabled before connector initialization is complete, causing a NULL pointer dereference race condition.
--

commit 0d74a63f280c9b3d909b1d5c59227a795c81f570
Author: Piyush Patle <piyushpatle228@gmail.com>
drm/bridge: chrontel-ch7033: replace deprecated DRM_ERROR macro

Replace DRM_ERROR() with device-aware drm_err() using bridge->dev.

No functional changes.

> diff --git a/drivers/gpu/drm/bridge/chrontel-ch7033.c b/drivers/gpu/drm/bridge/chrontel-ch7033.c
> index 54d49d4882c85..0e845f789f2e9 100644
> --- a/drivers/gpu/drm/bridge/chrontel-ch7033.c
> +++ b/drivers/gpu/drm/bridge/chrontel-ch7033.c
> @@ -302,7 +302,7 @@ static int ch7033_bridge_attach(struct drm_bridge *bridge,
>  					  priv->next_bridge->type,
>  					  priv->next_bridge->ddc);
>  	if (ret) {
> -		DRM_ERROR("Failed to initialize connector\n");
> +		drm_err(bridge->dev, "Failed to initialize connector\n");
>  		return ret;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does this early return
leak resources?

If drm_connector_init_with_ddc() fails here in ch7033_bridge_attach(),
we return without calling drm_bridge_hpd_disable(). Does this leave a
dangling pointer in next_bridge for the HPD callback (ch7033_hpd_event)
and its data (priv)?

If an HPD event fires later after the device probing failed, could it
lead to a use-after-free?

Also, if the subsequent drm_connector_attach_encoder() call fails:

drivers/gpu/drm/bridge/chrontel-ch7033.c:ch7033_bridge_attach() {
    ...
    if (ret) {
        drm_err(bridge->dev, "Failed to initialize connector\n");
        return ret;
    }

    return drm_connector_attach_encoder(&priv->connector, encoder);
}

Does returning that error directly fail to roll back
drm_connector_init_with_ddc() and leave the HPD enabled, leaking the
connector structure and the registered callback?

[Severity: High]
This is also a pre-existing issue, but is there a race condition with when
HPD is enabled in this same function?

Looking at ch7033_bridge_attach() in drivers/gpu/drm/bridge/chrontel-ch7033.c:

drivers/gpu/drm/bridge/chrontel-ch7033.c:ch7033_bridge_attach() {
    ...
    if (priv->next_bridge->ops & DRM_BRIDGE_OP_HPD) {
        drm_bridge_hpd_enable(priv->next_bridge, ch7033_hpd_event, priv);
    }

    drm_connector_helper_add(connector, &ch7033_connector_helper_funcs);
    ret = drm_connector_init_with_ddc(...);
    ...
}

Since drm_bridge_hpd_enable() is called before drm_connector_init_with_ddc(),
if an HPD event fires immediately, ch7033_hpd_event() is invoked.
That callback checks if priv->bridge.dev is set (which is true here)
and then passes priv->connector.dev to drm_helper_hpd_irq_event().

Since priv->connector.dev is only populated later during
drm_connector_init_with_ddc(), will it still be NULL here, causing
drm_helper_hpd_irq_event() to dereference a NULL pointer?

[Severity: High]
Finally, this is another pre-existing issue, but does ch7033_bridge_detach()
clean up uninitialized resources?

If ch7033_bridge_attach() is called with DRM_BRIDGE_ATTACH_NO_CONNECTOR,
it returns early and skips drm_connector_init_with_ddc().
However, ch7033_bridge_detach() unconditionally calls:

drivers/gpu/drm/bridge/chrontel-ch7033.c:ch7033_bridge_detach() {
    ...
    drm_connector_cleanup(&priv->connector);
}

Since priv is allocated with devm_drm_bridge_alloc() (which zeroes memory),
priv->connector contains zero-initialized list heads.
Because drm_connector_cleanup() iterates over &connector->probed_modes
using list_for_each_entry_safe(), could this dereference NULL list
pointers and cause a kernel crash when the uninitialized connector is
cleaned up?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260606193625.24465-1-piyushpatle228@gmail.com?part=3