Re: [PATCH 1/3] of: fix of_node leak caused in of_find_node_opts_by_path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2017年01月23日 16:21, Peter Rosin wrote:
On 2017-01-23 03:21, Qi Hou wrote:
During stepping down the tree, parent node is gotten first and its refcount is
increased with of_node_get() in __of_get_next_child(). Since it just being used
as step node, its refcount must be decreased with of_node_put() after traversing
its child nodes.

Or, its refcount will never be descreased to ZERO, then it will never be freed,
as well as other related memory blocks.

To fix this, decrease refcount of parent with of_node_put() after
__of_find_node_by_path().

Signed-off-by: Qi Hou <qi.hou@xxxxxxxxxxxxx>
Reviewed-by: Zhang Xiao <xiao.zhang@xxxxxxxxxxxxx>
Acked-by: Bruce Ashfield <bruce.ashfield@xxxxxxxxxxxxx>
---
  drivers/of/base.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index d4bea3c..c72e860 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -802,7 +802,7 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent,
   */
  struct device_node *of_find_node_opts_by_path(const char *path, const char **opts)
  {
-	struct device_node *np = NULL;
+	struct device_node *np = NULL, *npp = NULL;
Why initialize npp? And I would declare it (and initialize it to np) inside the
while loop instead.
Yes, that is a good idea. And it looks good.

What about this ?
diff --git a/drivers/of/base.c b/drivers/of/base.c
index c72e860..091ad29 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -802,7 +802,7 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent,
  */
struct device_node *of_find_node_opts_by_path(const char *path, const char **opts)
 {
-       struct device_node *np = NULL, *npp = NULL;
+       struct device_node *np = NULL;
        struct property *pp;
        unsigned long flags;
        const char *separator = strchr(path, ':');
@@ -842,8 +842,8 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
        if (!np)
                np = of_node_get(of_root);
        while (np && *path == '/') {
+               struct device_node *npp = np;
                path++; /* Increment past '/' delimiter */
-               npp = np;
                np = __of_find_node_by_path(npp, path);
                of_node_put(npp);
                path = strchrnul(path, '/');
-- Best regards, --
Qi Hou

Cheers,
peda

  	struct property *pp;
  	unsigned long flags;
  	const char *separator = strchr(path, ':');
@@ -843,7 +843,9 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
  		np = of_node_get(of_root);
  	while (np && *path == '/') {
  		path++; /* Increment past '/' delimiter */
-		np = __of_find_node_by_path(np, path);
+		npp = np;
+		np = __of_find_node_by_path(npp, path);
+		of_node_put(npp);
  		path = strchrnul(path, '/');
  		if (separator && separator < path)
  			break;


--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]