[PATCH] usb: typec: ucsi: glink: fix child node release in probe function

[PATCH] usb: typec: ucsi: glink: fix child node release in probe function

[Javier Carrasco]

The device_for_each_child_node() macro requires explicit calls to
fwnode_handle_put() in all early exits of the loop if the child node is
not required outside. Otherwise, the child node's refcount is not
decremented and the resource is not released.

The current implementation of pmic_glink_ucsi_probe() makes use of the
device_for_each_child_node(), but does not release the child node on
early returns. Add the missing calls to fwnode_handle_put().

Cc: stable@vger.kernel.org
Fixes: c6165ed2f425 ("usb: ucsi: glink: use the connector orientation GPIO to provide switch events")
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
This case would be a great opportunity for the recently introduced
device_for_each_child_node_scoped(), but given that it has not been
released yet, the traditional approach has been used to account for
stable kernels (bug introduced with v6.7). A second patch to clean
this up with that macro is ready to be sent once this fix is applied,
so this kind of problem does not arise if more early returns are added.

This issue has been found while analyzing the code and not tested with
hardware, only compiled and checked with static analysis tools. Any
tests with real hardware are always welcome.
---
 drivers/usb/typec/ucsi/ucsi_glink.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index f7546bb488c3..41375e0f9280 100644
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -372,6 +372,7 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
 		ret = fwnode_property_read_u32(fwnode, "reg", &port);
 		if (ret < 0) {
 			dev_err(dev, "missing reg property of %pOFn\n", fwnode);
+			fwnode_handle_put(fwnode);
 			return ret;
 		}
 
@@ -386,9 +387,11 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
 		if (!desc)
 			continue;
 
-		if (IS_ERR(desc))
+		if (IS_ERR(desc)) {
+			fwnode_handle_put(fwnode);
 			return dev_err_probe(dev, PTR_ERR(desc),
 					     "unable to acquire orientation gpio\n");
+		}
 		ucsi->port_orientation[port] = desc;
 	}
 

---
base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
change-id: 20240613-ucsi-glink-release-node-9fc09d81e138

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>

Re: [PATCH] usb: typec: ucsi: glink: fix child node release in probe function

[Dmitry Baryshkov]

On Thu, Jun 13, 2024 at 02:14:48PM +0200, Javier Carrasco wrote:
> The device_for_each_child_node() macro requires explicit calls to
> fwnode_handle_put() in all early exits of the loop if the child node is
> not required outside. Otherwise, the child node's refcount is not
> decremented and the resource is not released.
> 
> The current implementation of pmic_glink_ucsi_probe() makes use of the
> device_for_each_child_node(), but does not release the child node on
> early returns. Add the missing calls to fwnode_handle_put().
> 
> Cc: stable@vger.kernel.org
> Fixes: c6165ed2f425 ("usb: ucsi: glink: use the connector orientation GPIO to provide switch events")
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> This case would be a great opportunity for the recently introduced
> device_for_each_child_node_scoped(), but given that it has not been
> released yet, the traditional approach has been used to account for
> stable kernels (bug introduced with v6.7). A second patch to clean
> this up with that macro is ready to be sent once this fix is applied,
> so this kind of problem does not arise if more early returns are added.
> 
> This issue has been found while analyzing the code and not tested with
> hardware, only compiled and checked with static analysis tools. Any
> tests with real hardware are always welcome.
> ---
>  drivers/usb/typec/ucsi/ucsi_glink.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


-- 
With best wishes
Dmitry

Re: [PATCH] usb: typec: ucsi: glink: fix child node release in probe function

[Heikki Krogerus]

On Thu, Jun 13, 2024 at 02:14:48PM +0200, Javier Carrasco wrote:
> The device_for_each_child_node() macro requires explicit calls to
> fwnode_handle_put() in all early exits of the loop if the child node is
> not required outside. Otherwise, the child node's refcount is not
> decremented and the resource is not released.
> 
> The current implementation of pmic_glink_ucsi_probe() makes use of the
> device_for_each_child_node(), but does not release the child node on
> early returns. Add the missing calls to fwnode_handle_put().
> 
> Cc: stable@vger.kernel.org
> Fixes: c6165ed2f425 ("usb: ucsi: glink: use the connector orientation GPIO to provide switch events")
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> This case would be a great opportunity for the recently introduced
> device_for_each_child_node_scoped(), but given that it has not been
> released yet, the traditional approach has been used to account for
> stable kernels (bug introduced with v6.7). A second patch to clean
> this up with that macro is ready to be sent once this fix is applied,
> so this kind of problem does not arise if more early returns are added.
> 
> This issue has been found while analyzing the code and not tested with
> hardware, only compiled and checked with static analysis tools. Any
> tests with real hardware are always welcome.
> ---
>  drivers/usb/typec/ucsi/ucsi_glink.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index f7546bb488c3..41375e0f9280 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -372,6 +372,7 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
>  		ret = fwnode_property_read_u32(fwnode, "reg", &port);
>  		if (ret < 0) {
>  			dev_err(dev, "missing reg property of %pOFn\n", fwnode);
> +			fwnode_handle_put(fwnode);
>  			return ret;
>  		}
>  
> @@ -386,9 +387,11 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
>  		if (!desc)
>  			continue;
>  
> -		if (IS_ERR(desc))
> +		if (IS_ERR(desc)) {
> +			fwnode_handle_put(fwnode);
>  			return dev_err_probe(dev, PTR_ERR(desc),
>  					     "unable to acquire orientation gpio\n");
> +		}
>  		ucsi->port_orientation[port] = desc;
>  	}
>  
> 
> ---
> base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
> change-id: 20240613-ucsi-glink-release-node-9fc09d81e138
> 
> Best regards,
> -- 
> Javier Carrasco <javier.carrasco.cruz@gmail.com>

-- 
heikki