Re: [PATCH 2/2] watchdog: imx7ulp_wdt: Add TOVAL range check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 27.10.24 um 14:36 schrieb Guenter Roeck:
On 10/27/24 03:53, Stefan Wahren wrote:
The WDOG Timeout Value (TOVAL) is a 16 bit value, which is stored
at the beginning of a 32 bit register. So add a range check to
prevent writing in the reserved register area.

Signed-off-by: Stefan Wahren <wahrenst@xxxxxxx>
---
  drivers/watchdog/imx7ulp_wdt.c | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/drivers/watchdog/imx7ulp_wdt.c
b/drivers/watchdog/imx7ulp_wdt.c
index 0f92d2217088..a7574f9c9150 100644
--- a/drivers/watchdog/imx7ulp_wdt.c
+++ b/drivers/watchdog/imx7ulp_wdt.c
@@ -48,6 +48,8 @@

  #define RETRY_MAX 5

+#define TOVAL_MAX    0xFFFF
+
  static bool nowayout = WATCHDOG_NOWAYOUT;
  module_param(nowayout, bool, 0000);
  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started
(default="
@@ -192,6 +194,9 @@ static int imx7ulp_wdt_set_timeout(struct
watchdog_device *wdog,
      int ret;
      u32 loop = RETRY_MAX;

+    if (toval > TOVAL_MAX)
+        return -EINVAL;
+

The whole idea of having max_timeout in struct watchdog_device is to
avoid the need
for this check. max_timeout should be set to 0xffff /
wdt->hw->wdog_clock_rate.
It is currently set to 128. With wdt->hw->wdog_clock_rate set to
either 125 or 1000,
it can indeed overflow. However, checking the value above is wrong.
max_timeout should
be initialized correctly instead.

Even better would be to set max_hw_heartbeat_ms and let the watchdog
core handle
larger timeouts.
It's funny because I tried this on a i.MX93 board but it didn't work for
me. But I must confess that I didn't spend much time in the investigation.

Another question is why the driver enables a clock but doesn't use its
actual
frequency.
Yes, this would be better

Regards

Guenter








[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux