* [PATCH] leds: leds-max77650: Add of_node_put() before return
@ 2019-07-09 17:35 Nishka Dasgupta
2019-07-11 7:10 ` Bartosz Golaszewski
0 siblings, 1 reply; 3+ messages in thread
From: Nishka Dasgupta @ 2019-07-09 17:35 UTC (permalink / raw)
To: bgolaszewski, jacek.anaszewski, pavel, dmurphy, linux-leds
Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node puts the previous
node, but in the case of a return from the middle of the loop, there is
no put, thus causing a memory leak. Hence add an of_node_put before the
return in five places.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/leds/leds-max77650.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
index 6b74ce9cac12..417820f77c32 100644
--- a/drivers/leds/leds-max77650.c
+++ b/drivers/leds/leds-max77650.c
@@ -93,8 +93,10 @@ static int max77650_led_probe(struct platform_device *pdev)
for_each_child_of_node(of_node, child) {
rv = of_property_read_u32(child, "reg", ®);
- if (rv || reg >= MAX77650_LED_NUM_LEDS)
+ if (rv || reg >= MAX77650_LED_NUM_LEDS) {
+ of_node_put(child);
return -EINVAL;
+ }
led = &leds[reg];
led->map = map;
@@ -109,24 +111,32 @@ static int max77650_led_probe(struct platform_device *pdev)
} else {
led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
"max77650:%s", label);
- if (!led->cdev.name)
+ if (!led->cdev.name) {
+ of_node_put(child);
return -ENOMEM;
+ }
}
of_property_read_string(child, "linux,default-trigger",
&led->cdev.default_trigger);
rv = devm_of_led_classdev_register(dev, child, &led->cdev);
- if (rv)
+ if (rv) {
+ of_node_put(child);
return rv;
+ }
rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT);
- if (rv)
+ if (rv) {
+ of_node_put(child);
return rv;
+ }
rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT);
- if (rv)
+ if (rv) {
+ of_node_put(child);
return rv;
+ }
}
return regmap_write(map,
--
2.19.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] leds: leds-max77650: Add of_node_put() before return
2019-07-09 17:35 [PATCH] leds: leds-max77650: Add of_node_put() before return Nishka Dasgupta
@ 2019-07-11 7:10 ` Bartosz Golaszewski
2019-07-11 7:31 ` Nishka Dasgupta
0 siblings, 1 reply; 3+ messages in thread
From: Bartosz Golaszewski @ 2019-07-11 7:10 UTC (permalink / raw)
To: Nishka Dasgupta
Cc: Jacek Anaszewski, Pavel Machek, dmurphy, Linux LED Subsystem
wt., 9 lip 2019 o 19:35 Nishka Dasgupta <nishkadg.linux@gmail.com> napisał(a):
>
> Each iteration of for_each_child_of_node puts the previous
> node, but in the case of a return from the middle of the loop, there is
> no put, thus causing a memory leak. Hence add an of_node_put before the
> return in five places.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
> drivers/leds/leds-max77650.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
> index 6b74ce9cac12..417820f77c32 100644
> --- a/drivers/leds/leds-max77650.c
> +++ b/drivers/leds/leds-max77650.c
> @@ -93,8 +93,10 @@ static int max77650_led_probe(struct platform_device *pdev)
>
> for_each_child_of_node(of_node, child) {
> rv = of_property_read_u32(child, "reg", ®);
> - if (rv || reg >= MAX77650_LED_NUM_LEDS)
> + if (rv || reg >= MAX77650_LED_NUM_LEDS) {
> + of_node_put(child);
> return -EINVAL;
> + }
>
> led = &leds[reg];
> led->map = map;
> @@ -109,24 +111,32 @@ static int max77650_led_probe(struct platform_device *pdev)
> } else {
> led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
> "max77650:%s", label);
> - if (!led->cdev.name)
> + if (!led->cdev.name) {
> + of_node_put(child);
> return -ENOMEM;
> + }
> }
>
> of_property_read_string(child, "linux,default-trigger",
> &led->cdev.default_trigger);
>
> rv = devm_of_led_classdev_register(dev, child, &led->cdev);
> - if (rv)
> + if (rv) {
> + of_node_put(child);
> return rv;
> + }
>
> rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT);
> - if (rv)
> + if (rv) {
> + of_node_put(child);
> return rv;
> + }
>
> rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT);
> - if (rv)
> + if (rv) {
> + of_node_put(child);
> return rv;
> + }
> }
>
> return regmap_write(map,
> --
> 2.19.1
>
How about a label at the end of probe instead of adding this before
every return?
Bart
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] leds: leds-max77650: Add of_node_put() before return
2019-07-11 7:10 ` Bartosz Golaszewski
@ 2019-07-11 7:31 ` Nishka Dasgupta
0 siblings, 0 replies; 3+ messages in thread
From: Nishka Dasgupta @ 2019-07-11 7:31 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Jacek Anaszewski, Pavel Machek, dmurphy, Linux LED Subsystem
On 11/07/19 12:40 PM, Bartosz Golaszewski wrote:
> wt., 9 lip 2019 o 19:35 Nishka Dasgupta <nishkadg.linux@gmail.com> napisał(a):
>>
>> Each iteration of for_each_child_of_node puts the previous
>> node, but in the case of a return from the middle of the loop, there is
>> no put, thus causing a memory leak. Hence add an of_node_put before the
>> return in five places.
>> Issue found with Coccinelle.
>>
>> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
>> ---
>> drivers/leds/leds-max77650.c | 20 +++++++++++++++-----
>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
>> index 6b74ce9cac12..417820f77c32 100644
>> --- a/drivers/leds/leds-max77650.c
>> +++ b/drivers/leds/leds-max77650.c
>> @@ -93,8 +93,10 @@ static int max77650_led_probe(struct platform_device *pdev)
>>
>> for_each_child_of_node(of_node, child) {
>> rv = of_property_read_u32(child, "reg", ®);
>> - if (rv || reg >= MAX77650_LED_NUM_LEDS)
>> + if (rv || reg >= MAX77650_LED_NUM_LEDS) {
>> + of_node_put(child);
>> return -EINVAL;
>> + }
>>
>> led = &leds[reg];
>> led->map = map;
>> @@ -109,24 +111,32 @@ static int max77650_led_probe(struct platform_device *pdev)
>> } else {
>> led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
>> "max77650:%s", label);
>> - if (!led->cdev.name)
>> + if (!led->cdev.name) {
>> + of_node_put(child);
>> return -ENOMEM;
>> + }
>> }
>>
>> of_property_read_string(child, "linux,default-trigger",
>> &led->cdev.default_trigger);
>>
>> rv = devm_of_led_classdev_register(dev, child, &led->cdev);
>> - if (rv)
>> + if (rv) {
>> + of_node_put(child);
>> return rv;
>> + }
>>
>> rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT);
>> - if (rv)
>> + if (rv) {
>> + of_node_put(child);
>> return rv;
>> + }
>>
>> rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT);
>> - if (rv)
>> + if (rv) {
>> + of_node_put(child);
>> return rv;
>> + }
>> }
>>
>> return regmap_write(map,
>> --
>> 2.19.1
>>
>
> How about a label at the end of probe instead of adding this before
> every return?
I'll do that and send a version 2.
Thanking you,
Nishka
> Bart
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-07-11 7:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-09 17:35 [PATCH] leds: leds-max77650: Add of_node_put() before return Nishka Dasgupta
2019-07-11 7:10 ` Bartosz Golaszewski
2019-07-11 7:31 ` Nishka Dasgupta
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).