Re: [PATCH v3 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] The module registers a bus notifier to catch device additions but fails to iterate over already-registered devices, ignoring any Type-C ports present before the module was loaded.
- [High] The module attaches devm-managed bridges to a foreign USB-C controller device but fails to clean them up on module unload or device removal, leading to resource leaks and an execute-after-free kernel panic.
- [Medium] The return value of `bus_register_notifier()` is ignored.
--

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

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

This patch 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
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
> @@ -0,0 +1,54 @@
[ ... ]
> +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;
> +
> +	/*
> +	 * 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]
Will this leak devm resources on the controller device when a port is removed
and re-added, since BUS_NOTIFY_DEL_DEVICE is ignored?

Also, because the devm cleanup callbacks are tied to the controller device
rather than this module, could this cause an execute-after-free panic?
If this module and aux-hpd-bridge are unloaded, the devres callbacks on the
controller will point to unmapped memory when the controller is later unbound.

[ ... ]
> +static int __init drm_aux_hpd_typec_dp_bridge_module_init(void)
> +{
> +	bus_register_notifier(&typec_bus, &drm_typec_event_nb);

[Severity: Medium]
Is it safe to ignore the return value of bus_register_notifier() here?

If the notifier registration fails, the module initialization will succeed but
remain non-functional.

[Severity: High]
Does this code miss existing Type-C DP ports that were probed before this
module was loaded?

bus_register_notifier() only triggers notifications for devices added after
registration. Since there is no bus_for_each_dev() iteration over existing
devices on the typec_bus after registering the notifier, pre-existing ports
might never get an HPD bridge created.

> +
> +	return 0;
> +}

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

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