All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] treewide: test clock rate to avoid division by 0
@ 2016-03-02 22:33 ` Wolfram Sang
  0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2016-03-02 22:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-renesas-soc, linux-arm-kernel, linux-ide, linux-pwm,
	linux-watchdog, netdev

Here is the outcome of researching if the result of clk_get_rate() was directly
used as a divisor without checking if it is 0. Inspired by a Coverity report.

Wolfram Sang (6):
  ide: palm_bk3710: test clock rate to avoid division by 0
  net: ethernet: renesas: ravb_main: test clock rate to avoid division
    by 0
  pwm: pwm-img: test clock rate to avoid division by 0
  pwm: pwm-lpc18xx-sct: test clock rate to avoid division by 0
  watchdog: atlas7_wdt: test clock rate to avoid division by 0
  watchdog: tangox_wdt: test clock rate to avoid division by 0

 drivers/ide/palm_bk3710.c                | 2 ++
 drivers/net/ethernet/renesas/ravb_main.c | 3 +++
 drivers/pwm/pwm-img.c                    | 5 +++++
 drivers/pwm/pwm-lpc18xx-sct.c            | 2 ++
 drivers/watchdog/atlas7_wdt.c            | 5 +++++
 drivers/watchdog/tangox_wdt.c            | 2 ++
 6 files changed, 19 insertions(+)

-- 
2.7.0


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

* [PATCH 0/6] treewide: test clock rate to avoid division by 0
@ 2016-03-02 22:33 ` Wolfram Sang
  0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2016-03-02 22:33 UTC (permalink / raw)
  To: linux-arm-kernel

Here is the outcome of researching if the result of clk_get_rate() was directly
used as a divisor without checking if it is 0. Inspired by a Coverity report.

Wolfram Sang (6):
  ide: palm_bk3710: test clock rate to avoid division by 0
  net: ethernet: renesas: ravb_main: test clock rate to avoid division
    by 0
  pwm: pwm-img: test clock rate to avoid division by 0
  pwm: pwm-lpc18xx-sct: test clock rate to avoid division by 0
  watchdog: atlas7_wdt: test clock rate to avoid division by 0
  watchdog: tangox_wdt: test clock rate to avoid division by 0

 drivers/ide/palm_bk3710.c                | 2 ++
 drivers/net/ethernet/renesas/ravb_main.c | 3 +++
 drivers/pwm/pwm-img.c                    | 5 +++++
 drivers/pwm/pwm-lpc18xx-sct.c            | 2 ++
 drivers/watchdog/atlas7_wdt.c            | 5 +++++
 drivers/watchdog/tangox_wdt.c            | 2 ++
 6 files changed, 19 insertions(+)

-- 
2.7.0

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

* [PATCH 1/6] ide: palm_bk3710: test clock rate to avoid division by 0
  2016-03-02 22:33 ` Wolfram Sang
  (?)
@ 2016-03-02 22:33 ` Wolfram Sang
  -1 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2016-03-02 22:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-renesas-soc, David S. Miller, linux-ide

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The clk API may return 0 on clk_get_rate, so we should check the result before
using it as a divisor.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Should go individually via subsystem tree.

 drivers/ide/palm_bk3710.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c
index 8012e43bf8f618..46427ea01753b4 100644
--- a/drivers/ide/palm_bk3710.c
+++ b/drivers/ide/palm_bk3710.c
@@ -325,6 +325,8 @@ static int __init palm_bk3710_probe(struct platform_device *pdev)
 
 	clk_enable(clk);
 	rate = clk_get_rate(clk);
+	if (!rate)
+		return -EINVAL;
 
 	/* NOTE:  round *down* to meet minimum timings; we count in clocks */
 	ideclk_period = 1000000000UL / rate;
-- 
2.7.0

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

* [PATCH 2/6] net: ethernet: renesas: ravb_main: test clock rate to avoid division by 0
  2016-03-02 22:33 ` Wolfram Sang
  (?)
  (?)
@ 2016-03-02 22:33 ` Wolfram Sang
  2016-03-03 11:14   ` Sergei Shtylyov
  2016-04-03 21:36   ` Wolfram Sang
  -1 siblings, 2 replies; 22+ messages in thread
