Re: [PATCH v8 4/5] drm/bridge: analogix_dp: Add validation for samsung,lane-count property

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] devm_drm_bridge_add() is used inside a component bind callback, causing devres leakage and double-add on re-bind.
- [High] Missing analogix_dp_unbind() and encoder cleanup in component bind error paths.
--

commit 3586f9db6c3c7447ea980ac18be62fa966e709ae
Author: Damon Ding <damon.ding@rock-chips.com>

drm/bridge: analogix_dp: Add validation for samsung,lane-count property

Add validity check for samsung,lane-count to ensure DT-provided
lane count values are specification-compliant.

> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 4213cd68d5a0d..ba12fde8accde 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c

[ ... ]

This isn't a bug introduced by this patch, but I noticed an issue in the
analogix_dp_bind() component bind flow:

drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:analogix_dp_bind() {
	...
	bridge->type = DRM_MODE_CONNECTOR_eDP;
	ret = devm_drm_bridge_add(dp->dev, &dp->bridge);
	if (ret)
		goto err_unregister_aux;
	...
}

[Severity: High]
Since devm_drm_bridge_add() is called in a component bind function, does it
tie the bridge registration lifetime to the platform device rather than the
component? 

If the master DRM device defers, unbinds, and later rebinds,
analogix_dp_bind() executes again. Because the devres actions are not
cleaned up during component unbind, would this trigger duplicate devres
additions, list corruption via double list_add_tail() in drm_bridge_add(),
and double initialization of the bridge's hpd_mutex?

This is also a pre-existing issue, but I noticed the error paths in the
component bind functions seem to be missing cleanup steps:

drivers/gpu/drm/rockchip/analogix_dp-rockchip.c:rockchip_dp_bind() {
	...
	connector = drm_bridge_connector_init(dp->drm_dev,
					      dp->plat_data.encoder);
	if (IS_ERR(connector)) {
		ret = PTR_ERR(connector);
		dev_err(dp->dev, "Failed to initialize bridge_connector\n");
		goto err_cleanup_encoder;
	}

	return 0;
err_cleanup_encoder:
	dp->encoder.encoder.funcs->destroy(&dp->encoder.encoder);
	return ret;
}

[Severity: High]
If drm_bridge_connector_init() fails, does this error path skip calling
analogix_dp_unbind()? 

This would leave the DP AUX device registered. If component binding is
retried, would drm_dp_aux_register() be called again on the same device,
causing a sysfs duplicate name error and preventing the display from ever
initializing?

A similar issue appears in exynos_dp.c:

drivers/gpu/drm/exynos/exynos_dp.c:exynos_dp_bind() {
	...
	connector = drm_bridge_connector_init(dp->drm_dev,
					      dp->plat_data.encoder);
	if (IS_ERR(connector)) {
		ret = PTR_ERR(connector);
		dev_err(dp->dev, "Failed to initialize bridge_connector\n");
		return ret;
	}

	return 0;
}

[Severity: High]
If drm_bridge_connector_init() fails here, does the error path miss cleaning
up the resources by skipping both analogix_dp_unbind() and the encoder
destruction?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828065153.590802-1-damon.ding@rock-chips.com?part=4