All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] POWER: gpio-charger: Check result of kzalloc
@ 2010-11-18 22:08 Lars-Peter Clausen
  2010-11-18 22:08 ` [PATCH] POWER: gpio-charger: Fix potential race between irq handler and probe/remove function Lars-Peter Clausen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Lars-Peter Clausen @ 2010-11-18 22:08 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: linux-kernel, Lars-Peter Clausen

Since kzalloc can return NULL we have to check its result.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/power/gpio-charger.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/power/gpio-charger.c b/drivers/power/gpio-charger.c
index fccbe99..8458caf 100644
--- a/drivers/power/gpio-charger.c
+++ b/drivers/power/gpio-charger.c
@@ -87,6 +87,10 @@ static int __devinit gpio_charger_probe(struct platform_device *pdev)
 	}
 
 	gpio_charger = kzalloc(sizeof(*gpio_charger), GFP_KERNEL);
+	if (!gpio_charger) {
+		dev_err(&pdev->dev, "Failed to alloc driver structure\n");
+		return -ENOMEM;
+	}
 
 	charger = &gpio_charger->charger;
 
-- 
1.5.6.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH] POWER: gpio-charger: Fix potential race between irq handler and probe/remove function
  2010-11-18 22:08 [PATCH] POWER: gpio-charger: Check result of kzalloc Lars-Peter Clausen
@ 2010-11-18 22:08 ` Lars-Peter Clausen
  2010-12-22  0:05   ` Anton Vorontsov
  2010-11-18 22:08 ` [PATCH] POWER: gpio-chager: Provide default name for the power_supply Lars-Peter Clausen
  2010-12-22  0:03 ` [PATCH] POWER: gpio-charger: Check result of kzalloc Anton Vorontsov
  2 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2010-11-18 22:08 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: linux-kernel, Lars-Peter Clausen

This patch fixes a potential race between the irq handler and the probe and
remove functions.
The irq should not be requested before the chargers power_supply has been
registered and has to be freed before the power_supply is unregistered,
otherwise it is possible that the irq fires while the power_supply is not
initialized yet or has already been freed.

While we are at it replace request_irq with request_any_context_irq.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/power/gpio-charger.c |   28 +++++++++++++---------------
 1 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/power/gpio-charger.c b/drivers/power/gpio-charger.c
index 8458caf..016d5c0 100644
--- a/drivers/power/gpio-charger.c
+++ b/drivers/power/gpio-charger.c
@@ -98,7 +98,7 @@ static int __devinit gpio_charger_probe(struct platform_device *pdev)
 	charger->type = pdata->type;
 	charger->properties = gpio_charger_properties;
 	charger->num_properties = ARRAY_SIZE(gpio_charger_properties);
-	charger->get_property  = gpio_charger_get_property;
+	charger->get_property = gpio_charger_get_property;
 	charger->supplied_to = pdata->supplied_to;
 	charger->num_supplicants = pdata->num_supplicants;
 
@@ -113,9 +113,17 @@ static int __devinit gpio_charger_probe(struct platform_device *pdev)
 		goto err_gpio_free;
 	}
 
+	gpio_charger->pdata = pdata;
+
+	ret = power_supply_register(&pdev->dev, charger);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to register power supply: %d\n", ret);
+		goto err_gpio_free;
+	}
+
 	irq = gpio_to_irq(pdata->gpio);
 	if (irq > 0) {
-		ret = request_irq(irq, gpio_charger_irq,
+		ret = request_any_context_irq(irq, gpio_charger_irq,
 				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
 				dev_name(&pdev->dev), charger);
 		if (ret)
@@ -124,21 +132,10 @@ static int __devinit gpio_charger_probe(struct platform_device *pdev)
 			gpio_charger->irq = irq;
 	}
 
-	gpio_charger->pdata = pdata;
-
-	ret = power_supply_register(&pdev->dev, charger);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to register power supply: %d\n", ret);
-		goto err_irq_free;
-	}
-
 	platform_set_drvdata(pdev, gpio_charger);
 
 	return 0;
 
-err_irq_free:
-	if (gpio_charger->irq)
-		free_irq(gpio_charger->irq, charger);
 err_gpio_free:
 	gpio_free(pdata->gpio);
 err_free:
@@ -150,10 +147,11 @@ static int __devexit gpio_charger_remove(struct platform_device *pdev)
 {
 	struct gpio_charger *gpio_charger = platform_get_drvdata(pdev);
 
-	power_supply_unregister(&gpio_charger->charger);
-
 	if (gpio_charger->irq)
 		free_irq(gpio_charger->irq, &gpio_charger->charger);
+
+	power_supply_unregister(&gpio_charger->charger);
+
 	gpio_free(gpio_charger->pdata->gpio);
 
 	platform_set_drvdata(pdev, NULL);
-- 
1.5.6.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH] POWER: gpio-chager: Provide default name for the power_supply
  2010-11-18 22:08 [PATCH] POWER: gpio-charger: Check result of kzalloc Lars-Peter Clausen
  2010-11-18 22:08 ` [PATCH] POWER: gpio-charger: Fix potential race between irq handler and probe/remove function Lars-Peter Clausen