From: Wolfram Sang @ 2016-03-02 22:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-renesas-soc, Sergei Shtylyov, netdev

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The clk API may return 0 on clk_get_rate, so we should check the result before
using it as a divisor.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Should go individually via subsystem tree.

 drivers/net/ethernet/renesas/ravb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 88656ceb6e2946..ce1954a6a12726 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1691,6 +1691,9 @@ static int ravb_set_gti(struct net_device *ndev)
 	rate = clk_get_rate(clk);
 	clk_put(clk);
 
+	if (!rate)
+		return -EINVAL;
+
 	inc = 1000000000ULL << 20;
 	do_div(inc, rate);
 
-- 
2.7.0

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

* [PATCH 3/6] pwm: pwm-img: test clock rate to avoid division by 0
  2016-03-02 22:33 ` Wolfram Sang
                   ` (2 preceding siblings ...)
  (?)
@ 2016-03-02 22:33 ` Wolfram Sang
  2016-03-04 15:01   ` Thierry Reding
  -1 siblings, 1 reply; 22+ messages in thread
From: Wolfram Sang @ 2016-03-02 22:33 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-renesas-soc, Thierry Reding, linux-pwm

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The clk API may return 0 on clk_get_rate, so we should check the result before
using it as a divisor.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Should go individually via subsystem tree.

 drivers/pwm/pwm-img.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pwm/pwm-img.c b/drivers/pwm/pwm-img.c
index 8a029f9bc18cb0..2fb30deee34570 100644
--- a/drivers/pwm/pwm-img.c
+++ b/drivers/pwm/pwm-img.c
@@ -237,6 +237,11 @@ static int img_pwm_probe(struct platform_device *pdev)
 	}
 
 	clk_rate = clk_get_rate(pwm->pwm_clk);
+	if (!clk_rate) {
+		dev_err(&pdev->dev, "pwm clock has no frequency\n");
+		ret = -EINVAL;
+		goto disable_pwmclk;
+	}
 
 	/* The maximum input clock divider is 512 */
 	val = (u64)NSEC_PER_SEC * 512 * pwm->data->max_timebase;
-- 
2.7.0

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

* [PATCH 4/6] pwm: pwm-lpc18xx-sct: test clock rate to avoid division by 0
  2016-03-02 22:33 ` Wolfram Sang
  (?)
@ 2016-03-02 22:33   ` Wolfram Sang
  -1 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2016-03-02 22:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-renesas-soc, linux-pwm, Joachim Eastwood, Thierry Reding,
	linux-arm-kernel

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The clk API may return 0 on clk_get_rate, so we should check the result before
using it as a divisor.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Should go individually via subsystem tree.

 drivers/pwm/pwm-lpc18xx-sct.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
index 9163085101bc94..6487962c355c03 100644
--- a/drivers/pwm/pwm-lpc18xx-sct.c
+++ b/drivers/pwm/pwm-lpc18xx-sct.c
@@ -360,6 +360,8 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
 	}
 
 	lpc18xx_pwm->clk_rate = clk_get_rate(lpc18xx_pwm->pwm_clk);
+	if (!lpc18xx_pwm->clk_rate)
+		return -EINVAL;
 
 	mutex_init(&lpc18xx_pwm->res_lock);
 	mutex_init(&lpc18xx_pwm->period_lock);
-- 
2.7.0

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

* [PATCH 4/6] pwm: pwm-lpc18xx-sct: test clock rate to avoid division by 0
@ 2016-03-02 22:33   ` Wolfram Sang
  0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2016-03-02 22:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-renesas-soc, Thierry Reding, Joachim Eastwood, linux-pwm,
	linux-arm-kernel

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The clk API may return 0 on clk_get_rate, so we should check the result before
using it as a divisor.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Should go individually via subsystem tree.

 drivers/pwm/pwm-lpc18xx-sct.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
index 9163085101bc94..6487962c355c03 100644
--- a/drivers/pwm/pwm-lpc18xx-sct.c
+++ b/drivers/pwm/pwm-lpc18xx-sct.c
@@ -360,6 +360,8 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
 	}
 
 	lpc18xx_pwm->clk_rate = clk_get_rate(lpc18xx_pwm->pwm_clk);
+	if (!lpc18xx_pwm->clk_rate)
+		return -EINVAL;
 
 	mutex_init(&lpc18xx_pwm->res_lock);
 	mutex_init(&lpc18xx_pwm->period_lock);
-- 
2.7.0

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

* [PATCH 4/6] pwm: pwm-lpc18xx-sct: test clock rate to avoid division by 0
@ 2016-03-02 22:33   ` Wolfram Sang
  0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2016-03-02 22:33 UTC (permalink / raw)
  To: linux-arm-kernel

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The clk API may return 0 on clk_get_rate, so we should check the result before
using it as a divisor.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Should go individually via subsystem tree.

 drivers/pwm/pwm-lpc18xx-sct.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
