Re: [PATCH v4 1/5] drm/bridge: Implement generic USB Type-C DP HPD bridge

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Module load/unload cycles and dynamic Type-C port removals leak devm-managed bridges, resulting in duplicates and potential UAF.
- [High] A race condition between bus notifier registration and bus iteration causes duplicate DP HPD bridge registrations.
- [High] Unconditional bridge registration creates duplicate bridges for controllers that already register their own HPD bridge.
--

--- Patch [1]: [PATCH v4 1/5] drm/bridge: Implement generic USB Type-C DP HPD bridge ---
commit 8ca96a8d5692c2b3ff148f7a6803323281488779
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>

drm/bridge: Implement generic USB Type-C DP HPD bridge

This commit implements a generic DP HPD bridge based on aux-hpd-bridge.c.
It monitors Type-C bus events and automatically creates an HPD bridge
when a Type-C port device containing the DP svid is registered.

> diff --git a/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
> new file mode 100644
> index 0000000000000..43af3ea20f201
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c

[ ... ]

> +static int drm_typec_bus_event(struct notifier_block *nb,
> +			       unsigned long action, void *data)
> +{
> +	struct device *dev = (struct device *)data;
> +	struct typec_altmode *alt = to_typec_altmode(dev);
> +
> +	if (action != BUS_NOTIFY_ADD_DEVICE)
> +		return NOTIFY_OK;

[Severity: High]
Does this code leak devm-managed bridges when a Type-C altmode device is
dynamically removed? 

It looks like BUS_NOTIFY_DEL_DEVICE is not handled here, so the bridge might
linger and pin of_node references indefinitely after port removal.

> +
> +	/*
> +	 * alt->dev.parent->parent : USB-C controller device
> +	 * alt->dev.parent         : USB-C connector device
> +	 */
> +	if (is_typec_port_altmode(&alt->dev) && alt->svid == USB_TYPEC_DP_SID)
> +		drm_dp_hpd_bridge_register(alt->dev.parent->parent,
> +					   to_of_node(alt->dev.parent->fwnode));

[Severity: High]
What happens if the USB-C controller driver (like fusb302 or qcom_pmic_typec)
has already explicitly registered its own HPD bridge?

Will this unconditional registration create a second duplicate bridge? If so,
could this break of_drm_find_bridge() resolution, causing the display driver
to miss hotplug events fired on the controller's own bridge?

Additionally, because drm_dp_hpd_bridge_register() ties the devm allocation
to the external USB-C controller (alt->dev.parent->parent), will unloading
this module leave those devres allocations behind? Reloading the module might
then register duplicates.

> +
> +	return NOTIFY_OK;
> +}

[ ... ]

> +static int __init drm_aux_hpd_typec_dp_bridge_module_init(void)
> +{
> +	bus_register_notifier(&typec_bus, &drm_typec_event_nb);
> +	/*
> +	 * Before module initialization, some devices may have already been added.
> +         * Register the HPD bridge for these devices.
> +	 */
> +	bus_for_each_dev(&typec_bus, NULL, NULL, check_device_already_added);

[Severity: High]
Is there a race condition between the notifier registration and the bus
iteration here?

If a Type-C altmode device is added concurrently (for example, via an async
probe of the controller) right after bus_register_notifier() but before or
during the bus_for_each_dev() iteration, could it trigger both the notifier
and the check_device_already_added callback?

Would this result in duplicate HPD bridges being registered for a single
Type-C port?

> +	return 0;
> +}

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy