[PATCH v3] usb: typec: tcpci: fix of node refcount leak in tcpci_register_port()

[PATCH v3] usb: typec: tcpci: fix of node refcount leak in tcpci_register_port()

[Yang Yingliang]

I got the following report while doing device(mt6370-tcpc) load
test with CONFIG_OF_UNITTEST and CONFIG_OF_DYNAMIC enabled:

  OF: ERROR: memory leak, expected refcount 1 instead of 2,
  of_node_get()/of_node_put() unbalanced - destroy cset entry:
  attach overlay node /i2c/pmic@34/tcpc/connector

The 'fwnode' set in tcpci_parse_config() which is called
in tcpci_register_port(), its node refcount is increased
in device_get_named_child_node(). It needs be put while
exiting, so call fwnode_handle_put() in the error path of
tcpci_register_port() and in tcpci_unregister_port() to
avoid leak.

Fixes: 5e85a04c8c0d ("usb: typec: add fwnode to tcpc")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
v2 -> v3:
  Move fwnode_handle_put() into tcpci_unregister_port().

v1 -> v2:
  Add description to how is the report generated and
  the review tag from Guenter.
---
 drivers/usb/typec/tcpm/tcpci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index b2bfcebe218f..72f8d1e87600 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -794,8 +794,10 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
 		return ERR_PTR(err);
 
 	tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc);
-	if (IS_ERR(tcpci->port))
+	if (IS_ERR(tcpci->port)) {
+		fwnode_handle_put(tcpci->tcpc.fwnode);
 		return ERR_CAST(tcpci->port);
+	}
 
 	return tcpci;
 }
@@ -804,6 +806,7 @@ EXPORT_SYMBOL_GPL(tcpci_register_port);
 void tcpci_unregister_port(struct tcpci *tcpci)
 {
 	tcpm_unregister_port(tcpci->port);
+	fwnode_handle_put(tcpci->tcpc.fwnode);
 }
 EXPORT_SYMBOL_GPL(tcpci_unregister_port);
 
-- 
2.25.1

Re: [PATCH v3] usb: typec: tcpci: fix of node refcount leak in tcpci_register_port()

[Heikki Krogerus]

On Mon, Nov 21, 2022 at 02:24:16PM +0800, Yang Yingliang wrote:
> I got the following report while doing device(mt6370-tcpc) load
> test with CONFIG_OF_UNITTEST and CONFIG_OF_DYNAMIC enabled:
> 
>   OF: ERROR: memory leak, expected refcount 1 instead of 2,
>   of_node_get()/of_node_put() unbalanced - destroy cset entry:
>   attach overlay node /i2c/pmic@34/tcpc/connector
> 
> The 'fwnode' set in tcpci_parse_config() which is called
> in tcpci_register_port(), its node refcount is increased
> in device_get_named_child_node(). It needs be put while
> exiting, so call fwnode_handle_put() in the error path of
> tcpci_register_port() and in tcpci_unregister_port() to
> avoid leak.
> 
> Fixes: 5e85a04c8c0d ("usb: typec: add fwnode to tcpc")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>

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

> ---
> v2 -> v3:
>   Move fwnode_handle_put() into tcpci_unregister_port().
> 
> v1 -> v2:
>   Add description to how is the report generated and
>   the review tag from Guenter.
> ---
>  drivers/usb/typec/tcpm/tcpci.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index b2bfcebe218f..72f8d1e87600 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -794,8 +794,10 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
>  		return ERR_PTR(err);
>  
>  	tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc);
> -	if (IS_ERR(tcpci->port))
> +	if (IS_ERR(tcpci->port)) {
> +		fwnode_handle_put(tcpci->tcpc.fwnode);
>  		return ERR_CAST(tcpci->port);
> +	}
>  
>  	return tcpci;
>  }
> @@ -804,6 +806,7 @@ EXPORT_SYMBOL_GPL(tcpci_register_port);
>  void tcpci_unregister_port(struct tcpci *tcpci)
>  {
>  	tcpm_unregister_port(tcpci->port);
> +	fwnode_handle_put(tcpci->tcpc.fwnode);
>  }
>  EXPORT_SYMBOL_GPL(tcpci_unregister_port);
>  

thanks,

-- 
heikki

Re: [PATCH v3] usb: typec: tcpci: fix of node refcount leak in tcpci_register_port()

[Guenter Roeck]

On Mon, Nov 21, 2022 at 02:24:16PM +0800, Yang Yingliang wrote:
> I got the following report while doing device(mt6370-tcpc) load
> test with CONFIG_OF_UNITTEST and CONFIG_OF_DYNAMIC enabled:
> 
>   OF: ERROR: memory leak, expected refcount 1 instead of 2,
>   of_node_get()/of_node_put() unbalanced - destroy cset entry:
>   attach overlay node /i2c/pmic@34/tcpc/connector
> 
> The 'fwnode' set in tcpci_parse_config() which is called
> in tcpci_register_port(), its node refcount is increased
> in device_get_named_child_node(). It needs be put while
> exiting, so call fwnode_handle_put() in the error path of
> tcpci_register_port() and in tcpci_unregister_port() to
> avoid leak.
> 
> Fixes: 5e85a04c8c0d ("usb: typec: add fwnode to tcpc")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> v2 -> v3:
>   Move fwnode_handle_put() into tcpci_unregister_port().
> 
> v1 -> v2:
>   Add description to how is the report generated and
>   the review tag from Guenter.
> ---
>  drivers/usb/typec/tcpm/tcpci.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index b2bfcebe218f..72f8d1e87600 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -794,8 +794,10 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
>  		return ERR_PTR(err);
>  
>  	tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc);
> -	if (IS_ERR(tcpci->port))
> +	if (IS_ERR(tcpci->port)) {
> +		fwnode_handle_put(tcpci->tcpc.fwnode);
>  		return ERR_CAST(tcpci->port);
> +	}
>  
>  	return tcpci;
>  }
> @@ -804,6 +806,7 @@ EXPORT_SYMBOL_GPL(tcpci_register_port);
>  void tcpci_unregister_port(struct tcpci *tcpci)
>  {
>  	tcpm_unregister_port(tcpci->port);
> +	fwnode_handle_put(tcpci->tcpc.fwnode);
>  }
>  EXPORT_SYMBOL_GPL(tcpci_unregister_port);
>  
> -- 
> 2.25.1
>