index 9163085101bc94..6487962c355c03 100644
--- a/drivers/pwm/pwm-lpc18xx-sct.c
+++ b/drivers/pwm/pwm-lpc18xx-sct.c
@@ -360,6 +360,8 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
 	}
 
 	lpc18xx_pwm->clk_rate = clk_get_rate(lpc18xx_pwm->pwm_clk);
+	if (!lpc18xx_pwm->clk_rate)
+		return -EINVAL;
 
 	mutex_init(&lpc18xx_pwm->res_lock);
 	mutex_init(&lpc18xx_pwm->period_lock);
-- 
2.7.0

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

* [PATCH 5/6] watchdog: atlas7_wdt: test clock rate to avoid division by 0
  2016-03-02 22:33 ` Wolfram Sang
                   ` (4 preceding siblings ...)
  (?)
@ 2016-03-02 22:33 ` Wolfram Sang
  2016-03-03  4:25   ` Guenter Roeck
  2016-03-06  9:09   ` Wim Van Sebroeck
  -1 siblings, 2 replies; 22+ messages in thread
From: Wolfram Sang @ 2016-03-02 22:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-renesas-soc, Wim Van Sebroeck, Guenter Roeck,
	linux-watchdog

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The clk API may return 0 on clk_get_rate, so we should check the result before
using it as a divisor.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Should go individually via subsystem tree.

 drivers/watchdog/atlas7_wdt.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/watchdog/atlas7_wdt.c b/drivers/watchdog/atlas7_wdt.c
index df6d9242a31958..ed80734befae16 100644
--- a/drivers/watchdog/atlas7_wdt.c
+++ b/drivers/watchdog/atlas7_wdt.c
@@ -154,6 +154,11 @@ static int atlas7_wdt_probe(struct platform_device *pdev)
 	writel(0, wdt->base + ATLAS7_WDT_CNT_CTRL);
 
 	wdt->tick_rate = clk_get_rate(clk);
+	if (!wdt->tick_rate) {
+		ret = -EINVAL;
+		goto err1;
+	}
+
 	wdt->clk = clk;
 	atlas7_wdd.min_timeout = 1;
 	atlas7_wdd.max_timeout = UINT_MAX / wdt->tick_rate;
-- 
2.7.0

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

* [PATCH 6/6] watchdog: tangox_wdt: test clock rate to avoid division by 0
  2016-03-02 22:33 ` Wolfram Sang
                   ` (5 preceding siblings ...)
  (?)
@ 2016-03-02 22:33 ` Wolfram Sang
  2016-03-03  4:26   ` Guenter Roeck
  -1 siblings, 1 reply; 22+ messages in thread
From: Wolfram Sang @ 2016-03-02 22:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-renesas-soc, Wim Van Sebroeck, Guenter Roeck,
	linux-watchdog

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The clk API may return 0 on clk_get_rate, so we should check the result before
using it as a divisor.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Should go individually via subsystem tree.

 drivers/watchdog/tangox_wdt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/watchdog/tangox_wdt.c b/drivers/watchdog/tangox_wdt.c
index 709c1ed6fd79b9..cc702718ae4dbb 100644
--- a/drivers/watchdog/tangox_wdt.c
+++ b/drivers/watchdog/tangox_wdt.c
@@ -139,6 +139,8 @@ static int tangox_wdt_probe(struct platform_device *pdev)
 		return err;
 
 	dev->clk_rate = clk_get_rate(dev->clk);
+	if (!dev->clk_rate)
+		return -EINVAL;
 
 	dev->wdt.parent = &pdev->dev;
 	dev->wdt.info = &tangox_wdt_info;
-- 
2.7.0

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

* Re: [PATCH 4/6] pwm: pwm-lpc18xx-sct: test clock rate to avoid division by 0
  2016-03-02 22:33   ` Wolfram Sang
