* [PATCH] of: drop a reference on error in __of_attach_node_sysfs()
@ 2020-05-18 11:30 Dan Carpenter
2020-05-20 10:06 ` Michael Ellerman
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2020-05-18 11:30 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: Michael Ellerman, devicetree, linux-kernel, kernel-janitors
We add a new of_node_get() to this function, but we should drop the
reference if kobject_add().
Fixes: 5b2c2f5a0ea3 ("of: overlay: add missing of_node_get() in __of_attach_node_sysfs")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
From static analysis. Maybe we should just call of_node_get() right
before we return 0?
drivers/of/kobj.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c
index c72eef988041..a90dc4b3b060 100644
--- a/drivers/of/kobj.c
+++ b/drivers/of/kobj.c
@@ -138,8 +138,10 @@ int __of_attach_node_sysfs(struct device_node *np)
rc = kobject_add(&np->kobj, parent, "%s", name);
kfree(name);
- if (rc)
+ if (rc) {
+ of_node_put(np);
return rc;
+ }
for_each_property_of_node(np, pp)
__of_add_property_sysfs(np, pp);
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] of: drop a reference on error in __of_attach_node_sysfs()
2020-05-18 11:30 [PATCH] of: drop a reference on error in __of_attach_node_sysfs() Dan Carpenter
@ 2020-05-20 10:06 ` Michael Ellerman
2020-05-20 12:04 ` [PATCH v2] of: Fix a refcounting bug " Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Michael Ellerman @ 2020-05-20 10:06 UTC (permalink / raw)
To: Dan Carpenter, Rob Herring, Frank Rowand
Cc: devicetree, linux-kernel, kernel-janitors
Dan Carpenter <dan.carpenter@oracle.com> writes:
> We add a new of_node_get() to this function, but we should drop the
> reference if kobject_add().
^
fails?
>
> Fixes: 5b2c2f5a0ea3 ("of: overlay: add missing of_node_get() in __of_attach_node_sysfs")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> From static analysis. Maybe we should just call of_node_get() right
> before we return 0?
Yeah that would be simpler and equally correct AFAICS.
cheers
> diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c
> index c72eef988041..a90dc4b3b060 100644
> --- a/drivers/of/kobj.c
> +++ b/drivers/of/kobj.c
> @@ -138,8 +138,10 @@ int __of_attach_node_sysfs(struct device_node *np)
>
> rc = kobject_add(&np->kobj, parent, "%s", name);
> kfree(name);
> - if (rc)
> + if (rc) {
> + of_node_put(np);
> return rc;
> + }
>
> for_each_property_of_node(np, pp)
> __of_add_property_sysfs(np, pp);
> --
> 2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] of: Fix a refcounting bug in __of_attach_node_sysfs()
2020-05-20 10:06 ` Michael Ellerman
@ 2020-05-20 12:04 ` Dan Carpenter
2020-05-27 18:40 ` Rob Herring
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2020-05-20 12:04 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: Michael Ellerman, devicetree, linux-kernel, kernel-janitors
The problem in this code is that if kobject_add() fails, then it should
call of_node_put(np) to drop the reference count. I've actually moved
the of_node_get(np) later in the function to avoid needing to do clean
up.
Fixes: 5b2c2f5a0ea3 ("of: overlay: add missing of_node_get() in __of_attach_node_sysfs")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: move the of_node_get() instead of doing clean up. Also the v1 had a
confusing typo in the commit message.
drivers/of/kobj.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c
index c72eef988041..a32e60b024b8 100644
--- a/drivers/of/kobj.c
+++ b/drivers/of/kobj.c
@@ -134,8 +134,6 @@ int __of_attach_node_sysfs(struct device_node *np)
if (!name)
return -ENOMEM;
- of_node_get(np);
-
rc = kobject_add(&np->kobj, parent, "%s", name);
kfree(name);
if (rc)
@@ -144,6 +142,7 @@ int __of_attach_node_sysfs(struct device_node *np)
for_each_property_of_node(np, pp)
__of_add_property_sysfs(np, pp);
+ of_node_get(np);
return 0;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] of: Fix a refcounting bug in __of_attach_node_sysfs()
2020-05-20 12:04 ` [PATCH v2] of: Fix a refcounting bug " Dan Carpenter
@ 2020-05-27 18:40 ` Rob Herring
0 siblings, 0 replies; 4+ messages in thread
From: Rob Herring @ 2020-05-27 18:40 UTC (permalink / raw)
To: Dan Carpenter
Cc: Rob Herring, linux-kernel, devicetree, kernel-janitors,
Michael Ellerman, Frank Rowand
On Wed, 20 May 2020 15:04:14 +0300, Dan Carpenter wrote:
> The problem in this code is that if kobject_add() fails, then it should
> call of_node_put(np) to drop the reference count. I've actually moved
> the of_node_get(np) later in the function to avoid needing to do clean
> up.
>
> Fixes: 5b2c2f5a0ea3 ("of: overlay: add missing of_node_get() in __of_attach_node_sysfs")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: move the of_node_get() instead of doing clean up. Also the v1 had a
> confusing typo in the commit message.
>
> drivers/of/kobj.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
Applied, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-27 18:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-18 11:30 [PATCH] of: drop a reference on error in __of_attach_node_sysfs() Dan Carpenter
2020-05-20 10:06 ` Michael Ellerman
2020-05-20 12:04 ` [PATCH v2] of: Fix a refcounting bug " Dan Carpenter
2020-05-27 18:40 ` Rob Herring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).