linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][next] rtc: ds1307: check for failed memory allocation on wdt
@ 2020-04-02 13:14 Colin King
  2020-04-02 13:40 ` AW: " Walter Harms
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Colin King @ 2020-04-02 13:14 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Guenter Roeck,
	Chris Packham, linux-rtc
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Currently a failed memory allocation will lead to a null pointer
dereference on point wdt.  Fix this by checking for a failed allocation
and adding error return handling to function ds1307_wdt_register.

Addresses-Coverity: ("Dereference null return")
Fixes: fd90d48db037 ("rtc: ds1307: add support for watchdog timer on ds1388")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/rtc/rtc-ds1307.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index fad042118862..95c5b6facc59 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1665,14 +1665,16 @@ static const struct watchdog_ops ds1388_wdt_ops = {
 
 };
 
-static void ds1307_wdt_register(struct ds1307 *ds1307)
+static int ds1307_wdt_register(struct ds1307 *ds1307)
 {
 	struct watchdog_device	*wdt;
 
 	if (ds1307->type != ds_1388)
-		return;
+		return 0;
 
 	wdt = devm_kzalloc(ds1307->dev, sizeof(*wdt), GFP_KERNEL);
+	if (!wdt)
+		return -ENOMEM;
 
 	wdt->info = &ds1388_wdt_info;
 	wdt->ops = &ds1388_wdt_ops;
@@ -1683,10 +1685,13 @@ static void ds1307_wdt_register(struct ds1307 *ds1307)
 	watchdog_init_timeout(wdt, 0, ds1307->dev);
 	watchdog_set_drvdata(wdt, ds1307);
 	devm_watchdog_register_device(ds1307->dev, wdt);
+
+	return 0;
 }
 #else
-static void ds1307_wdt_register(struct ds1307 *ds1307)
+static int ds1307_wdt_register(struct ds1307 *ds1307)
 {
+	return 0;
 }
 #endif /* CONFIG_WATCHDOG_CORE */
 
@@ -1979,9 +1984,9 @@ static int ds1307_probe(struct i2c_client *client,
 
 	ds1307_hwmon_register(ds1307);
 	ds1307_clks_register(ds1307);
-	ds1307_wdt_register(ds1307);
+	err = ds1307_wdt_register(ds1307);
 
-	return 0;
+	return err;
 
 exit:
 	return err;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* AW: [PATCH][next] rtc: ds1307: check for failed memory allocation on wdt
  2020-04-02 13:14 [PATCH][next] rtc: ds1307: check for failed memory allocation on wdt Colin King
@ 2020-04-02 13:40 ` Walter Harms
  2020-04-02 13:52 ` Alexandre Belloni
  2020-04-03  8:40 ` Dan Carpenter
  2 siblings, 0 replies; 4+ messages in thread
From: Walter Harms @ 2020-04-02 13:40 UTC (permalink / raw)
  To: Colin King, Alessandro Zummo, Alexandre Belloni, Guenter Roeck,
	Chris Packham, linux-rtc
  Cc: kernel-janitors, linux-kernel


________________________________________
Von: kernel-janitors-owner@vger.kernel.org <kernel-janitors-owner@vger.kernel.org> im Auftrag von Colin King <colin.king@canonical.com>
Gesendet: Donnerstag, 2. April 2020 15:14
An: Alessandro Zummo; Alexandre Belloni; Guenter Roeck; Chris Packham; linux-rtc@vger.kernel.org
Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org
Betreff: [PATCH][next] rtc: ds1307: check for failed memory allocation on wdt

From: Colin Ian King <colin.king@canonical.com>

Currently a failed memory allocation will lead to a null pointer
dereference on point wdt.  Fix this by checking for a failed allocation
and adding error return handling to function ds1307_wdt_register.

Addresses-Coverity: ("Dereference null return")
Fixes: fd90d48db037 ("rtc: ds1307: add support for watchdog timer on ds1388")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/rtc/rtc-ds1307.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index fad042118862..95c5b6facc59 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1665,14 +1665,16 @@ static const struct watchdog_ops ds1388_wdt_ops = {

 };

-static void ds1307_wdt_register(struct ds1307 *ds1307)
+static int ds1307_wdt_register(struct ds1307 *ds1307)
 {
        struct watchdog_device  *wdt;

        if (ds1307->type != ds_1388)
-               return;
+               return 0;

        wdt = devm_kzalloc(ds1307->dev, sizeof(*wdt), GFP_KERNEL);
+       if (!wdt)
+               return -ENOMEM;

        wdt->info = &ds1388_wdt_info;
        wdt->ops = &ds1388_wdt_ops;
@@ -1683,10 +1685,13 @@ static void ds1307_wdt_register(struct ds1307 *ds1307)
        watchdog_init_timeout(wdt, 0, ds1307->dev);
        watchdog_set_drvdata(wdt, ds1307);
        devm_watchdog_register_device(ds1307->dev, wdt);
+
+       return 0;
 }
 #else