@ 2016-03-02 22:44     ` Joachim Eastwood
  -1 siblings, 0 replies; 22+ messages in thread
From: Joachim Eastwood @ 2016-03-02 22:44 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-kernel@vger.kernel.org, linux-renesas-soc, Thierry Reding,
	linux-pwm, linux-arm-kernel@lists.infradead.org,
	Ariel D'Alessandro

Hi Wolfram,

On 2 March 2016 at 23:33, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> The clk API may return 0 on clk_get_rate, so we should check the result before
> using it as a divisor.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> Should go individually via subsystem tree.
>
>  drivers/pwm/pwm-lpc18xx-sct.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
> index 9163085101bc94..6487962c355c03 100644
> --- a/drivers/pwm/pwm-lpc18xx-sct.c
> +++ b/drivers/pwm/pwm-lpc18xx-sct.c
> @@ -360,6 +360,8 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
>         }
>
>         lpc18xx_pwm->clk_rate = clk_get_rate(lpc18xx_pwm->pwm_clk);
> +       if (!lpc18xx_pwm->clk_rate)
> +               return -EINVAL;

This needs to be:
if (!lpc18xx_pwm->clk_rate) {
    ret = -EINVAL;
    goto disable_pwmclk;
}

I would also prefer an explicit check against 0 here. ie.:
'lpc18xx_pwm->clk_rate == 0'
A dev_err() message would also be nice to have.


regards,
Joachim Eastwood

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

* [PATCH 4/6] pwm: pwm-lpc18xx-sct: test clock rate to avoid division by 0
@ 2016-03-02 22:44     ` Joachim Eastwood
  0 siblings, 0 replies; 22+ messages in thread
From: Joachim Eastwood @ 2016-03-02 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Wolfram,

On 2 March 2016 at 23:33, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> The clk API may return 0 on clk_get_rate, so we should check the result before
> using it as a divisor.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> Should go individually via subsystem tree.
>
>  drivers/pwm/pwm-lpc18xx-sct.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
> index 9163085101bc94..6487962c355c03 100644
> --- a/drivers/pwm/pwm-lpc18xx-sct.c
> +++ b/drivers/pwm/pwm-lpc18xx-sct.c
> @@ -360,6 +360,8 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
>         }
>
>         lpc18xx_pwm->clk_rate = clk_get_rate(lpc18xx_pwm->pwm_clk);
> +       if (!lpc18xx_pwm->clk_rate)
> +               return -EINVAL;

This needs to be:
if (!lpc18xx_pwm->clk_rate) {
    ret = -EINVAL;
    goto disable_pwmclk;
}

I would also prefer an explicit check against 0 here. ie.:
'lpc18xx_pwm->clk_rate == 0'
A dev_err() message would also be nice to have.


regards,
Joachim Eastwood

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

* Re: [PATCH 4/6] pwm: pwm-lpc18xx-sct: test clock rate to avoid division by 0
  2016-03-02 22:44     ` Joachim Eastwood
@ 2016-03-02 22:58       ` Wolfram Sang
  -1 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2016-03-02 22:58 UTC (permalink / raw)
  To: Joachim Eastwood
  Cc: linux-kernel@vger.kernel.org, linux-renesas-soc, Thierry Reding,
	linux-pwm, linux-arm-kernel@lists.infradead.org,
	Ariel D'Alessandro

[-- Attachment #1: Type: text/plain, Size: 1316 bytes --]

On Wed, Mar 02, 2016 at 11:44:02PM +0100, Joachim Eastwood wrote:
> Hi Wolfram,
> 
> On 2 March 2016 at 23:33, Wolfram Sang <wsa@the-dreams.de> wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >
> > The clk API may return 0 on clk_get_rate, so we should check the result before
> > using it as a divisor.
> >
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >
> > Should go individually via subsystem tree.
> >
> >  drivers/pwm/pwm-lpc18xx-sct.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
> > index 9163085101bc94..6487962c355c03 100644
> > --- a/drivers/pwm/pwm-lpc18xx-sct.c
> > +++ b/drivers/pwm/pwm-lpc18xx-sct.c
> > @@ -360,6 +360,8 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
> >         }
> >
> >         lpc18xx_pwm->clk_rate = clk_get_rate(lpc18xx_pwm->pwm_clk);
> > +       if (!lpc18xx_pwm->clk_rate)
> > +               return -EINVAL;
> 
> This needs to be:
> if (!lpc18xx_pwm->clk_rate) {
>     ret = -EINVAL;
>     goto disable_pwmclk;
> }

