2016-03-02 22:33:52

by Wolfram Sang

Subject: [PATCH 0/6] treewide: test clock rate to avoid division by 0

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

2016-03-02 22:33:53

by Wolfram Sang

Subject: [PATCH 1/6] ide: palm_bk3710: test clock rate to avoid division by 0

From: Wolfram Sang <[email protected]>

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 <[email protected]>
---

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

2016-03-02 22:34:16

by Wolfram Sang

Subject: [PATCH 6/6] watchdog: tangox_wdt: test clock rate to avoid division by 0

From: Wolfram Sang <[email protected]>

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 <[email protected]>
---

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

2016-03-02 22:34:36

by Wolfram Sang

Subject: [PATCH 5/6] watchdog: atlas7_wdt: test clock rate to avoid division by 0

From: Wolfram Sang <[email protected]>

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 <[email protected]>
---

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

2016-03-02 22:33:51

by Wolfram Sang

Subject: [PATCH 2/6] net: ethernet: renesas: ravb_main: test clock rate to avoid division by 0

From: Wolfram Sang <[email protected]>

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 <[email protected]>
---

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

2016-03-02 22:34:58

by Wolfram Sang

Subject: [PATCH 4/6] pwm: pwm-lpc18xx-sct: test clock rate to avoid division by 0

From: Wolfram Sang <[email protected]>

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 <[email protected]>
---

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

2016-03-02 22:35:24

by Wolfram Sang

Subject: [PATCH 3/6] pwm: pwm-img: test clock rate to avoid division by 0

From: Wolfram Sang <[email protected]>

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 <[email protected]>
---

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

2016-03-02 22:44:06

by Joachim Eastwood

Subject: Re: [PATCH 4/6] pwm: pwm-lpc18xx-sct: test clock rate to avoid division by 0

Hi Wolfram,

On 2 March 2016 at 23:33, Wolfram Sang <[email protected]> wrote:
> From: Wolfram Sang <[email protected]>
>
> 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 <[email protected]>
> ---
>
> 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

2016-03-02 22:59:03

by Wolfram Sang

Subject: Re: [PATCH 4/6] pwm: pwm-lpc18xx-sct: test clock rate to avoid division by 0

On Wed, Mar 02, 2016 at 11:44:02PM +0100, Joachim Eastwood wrote:
> Hi Wolfram,
>
> On 2 March 2016 at 23:33, Wolfram Sang <[email protected]> wrote:
> > From: Wolfram Sang <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> >
> > 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...

Attachments:

2016-03-03 04:25:10

by Guenter Roeck

Subject: Re: [PATCH 5/6] watchdog: atlas7_wdt: test clock rate to avoid division by 0

On Wed, Mar 02, 2016 at 11:33:36PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <[email protected]>
>
> 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 <[email protected]>

Reviewed-by: Guenter Roeck <[email protected]>

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

2016-03-03 04:26:22

by Guenter Roeck

Subject: Re: [PATCH 6/6] watchdog: tangox_wdt: test clock rate to avoid division by 0

On Wed, Mar 02, 2016 at 11:33:37PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <[email protected]>
>
> 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 <[email protected]>
> ---
>
> 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
>

2016-03-03 08:12:53

by Wolfram Sang

Subject: Re: [PATCH 6/6] watchdog: tangox_wdt: test clock rate to avoid division by 0

> > + 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!

Attachments:

2016-03-03 11:14:27

by Sergei Shtylyov

Subject: Re: [PATCH 2/6] net: ethernet: renesas: ravb_main: test clock rate to avoid division by 0

Hello.

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

> From: Wolfram Sang <[email protected]>
>
> 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 <[email protected]>

Acked-by: Sergei Shtylyov <[email protected]>

MBR, Sergei

2016-03-04 15:02:57

by Thierry Reding

Subject: Re: [PATCH 3/6] pwm: pwm-img: test clock rate to avoid division by 0

On Wed, Mar 02, 2016 at 11:33:34PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <[email protected]>
>
> 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 <[email protected]>
> ---
>
> Should go individually via subsystem tree.
>
> drivers/pwm/pwm-img.c | 5 +++++
> 1 file changed, 5 insertions(+)

Applied, thanks.

Thierry

Attachments:

2016-03-06 09:09:33

by Wim Van Sebroeck

Subject: Re: [PATCH 5/6] watchdog: atlas7_wdt: test clock rate to avoid division by 0

Hi Wolfram,

> From: Wolfram Sang <[email protected]>
>
> 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 <[email protected]>
> ---
>
> 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.

2016-04-03 21:36:53

by Wolfram Sang

Subject: Re: [PATCH 2/6] net: ethernet: renesas: ravb_main: test clock rate to avoid division by 0

On Wed, Mar 02, 2016 at 11:33:33PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <[email protected]>
>
> 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 <[email protected]>

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

Attachments:

2016-04-03 22:28:51

by Sergei Shtylyov

Subject: Re: [PATCH 2/6] net: ethernet: renesas: ravb_main: test clock rate to avoid division by 0

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

>> From: Wolfram Sang <[email protected]>
>>
>> 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 <[email protected]>
>
> Ping.

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

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

MBR, Sergei