Re: [PATCH v7 6/7] drm/rockchip: cdn-dp: Support handle lane info without extcon

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] The condition `PTR_ERR(extcon) != -ENODEV` incorrectly skips valid extcon pointers during probe, breaking backward compatibility for all existing devices with an extcon.

Pre-existing issues:
- [High] Extcon notifier registered using devres in component bind is not unregistered in component unbind, leading to Use-After-Free of released firmware.
- [High] Using devm_drm_bridge_add in component bind causes duplicate bridge registrations and devres corruption upon re-bind.
--

commit 30782eee380f850a10d083003cae3953c1e3c8d7
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>

drm/rockchip: cdn-dp: Support handle lane info without extcon

This commit adds support for retrieving PHY lane information without
relying on extcon. This is useful for setups where a Type-C controller
is present and lane info can be acquired directly from the PHY.

> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index 190cce9530c82..74e1f5422a6e2 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c

[ ... ]

> @@ -1027,6 +1033,9 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>  	for (i = 0; i < dp->ports; i++) {
>  		port = dp->port[i];
>  
> +		if (!port->extcon)
> +			continue;
> +
>  		port->event_nb.notifier_call = cdn_dp_pd_event;
>  		ret = devm_extcon_register_notifier(dp->dev, port->extcon,
>  						    EXTCON_DISP_DP,

[Severity: High]
This is a pre-existing issue, but does using devres for the notifier here
cause a use-after-free upon component unbind?

Because devm_extcon_register_notifier() is tied to the platform device's
remove lifecycle rather than component unbind, the notifier remains active
after cdn_dp_unbind() completes. Since cdn_dp_unbind() frees the firmware
via release_firmware(), a subsequent extcon state change could trigger the
still-active notifier callback.

Could this lead to cdn_dp_pd_event_work() calling cdn_dp_firmware_init(),
which then dereferences the freed firmware data?

[ ... ]

> @@ -1119,14 +1128,14 @@ static int cdn_dp_probe(struct platform_device *pdev)
>  		    PTR_ERR(phy) == -EPROBE_DEFER)
>  			return -EPROBE_DEFER;
>  
> -		if (IS_ERR(extcon) || IS_ERR(phy))
> +		if (IS_ERR(phy) || PTR_ERR(extcon) != -ENODEV)
>  			continue;

[Severity: Critical]
Does this condition inadvertently skip valid extcon pointers during probe?

When extcon_get_edev_by_phandle() returns a valid kernel pointer,
PTR_ERR(extcon) casts that valid address to a long. Since a valid pointer
will never equal -ENODEV, the condition PTR_ERR(extcon) != -ENODEV will
evaluate to true.

Will this completely disable port allocation for existing setups with a
valid extcon, resulting in a loss of display output?

[ ... ]

This is a pre-existing issue, but I also noticed a potential problem in
cdn_bridge_add():

drivers/gpu/drm/rockchip/cdn-dp-core.c:cdn_bridge_add() {
        ...
        ret = devm_drm_bridge_add(dev, bridge);
        if (ret)
                return ret;
        ...
}

[Severity: High]
Does using devm_drm_bridge_add() in a component bind hook cause duplicate
bridge registrations?

Because devres actions only execute on full driver remove, unbinding and
rebinding the DRM component via sysfs appears to queue duplicate devres
cleanup actions. This adds the exact same bridge structure to the global
bridge_list repeatedly. 

Could this corrupt the bridge list and lead to a double-free on removal?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828084737.565-1-kernel@airkyi.com?part=6