Yes, that slipped through. Thanks!

> I would also prefer an explicit check against 0 here. ie.:

Well, I like the reading "if not rate then error"

Will send V2 now...


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* [PATCH 4/6] pwm: pwm-lpc18xx-sct: test clock rate to avoid division by 0
@ 2016-03-02 22:58       ` Wolfram Sang
  0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2016-03-02 22:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 02, 2016 at 11:44:02PM +0100, Joachim Eastwood wrote:
> Hi Wolfram,
> 
> On 2 March 2016 at 23:33, Wolfram Sang <wsa@the-dreams.de> wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >
> > The clk API may return 0 on clk_get_rate, so we should check the result before
> > using it as a divisor.
> >
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >
> > Should go individually via subsystem tree.
> >
> >  drivers/pwm/pwm-lpc18xx-sct.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
> > index 9163085101bc94..6487962c355c03 100644
> > --- a/drivers/pwm/pwm-lpc18xx-sct.c
> > +++ b/drivers/pwm/pwm-lpc18xx-sct.c
> > @@ -360,6 +360,8 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
> >         }
> >
> >         lpc18xx_pwm->clk_rate = clk_get_rate(lpc18xx_pwm->pwm_clk);
> > +       if (!lpc18xx_pwm->clk_rate)
> > +               return -EINVAL;
> 
> This needs to be:
> if (!lpc18xx_pwm->clk_rate) {
>     ret = -EINVAL;
>     goto disable_pwmclk;
> }

Yes, that slipped through. Thanks!

> I would also prefer an explicit check against 0 here. ie.:

Well, I like the reading "if not rate then error"

Will send V2 now...

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160302/e24b2786/attachment.sig>

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

* Re: [PATCH 5/6] watchdog: atlas7_wdt: test clock rate to avoid division by 0
  2016-03-02 22:33 ` [PATCH 5/6] watchdog: atlas7_wdt: " Wolfram Sang
@ 2016-03-03  4:25   ` Guenter Roeck
  2016-03-06  9:09   ` Wim Van Sebroeck
  1 sibling, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2016-03-03  4:25 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-kernel, linux-renesas-soc, Wim Van Sebroeck, linux-watchdog

On Wed, Mar 02, 2016 at 11:33:36PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> The clk API may return 0 on clk_get_rate, so we should check the result before
> using it as a divisor.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Should go individually via subsystem tree.
> 
>  drivers/watchdog/atlas7_wdt.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/watchdog/atlas7_wdt.c b/drivers/watchdog/atlas7_wdt.c
> index df6d9242a31958..ed80734befae16 100644
> --- a/drivers/watchdog/atlas7_wdt.c
> +++ b/drivers/watchdog/atlas7_wdt.c
> @@ -154,6 +154,11 @@ static int atlas7_wdt_probe(struct platform_device *pdev)
>  	writel(0, wdt->base + ATLAS7_WDT_CNT_CTRL);
>  
>  	wdt->tick_rate = clk_get_rate(clk);
> +	if (!wdt->tick_rate) {
> +		ret = -EINVAL;
> +		goto err1;
> +	}
> +
>  	wdt->clk = clk;
>  	atlas7_wdd.min_timeout = 1;
>  	atlas7_wdd.max_timeout = UINT_MAX / wdt->tick_rate;
> -- 
> 2.7.0
> 

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