@ 2010-11-18 22:08 ` Lars-Peter Clausen
  2010-12-22  0:04   ` Anton Vorontsov
  2010-12-22  0:03 ` [PATCH] POWER: gpio-charger: Check result of kzalloc Anton Vorontsov
  2 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2010-11-18 22:08 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: linux-kernel, Lars-Peter Clausen

This patch sets a default name for the power_supply in case there was no name
supplied through the platform_data.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/power/gpio-charger.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/power/gpio-charger.c b/drivers/power/gpio-charger.c
index 016d5c0..27b8520 100644
--- a/drivers/power/gpio-charger.c
+++ b/drivers/power/gpio-charger.c
@@ -94,7 +94,7 @@ static int __devinit gpio_charger_probe(struct platform_device *pdev)
 
 	charger = &gpio_charger->charger;
 
-	charger->name = pdata->name;
+	charger->name = pdata->name ?: "gpio-charger";
 	charger->type = pdata->type;
 	charger->properties = gpio_charger_properties;
 	charger->num_properties = ARRAY_SIZE(gpio_charger_properties);
-- 
1.5.6.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] POWER: gpio-charger: Check result of kzalloc
  2010-11-18 22:08 [PATCH] POWER: gpio-charger: Check result of kzalloc Lars-Peter Clausen
  2010-11-18 22:08 ` [PATCH] POWER: gpio-charger: Fix potential race between irq handler and probe/remove function Lars-Peter Clausen
  2010-11-18 22:08 ` [PATCH] POWER: gpio-chager: Provide default name for the power_supply Lars-Peter Clausen
@ 2010-12-22  0:03 ` Anton Vorontsov
  2 siblings, 0 replies; 6+ messages in thread
From: Anton Vorontsov @ 2010-12-22  0:03 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: linux-kernel, Vasiliy Kulikov, Dan Carpenter

On Thu, Nov 18, 2010 at 11:08:37PM +0100, Lars-Peter Clausen wrote:
> Since kzalloc can return NULL we have to check its result.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---

Lars, Vasiliy, Dan,

Thanks for the patch! I applied the following:

commit 2e9ff5f5e4c6b034554f3539f29529265279102c
Author: Lars-Peter Clausen <lars@metafoo.de>
Date:   Thu Nov 18 23:08:37 2010 +0100

    gpio-charger: Check result of kzalloc
    
    Since kzalloc can return NULL we have to check its result.
    
    Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
    Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
    Signed-off-by: Dan Carpenter <error27@gmail.com>
    Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>

diff --git a/drivers/power/gpio-charger.c b/drivers/power/gpio-charger.c
index fccbe99..8458caf 100644
--- a/drivers/power/gpio-charger.c
+++ b/drivers/power/gpio-charger.c
@@ -87,6 +87,10 @@ static int __devinit gpio_charger_probe(struct platform_device *pdev)
 	}
 
 	gpio_charger = kzalloc(sizeof(*gpio_charger), GFP_KERNEL);
+	if (!gpio_charger) {
+		dev_err(&pdev->dev, "Failed to alloc driver structure\n");
+		return -ENOMEM;
+	}
 
 	charger = &gpio_charger->charger;
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] POWER: gpio-chager: Provide default name for the power_supply
  2010-11-18 22:08 ` [PATCH] POWER: gpio-chager: Provide default name for the power_supply Lars-Peter Clausen
@ 2010-12-22  0:04   ` Anton Vorontsov
  0 siblings, 0 replies; 6+ messages in thread
From: Anton Vorontsov @ 2010-12-22  0:04 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: linux-kernel

On Thu, Nov 18, 2010 at 11:08:39PM +0100, Lars-Peter Clausen wrote:
> This patch sets a default name for the power_supply in case there was no name
> supplied through the platform_data.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---

Applied, thanks!

>  drivers/power/gpio-charger.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/power/gpio-charger.c b/drivers/power/gpio-charger.c
> index 016d5c0..27b8520 100644
> --- a/drivers/power/gpio-charger.c
> +++ b/drivers/power/gpio-charger.c
> @@ -94,7 +94,7 @@ static int __devinit gpio_charger_probe(struct platform_device *pdev)
>  
>  	charger = &gpio_charger->charger;
>  
> -	charger->name = pdata->name;
> +	charger->name = pdata->name ?: "gpio-charger";
>  	charger->type = pdata->type;
>  	charger->properties = gpio_charger_properties;
>  	charger->num_properties = ARRAY_SIZE(gpio_charger_properties);

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] POWER: gpio-charger: Fix potential race between irq handler and probe/remove function
  2010-11-18 22:08 ` [PATCH] POWER: gpio-charger: Fix potential race between irq handler and probe/remove function Lars-Peter Clausen
@ 2010-12-22  0:05   ` Anton Vorontsov
  0 siblings, 0 replies; 6+ messages in thread
From: Anton Vorontsov @ 2010-12-22  0:05 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: linux-kernel

On Thu, Nov 18, 2010 at 11:08:38PM +0100, Lars-Peter Clausen wrote:
> This patch fixes a potential race between the irq handler and the probe and
> remove functions.
> The irq should not be requested before the chargers power_supply has been
> registered and has to be freed before the power_supply is unregistered,
> otherwise it is possible that the irq fires while the power_supply is not
> initialized yet or has already been freed.
> 
> While we are at it replace request_irq with request_any_context_irq.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Applied to battery-2.6.git, thanks!

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-12-22  0:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-18 22:08 [PATCH] POWER: gpio-charger: Check result of kzalloc Lars-Peter Clausen
2010-11-18 22:08 ` [PATCH] POWER: gpio-charger: Fix potential race between irq handler and probe/remove function Lars-Peter Clausen
2010-12-22  0:05   ` Anton Vorontsov
2010-11-18 22:08 ` [PATCH] POWER: gpio-chager: Provide default name for the power_supply Lars-Peter Clausen
2010-12-22  0:04   ` Anton Vorontsov
2010-12-22  0:03 ` [PATCH] POWER: gpio-charger: Check result of kzalloc Anton Vorontsov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.