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月24日 22:51, Peter Rosin wrote:
On 2017-01-24 01:24, qhou wrote:
On 2017年01月24日 00:27, Peter Rosin wrote:
On 2017-01-23 11:40, 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>
Acked-by: Peter Rosin <peda@xxxxxxxxxx>
---
   drivers/of/base.c | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index d4bea3c..091ad29 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -842,8 +842,10 @@ 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;
Hi,

You failed to add a blank line here...
Sorry,  I miss it, I will add.
   		path++; /* Increment past '/' delimiter */
-		np = __of_find_node_by_path(np, path);
+		np = __of_find_node_by_path(npp, path);
...and there's no need to change the above line if you
want to make a minimal change.
Just want the code readable, as npp is parent and np is a some child of
parent.
If you go for readable, name the variable 'parent' instead of 'npp'.
But 'prev', 'old', 'last', 'tmp' or something like that would also
work, at least for me. The thing is just a  temporary holding space
for the of_node_put call and the fact that it's the parent of 'np'
is kind of irrelevant...
I will take 'tmp' as the name of the variable. As its name, (1)it is a temporary, there will not need for some explanation, just release it after __of_find_node_by_path.
(2) it will make a minimal change.

Not my code though, and I don't really care.
Excuse me for my presumption, linux OS is so great, I never intend to introduce any flaw
or obscure.

Best regards,
Qi Hou

Cheers,
peda

-- Best regards,
Qi Hou
Cheers,
peda

+		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]