Re: [PATCH v6 4/7] drm/bridge: aux: Add drm_aux_bridge_register_from_node()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm_aux_bridge_register_from_node() assigns the provided device node without taking a reference, leading to a reference count underflow when the device is released or initialization fails.
--

--- Patch [4]: [PATCH v6 4/7] drm/bridge: aux: Add drm_aux_bridge_register_from_node() ---
commit 589f6bc20e86e41fcd3fb17c01ef52cecefa8a82
Author: Chaoyi Chen <[email protected]>

drm/bridge: aux: Add drm_aux_bridge_register_from_node()

This commit introduces drm_aux_bridge_register_from_node() to allow
specifying the of_node corresponding to the bridge, extending the
existing drm_aux_bridge_register() functionality.

> diff --git a/drivers/gpu/drm/bridge/aux-bridge.c b/drivers/gpu/drm/bridge/aux-bridge.c
> index 1ed21a8713bfd..06a1466f49bd2 100644
> --- a/drivers/gpu/drm/bridge/aux-bridge.c
> +++ b/drivers/gpu/drm/bridge/aux-bridge.c
[ ... ]
> @@ -62,7 +63,10 @@ int drm_aux_bridge_register(struct device *parent)
>  	adev->dev.parent = parent;
>  	adev->dev.release = drm_aux_bridge_release;
>  
> -	device_set_of_node_from_dev(&adev->dev, parent);
> +	if (np)
> +		device_set_node(&adev->dev, of_fwnode_handle(np));

[Severity: High]
Does this code need to take a reference on np before setting it?

Looking at device_set_of_node_from_dev(), it correctly acquires a reference
using of_node_get(). However, device_set_node() only assigns the pointer
without taking a reference.

If initialization fails, the error path unconditionally drops a reference:

	ret = auxiliary_device_init(adev);
	if (ret) {
		of_node_put(adev->dev.of_node);
		...

Additionally, drm_aux_bridge_release() unconditionally calls of_node_put()
when the device is released.

Could assigning the node here without of_node_get() cause a reference count
underflow and a potential use-after-free for the provided node?

> +	else
> +		device_set_of_node_from_dev(&adev->dev, parent);
>  
>  	ret = auxiliary_device_init(adev);
>  	if (ret) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy