[PATCH v2] watchdog: dw_wdt: fix overflow issue in dw_wdt_top_in_seconds

[PATCH v2] watchdog: dw_wdt: fix overflow issue in dw_wdt_top_in_seconds

[Huibin Hong]

If top is 15, (1 << (16 + top)) may be negative.

Signed-off-by: Huibin Hong <huibin.hong@rock-chips.com>
---

Changes in v2:
    - Rebase mainline Linux 4.13-rc5

 drivers/watchdog/dw_wdt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 36be987..59ed958 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -72,7 +72,9 @@ static inline int dw_wdt_top_in_seconds(struct dw_wdt *dw_wdt, unsigned top)
 	 * There are 16 possible timeout values in 0..15 where the number of
 	 * cycles is 2 ^ (16 + i) and the watchdog counts down.
 	 */
-	return (1U << (16 + top)) / dw_wdt->rate;
+	unsigned int cycles = 1 << (16 + top);
+
+	return (cycles / clk_get_rate(dw_wdt.clk));
 }
 
 static int dw_wdt_get_top(struct dw_wdt *dw_wdt)
-- 
1.9.1

Re: [PATCH v2] watchdog: dw_wdt: fix overflow issue in dw_wdt_top_in_seconds

[Guenter Roeck]

On 08/14/2017 07:45 PM, Huibin Hong wrote:
> If top is 15, (1 << (16 + top)) may be negative.
> 
> Signed-off-by: Huibin Hong <huibin.hong@rock-chips.com>
> ---
> 
> Changes in v2:
>      - Rebase mainline Linux 4.13-rc5
> 
>   drivers/watchdog/dw_wdt.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index 36be987..59ed958 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -72,7 +72,9 @@ static inline int dw_wdt_top_in_seconds(struct dw_wdt *dw_wdt, unsigned top)
>   	 * There are 16 possible timeout values in 0..15 where the number of
>   	 * cycles is 2 ^ (16 + i) and the watchdog counts down.
>   	 */
> -	return (1U << (16 + top)) / dw_wdt->rate;
> +	unsigned int cycles = 1 << (16 + top);
> +
> +	return (cycles / clk_get_rate(dw_wdt.clk));

Sorry I didn't catch this earlier. Why not use dw_wdt->rate ?
It has been checked against 0, and presumably the rate does not change
on its own. If it can change, you'll have to submit a separate patch
dropping the use of dw_wdt->rate entirely and explaining the reason.
You'll also have to make sure that the driver somehow catches the
rate change and adjusts its settings automatically; otherwise, timeouts
will be off target, and the system can reset unexpectedly.

Thanks,
Guenter

>   }
>   
>   static int dw_wdt_get_top(struct dw_wdt *dw_wdt)
> 

Re: [PATCH v2] watchdog: dw_wdt: fix overflow issue in dw_wdt_top_in_seconds

[kbuild test robot]

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

Hi Huibin,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.13-rc5 next-20170816]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Huibin-Hong/watchdog-dw_wdt-fix-overflow-issue-in-dw_wdt_top_in_seconds/20170817-033555
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All error/warnings (new ones prefixed by >>):

   drivers//watchdog/dw_wdt.c: In function 'dw_wdt_top_in_seconds':
>> drivers//watchdog/dw_wdt.c:77:38: error: request for member 'clk' in something not a structure or union
     return (cycles / clk_get_rate(dw_wdt.clk));
                                         ^
>> drivers//watchdog/dw_wdt.c:78:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^

vim +/clk +77 drivers//watchdog/dw_wdt.c

    68	
    69	static inline int dw_wdt_top_in_seconds(struct dw_wdt *dw_wdt, unsigned top)
    70	{
    71		/*
    72		 * There are 16 possible timeout values in 0..15 where the number of
    73		 * cycles is 2 ^ (16 + i) and the watchdog counts down.
    74		 */
    75		unsigned int cycles = 1 << (16 + top);
    76	
  > 77		return (cycles / clk_get_rate(dw_wdt.clk));
  > 78	}
    79	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50926 bytes --]

Re: [PATCH v2] watchdog: dw_wdt: fix overflow issue in dw_wdt_top_in_seconds

[kbuild test robot]

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

Hi Huibin,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.13-rc5 next-20170816]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Huibin-Hong/watchdog-dw_wdt-fix-overflow-issue-in-dw_wdt_top_in_seconds/20170817-033555
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   drivers/watchdog/dw_wdt.c: In function 'dw_wdt_top_in_seconds':
>> drivers/watchdog/dw_wdt.c:77:38: error: 'dw_wdt' is a pointer; did you mean to use '->'?
     return (cycles / clk_get_rate(dw_wdt.clk));
                                         ^
                                         ->
   drivers/watchdog/dw_wdt.c:78:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^

vim +77 drivers/watchdog/dw_wdt.c

    68	
    69	static inline int dw_wdt_top_in_seconds(struct dw_wdt *dw_wdt, unsigned top)
    70	{
    71		/*
    72		 * There are 16 possible timeout values in 0..15 where the number of
    73		 * cycles is 2 ^ (16 + i) and the watchdog counts down.
    74		 */
    75		unsigned int cycles = 1 << (16 + top);
    76	
  > 77		return (cycles / clk_get_rate(dw_wdt.clk));
    78	}
    79	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51272 bytes --]