* Re: [PATCH 6/6] watchdog: tangox_wdt: test clock rate to avoid division by 0
  2016-03-02 22:33 ` [PATCH 6/6] watchdog: tangox_wdt: " Wolfram Sang
@ 2016-03-03  4:26   ` Guenter Roeck
  2016-03-03  8:12     ` Wolfram Sang
  0 siblings, 1 reply; 22+ messages in thread
From: Guenter Roeck @ 2016-03-03  4:26 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-kernel, linux-renesas-soc, Wim Van Sebroeck, linux-watchdog

On Wed, Mar 02, 2016 at 11:33:37PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> The clk API may return 0 on clk_get_rate, so we should check the result before
> using it as a divisor.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> Should go individually via subsystem tree.
> 
>  drivers/watchdog/tangox_wdt.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/watchdog/tangox_wdt.c b/drivers/watchdog/tangox_wdt.c
> index 709c1ed6fd79b9..cc702718ae4dbb 100644
> --- a/drivers/watchdog/tangox_wdt.c
> +++ b/drivers/watchdog/tangox_wdt.c
> @@ -139,6 +139,8 @@ static int tangox_wdt_probe(struct platform_device *pdev)
>  		return err;
>  
>  	dev->clk_rate = clk_get_rate(dev->clk);
> +	if (!dev->clk_rate)
> +		return -EINVAL;

May be a nitpick, but clk_disable_unprepare() is missing.

Guenter

>  
>  	dev->wdt.parent = &pdev->dev;
>  	dev->wdt.info = &tangox_wdt_info;
> -- 
> 2.7.0
> 

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

* Re: [PATCH 6/6] watchdog: tangox_wdt: test clock rate to avoid division by 0
  2016-03-03  4:26   ` Guenter Roeck
@ 2016-03-03  8:12     ` Wolfram Sang
  0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2016-03-03  8:12 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-kernel, linux-renesas-soc, Wim Van Sebroeck, linux-watchdog

[-- Attachment #1: Type: text/plain, Size: 183 bytes --]

> > +	if (!dev->clk_rate)
> > +		return -EINVAL;
> 
> May be a nitpick, but clk_disable_unprepare() is missing.

Oh, definately not a nitpick in my book, will resend! Thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH 2/6] net: ethernet: renesas: ravb_main: test clock rate to avoid division by 0
  2016-03-02 22:33 ` [PATCH 2/6] net: ethernet: renesas: ravb_main: " Wolfram Sang
@ 2016-03-03 11:14   ` Sergei Shtylyov
  2016-04-03 21:36   ` Wolfram Sang
  1 sibling, 0 replies; 22+ messages in thread
From: Sergei Shtylyov @ 2016-03-03 11:14 UTC (permalink / raw)
  To: Wolfram Sang, linux-kernel; +Cc: linux-renesas-soc, netdev

Hello.

On 3/3/2016 1:33 AM, Wolfram Sang wrote:

> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> The clk API may return 0 on clk_get_rate, so we should check the result before
> using it as a divisor.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

MBR, Sergei

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

* Re: [PATCH 3/6] pwm: pwm-img: test clock rate to avoid division by 0
  2016-03-02 22:33 ` [PATCH 3/6] pwm: pwm-img: " Wolfram Sang
@ 2016-03-04 15:01   ` Thierry Reding
  0 siblings, 0 replies; 22+ messages in thread
From: Thierry Reding @ 2016-03-04 15:01 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-kernel, linux-renesas-soc, linux-pwm

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

On Wed, Mar 02, 2016 at 11:33:34PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> The clk API may return 0 on clk_get_rate, so we should check the result before
> using it as a divisor.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> Should go individually via subsystem tree.
> 
>  drivers/pwm/pwm-img.c | 5 +++++
>  1 file changed, 5 insertions(+)

Applied, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH 5/6] watchdog: atlas7_wdt: test clock rate to avoid division by 0
  2016-03-02 22:33 ` [PATCH 5/6] watchdog: atlas7_wdt: " Wolfram Sang
  2016-03-03  4:25   ` Guenter Roeck
