Re: [PATCH v10 05/11] drm/rockchip: analogix_dp: Enable hclk for RK3588

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing analogix_dp_unbind() on error path in rockchip_dp_bind() causes resource leak and Use-After-Free risk.
- [High] Use-after-free risk via unreferenced device node pointer `endpoint.local_node`.
--

commit 9a213026e9dc13c601798dee6df75e3fbf1fabb3
Author: Damon Ding <damon.ding@rock-chips.com>

drm/rockchip: analogix_dp: Enable hclk for RK3588

Acquire and enable the HCLK_VO1 bus clock explicitly for RK3588
eDP controller to guarantee register and datapath access.

> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 06072efd7fca3..d2af5eb29dbb5 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but does rockchip_dp_bind() leak the AUX device
registration on this error path?

drivers/gpu/drm/rockchip/analogix_dp-rockchip.c:rockchip_dp_bind() {
...
err_cleanup_encoder:
	dp->encoder.encoder.funcs->destroy(&dp->encoder.encoder);
	return ret;
}

If analogix_dp_bind() succeeds but the subsequent drm_bridge_connector_init()
fails, the code jumps to err_cleanup_encoder. This path destroys the encoder
but fails to call analogix_dp_unbind(dp->adp).

Because analogix_dp_bind() successfully registered the DRM DP AUX device,
failing to unregister it leaves the adapter active in the I2C subsystem. If
the driver is unbound, devm will free the memory containing the drm_dp_aux
structure, potentially leading to a use-after-free crash in the I2C subsystem
during any subsequent access.

[Severity: High]
This isn't a bug introduced by this patch, but is there a use-after-free risk
when accessing endpoint.local_node?

drivers/gpu/drm/rockchip/analogix_dp-rockchip.c:rockchip_dp_drm_encoder_enable() {
...
	ret = drm_of_encoder_active_endpoint(dp->dev->of_node, encoder, &endpoint);
	if (ret < 0)
		return;

	remote_port_parent = of_graph_get_remote_port_parent(endpoint.local_node);
...
}

Inside drm_of_encoder_active_endpoint(), of_node_put() drops the reference
on the local endpoint node before returning.

Since the reference is already dropped, could passing endpoint.local_node to
of_graph_get_remote_port_parent() result in a use-after-free if a device tree
overlay containing the node is removed concurrently?

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