| Message ID | 20220228091103.39749-1-hbh25y@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc: kernel: fix a refcount leak in format_show() | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
Hangyu Hua <hbh25y@gmail.com> writes: > node needs to be dropped when of_property_read_string fails. So an earlier call > to of_node_put is required here. That's true but ... > diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c > index a0a78aba2083..cd0fa7028d86 100644 > --- a/arch/powerpc/kernel/secvar-sysfs.c > +++ b/arch/powerpc/kernel/secvar-sysfs.c > @@ -30,13 +30,12 @@ static ssize_t format_show(struct kobject *kobj, struct kobj_attribute *attr, > return -ENODEV; There's also a reference leak there ^ So if you're going to touch this code I'd like you to fix both reference leaks in a single patch please. Having the error cases set rc and then goto "out" which does the of_node_put() is the obvious solution I think. cheers > rc = of_property_read_string(node, "format", &format); > + of_node_put(node); > if (rc) > return rc; > > rc = sprintf(buf, "%s\n", format); > > - of_node_put(node); > - > return rc; > } > > -- > 2.25.1
On 3/1/22 04:55, Michael Ellerman wrote: > Hangyu Hua <hbh25y@gmail.com> writes: >> node needs to be dropped when of_property_read_string fails. So an earlier call >> to of_node_put is required here. > > That's true but ... > >> diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c >> index a0a78aba2083..cd0fa7028d86 100644 >> --- a/arch/powerpc/kernel/secvar-sysfs.c >> +++ b/arch/powerpc/kernel/secvar-sysfs.c >> @@ -30,13 +30,12 @@ static ssize_t format_show(struct kobject *kobj, struct kobj_attribute *attr, >> return -ENODEV; > > There's also a reference leak there ^ > > So if you're going to touch this code I'd like you to fix both reference > leaks in a single patch please. > > Having the error cases set rc and then goto "out" which does the > of_node_put() is the obvious solution I think. update_kobj_size() in the same source file provides a good example of the suggested solution. -Tyrel > > cheers > >> rc = of_property_read_string(node, "format", &format); >> + of_node_put(node); >> if (rc) >> return rc; >> >> rc = sprintf(buf, "%s\n", format); >> >> - of_node_put(node); >> - >> return rc; >> } >> >> -- >> 2.25.1
Thanks. I will resubmit my patch latter. On 2022/3/2 03:50, Tyrel Datwyler wrote: > On 3/1/22 04:55, Michael Ellerman wrote: >> Hangyu Hua <hbh25y@gmail.com> writes: >>> node needs to be dropped when of_property_read_string fails. So an earlier call >>> to of_node_put is required here. >> >> That's true but ... >> >>> diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c >>> index a0a78aba2083..cd0fa7028d86 100644 >>> --- a/arch/powerpc/kernel/secvar-sysfs.c >>> +++ b/arch/powerpc/kernel/secvar-sysfs.c >>> @@ -30,13 +30,12 @@ static ssize_t format_show(struct kobject *kobj, struct kobj_attribute *attr, >>> return -ENODEV; >> >> There's also a reference leak there ^ >> >> So if you're going to touch this code I'd like you to fix both reference >> leaks in a single patch please. >> >> Having the error cases set rc and then goto "out" which does the >> of_node_put() is the obvious solution I think. > > update_kobj_size() in the same source file provides a good example of the > suggested solution. > > -Tyrel > >> >> cheers >> >>> rc = of_property_read_string(node, "format", &format); >>> + of_node_put(node); >>> if (rc) >>> return rc; >>> >>> rc = sprintf(buf, "%s\n", format); >>> >>> - of_node_put(node); >>> - >>> return rc; >>> } >>> >>> -- >>> 2.25.1 >
diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c index a0a78aba2083..cd0fa7028d86 100644 --- a/arch/powerpc/kernel/secvar-sysfs.c +++ b/arch/powerpc/kernel/secvar-sysfs.c @@ -30,13 +30,12 @@ static ssize_t format_show(struct kobject *kobj, struct kobj_attribute *attr, return -ENODEV; rc = of_property_read_string(node, "format", &format); + of_node_put(node); if (rc) return rc; rc = sprintf(buf, "%s\n", format); - of_node_put(node); - return rc; }
node needs to be dropped when of_property_read_string fails. So an earlier call to of_node_put is required here. Signed-off-by: Hangyu Hua <hbh25y@gmail.com> --- arch/powerpc/kernel/secvar-sysfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)