@ 2016-03-06  9:09   ` Wim Van Sebroeck
  1 sibling, 0 replies; 22+ messages in thread
From: Wim Van Sebroeck @ 2016-03-06  9:09 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-kernel, linux-renesas-soc, Guenter Roeck, linux-watchdog

Hi Wolfram,

> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> The clk API may return 0 on clk_get_rate, so we should check the result before
> using it as a divisor.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> Should go individually via subsystem tree.
> 
>  drivers/watchdog/atlas7_wdt.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/watchdog/atlas7_wdt.c b/drivers/watchdog/atlas7_wdt.c
> index df6d9242a31958..ed80734befae16 100644
> --- a/drivers/watchdog/atlas7_wdt.c
> +++ b/drivers/watchdog/atlas7_wdt.c
> @@ -154,6 +154,11 @@ static int atlas7_wdt_probe(struct platform_device *pdev)
>  	writel(0, wdt->base + ATLAS7_WDT_CNT_CTRL);
>  
>  	wdt->tick_rate = clk_get_rate(clk);
> +	if (!wdt->tick_rate) {
> +		ret = -EINVAL;
> +		goto err1;
> +	}
> +
>  	wdt->clk = clk;
>  	atlas7_wdd.min_timeout = 1;
>  	atlas7_wdd.max_timeout = UINT_MAX / wdt->tick_rate;
> -- 
> 2.7.0
> 

Patch has been added to linux-watchdog-next.

Kind regards,
Wim.

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

* Re: [PATCH 2/6] net: ethernet: renesas: ravb_main: test clock rate to avoid division by 0
  2016-03-02 22:33 ` [PATCH 2/6] net: ethernet: renesas: ravb_main: " Wolfram Sang
  2016-03-03 11:14   ` Sergei Shtylyov
@ 2016-04-03 21:36   ` Wolfram Sang
  2016-04-03 22:28     ` Sergei Shtylyov
  1 sibling, 1 reply; 22+ messages in thread
From: Wolfram Sang @ 2016-04-03 21:36 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-renesas-soc, Sergei Shtylyov, netdev

[-- Attachment #1: Type: text/plain, Size: 971 bytes --]

On Wed, Mar 02, 2016 at 11:33:33PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> The clk API may return 0 on clk_get_rate, so we should check the result before
> using it as a divisor.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Ping.

> ---
> 
> Should go individually via subsystem tree.
> 
>  drivers/net/ethernet/renesas/ravb_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 88656ceb6e2946..ce1954a6a12726 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1691,6 +1691,9 @@ static int ravb_set_gti(struct net_device *ndev)
>  	rate = clk_get_rate(clk);
>  	clk_put(clk);
>  
> +	if (!rate)
> +		return -EINVAL;
> +
>  	inc = 1000000000ULL << 20;
>  	do_div(inc, rate);
>  
> -- 
> 2.7.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH 2/6] net: ethernet: renesas: ravb_main: test clock rate to avoid division by 0
  2016-04-03 21:36   ` Wolfram Sang
@ 2016-04-03 22:28     ` Sergei Shtylyov
  0 siblings, 0 replies; 22+ messages in thread
From: Sergei Shtylyov @ 2016-04-03 22:28 UTC (permalink / raw)
  To: Wolfram Sang, linux-kernel; +Cc: linux-renesas-soc, netdev, David Miller

On 04/04/2016 12:36 AM, Wolfram Sang wrote:

>> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>
>> The clk API may return 0 on clk_get_rate, so we should check the result before
>> using it as a divisor.
>>
>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Ping.

    http://patchwork.ozlabs.org/patch/591160/

    I have no idea why it's marked this way... Dave?

MBR, Sergei

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

end of thread, other threads:[~2016-04-03 22:28 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-02 22:33 [PATCH 0/6] treewide: test clock rate to avoid division by 0 Wolfram Sang
2016-03-02 22:33 ` Wolfram Sang
2016-03-02 22:33 ` [PATCH 1/6] ide: palm_bk3710: " Wolfram Sang
2016-03-02 22:33 ` [PATCH 2/6] net: ethernet: renesas: ravb_main: " Wolfram Sang
2016-03-03 11:14   ` Sergei Shtylyov
2016-04-03 21:36   ` Wolfram Sang
2016-04-03 22:28     ` Sergei Shtylyov
2016-03-02 22:33 ` [PATCH 3/6] pwm: pwm-img: " Wolfram Sang
2016-03-04 15:01   ` Thierry Reding
2016-03-02 22:33 ` [PATCH 4/6] pwm: pwm-lpc18xx-sct: " Wolfram Sang
2016-03-02 22:33   ` Wolfram Sang
2016-03-02 22:33   ` Wolfram Sang
2016-03-02 22:44   ` Joachim Eastwood
2016-03-02 22:44     ` Joachim Eastwood
2016-03-02 22:58     ` Wolfram Sang
2016-03-02 22:58       ` Wolfram Sang
2016-03-02 22:33 ` [PATCH 5/6] watchdog: atlas7_wdt: " Wolfram Sang
2016-03-03  4:25   ` Guenter Roeck
2016-03-06  9:09   ` Wim Van Sebroeck
2016-03-02 22:33 ` [PATCH 6/6] watchdog: tangox_wdt: " Wolfram Sang
2016-03-03  4:26   ` Guenter Roeck
2016-03-03  8:12     ` Wolfram Sang

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.