-static void ds1307_wdt_register(struct ds1307 *ds1307)
+static int ds1307_wdt_register(struct ds1307 *ds1307)
 {
+       return 0;
 }
 #endif /* CONFIG_WATCHDOG_CORE */

@@ -1979,9 +1984,9 @@ static int ds1307_probe(struct i2c_client *client,

        ds1307_hwmon_register(ds1307);
        ds1307_clks_register(ds1307);
-       ds1307_wdt_register(ds1307);
+       err = ds1307_wdt_register(ds1307);

-       return 0;
+       return err;

 exit:
        return err;
--
2.25.1

IMHO, one "return err;" is sufficient.

re,
 wh

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH][next] rtc: ds1307: check for failed memory allocation on wdt
  2020-04-02 13:14 [PATCH][next] rtc: ds1307: check for failed memory allocation on wdt Colin King
  2020-04-02 13:40 ` AW: " Walter Harms
@ 2020-04-02 13:52 ` Alexandre Belloni
  2020-04-03  8:40 ` Dan Carpenter
  2 siblings, 0 replies; 4+ messages in thread
From: Alexandre Belloni @ 2020-04-02 13:52 UTC (permalink / raw)
  To: Colin King
  Cc: Alessandro Zummo, Guenter Roeck, Chris Packham, linux-rtc,
	kernel-janitors, linux-kernel

On 02/04/2020 14:14:41+0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Currently a failed memory allocation will lead to a null pointer
> dereference on point wdt.  Fix this by checking for a failed allocation
> and adding error return handling to function ds1307_wdt_register.
> 
> Addresses-Coverity: ("Dereference null return")
> Fixes: fd90d48db037 ("rtc: ds1307: add support for watchdog timer on ds1388")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/rtc/rtc-ds1307.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index fad042118862..95c5b6facc59 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -1665,14 +1665,16 @@ static const struct watchdog_ops ds1388_wdt_ops = {
>  
>  };
>  
> -static void ds1307_wdt_register(struct ds1307 *ds1307)
> +static int ds1307_wdt_register(struct ds1307 *ds1307)
>  {
>  	struct watchdog_device	*wdt;
>  
>  	if (ds1307->type != ds_1388)
> -		return;
> +		return 0;
>  
>  	wdt = devm_kzalloc(ds1307->dev, sizeof(*wdt), GFP_KERNEL);
> +	if (!wdt)
> +		return -ENOMEM;
>  
>  	wdt->info = &ds1388_wdt_info;
>  	wdt->ops = &ds1388_wdt_ops;
> @@ -1683,10 +1685,13 @@ static void ds1307_wdt_register(struct ds1307 *ds1307)
>  	watchdog_init_timeout(wdt, 0, ds1307->dev);
>  	watchdog_set_drvdata(wdt, ds1307);
>  	devm_watchdog_register_device(ds1307->dev, wdt);
> +
> +	return 0;
>  }
>  #else
> -static void ds1307_wdt_register(struct ds1307 *ds1307)
> +static int ds1307_wdt_register(struct ds1307 *ds1307)
>  {
> +	return 0;
>  }
>  #endif /* CONFIG_WATCHDOG_CORE */
>  
> @@ -1979,9 +1984,9 @@ static int ds1307_probe(struct i2c_client *client,
>  
>  	ds1307_hwmon_register(ds1307);
>  	ds1307_clks_register(ds1307);
> -	ds1307_wdt_register(ds1307);
> +	err = ds1307_wdt_register(ds1307);
>  
> -	return 0;
> +	return err;

As the watchdog is optional and the RTC can work properly without the
watchdog driver being compiled, my opinion is that the error should not
be returned.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH][next] rtc: ds1307: check for failed memory allocation on wdt
  2020-04-02 13:14 [PATCH][next] rtc: ds1307: check for failed memory allocation on wdt Colin King
  2020-04-02 13:40 ` AW: " Walter Harms
  2020-04-02 13:52 ` Alexandre Belloni
@ 2020-04-03  8:40 ` Dan Carpenter
  2 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2020-04-03  8:40 UTC (permalink / raw)
  To: Colin King
  Cc: Alessandro Zummo, Alexandre Belloni, Guenter Roeck,
	Chris Packham, linux-rtc, kernel-janitors, linux-kernel

On Thu, Apr 02, 2020 at 02:14:41PM +0100, Colin King wrote:
> @@ -1979,9 +1984,9 @@ static int ds1307_probe(struct i2c_client *client,
>  
>  	ds1307_hwmon_register(ds1307);
>  	ds1307_clks_register(ds1307);
> -	ds1307_wdt_register(ds1307);
> +	err = ds1307_wdt_register(ds1307);
>  
> -	return 0;
> +	return err;
>  
>  exit:
>  	return err;

This bit is weird.  I guess just delete the "return 0;" without
introducing a new "return err;".

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-04-03  8:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-02 13:14 [PATCH][next] rtc: ds1307: check for failed memory allocation on wdt Colin King
2020-04-02 13:40 ` AW: " Walter Harms
2020-04-02 13:52 ` Alexandre Belloni
2020-04-03  8:40 ` Dan Carpenter

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).