[PATCH v2 1/8] watchdog: core: Clear wd_data pointer on errors

[PATCH v2 1/8] watchdog: core: Clear wd_data pointer on errors

[Guenter Roeck]

In watchdog_cdev_register(), if device registration fails after the core
watchdog_core_data structure is allocated and assigned to the persistent
watchdog_device structure, the function frees the data but leaves a
dangling pointer.

During device registration, watchdog_cdev_register() links the newly
allocated wd_data to wdd->wd_data. If a subsequent initialization step
fails, such as the watchdog_kworker validation, dev_set_name(),
misc_register(), or cdev_device_add(), the function cleans up by freeing
wd_data via kfree() or put_device(). However, it fails to clear the
wdd->wd_data pointer before returning. Furthermore, if cdev_device_add()
fails after misc_register() exposed /dev/watchdog to userspace, a
concurrent open may hold a reference to wd_data while the caller frees wdd,
leaving wd_data->wdd dangling.

Fix the problem by clearing wdd->wd_data on all error paths during
watchdog registration, and clearing wd_data->wdd under wd_data->lock if
cdev_device_add() fails.

Fixes: b4ffb1909843 ("watchdog: Separate and maintain variables based on variable lifetime")
Assisted-by: LLM
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Added Reviewed-by: tag

 drivers/watchdog/watchdog_dev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index e8c1fcb53c56..ed5b35184271 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -1033,6 +1033,7 @@ static int watchdog_cdev_register(struct watchdog_device *wdd)
 	wdd->wd_data = wd_data;
 
 	if (IS_ERR_OR_NULL(watchdog_kworker)) {
+		wdd->wd_data = NULL;
 		kfree(wd_data);
 		return -ENODEV;
 	}
@@ -1046,6 +1047,7 @@ static int watchdog_cdev_register(struct watchdog_device *wdd)
 	dev_set_drvdata(&wd_data->dev, wdd);
 	err = dev_set_name(&wd_data->dev, "watchdog%d", wdd->id);
 	if (err) {
+		wdd->wd_data = NULL;
 		put_device(&wd_data->dev);
 		return err;
 	}
@@ -1066,6 +1068,7 @@ static int watchdog_cdev_register(struct watchdog_device *wdd)
 				pr_err("%s: a legacy watchdog module is probably present.\n",
 					wdd->info->identity);
 			old_wd_data = NULL;
+			wdd->wd_data = NULL;
 			put_device(&wd_data->dev);
 			return err;
 		}
@@ -1084,6 +1087,10 @@ static int watchdog_cdev_register(struct watchdog_device *wdd)
 			misc_deregister(&watchdog_miscdev);
 			old_wd_data = NULL;
 		}
+		mutex_lock(&wd_data->lock);
+		wd_data->wdd = NULL;
+		wdd->wd_data = NULL;
+		mutex_unlock(&wd_data->lock);
 		put_device(&wd_data->dev);
 		return err;
 	}
-- 
2.45.2

[PATCH v2 0/8] watchdog: core: Fix locking, lifetime, suspend, and state management bugs

[Guenter Roeck]

This series fixes several locking, reference counting, object lifetime,
suspend/resume, and state management bugs in the watchdog core character
device and timer handling:

1. watchdog: core: Clear wd_data pointer on errors
   Clear wdd->wd_data on all registration error paths in
   watchdog_cdev_register(), and clear wd_data->wdd under wd_data->lock
   when failing after misc_register() may have exposed the device to
   userspace, preventing dangling pointers.

2. watchdog: core: Add missing locks
   Acquire wd_data->lock in watchdog_open(), across watchdog_stop() and
   pretimeout teardown in watchdog_cdev_unregister(), and in
   watchdog_set_last_hw_keepalive(). Also protect old_wd_data with
   old_wd_data_lock to prevent races when opening or unregistering
   /dev/watchdog.

3. watchdog: core: Prevent ping worker from re-arming timer on suspend
   Introduce a _WDOG_SUSPENDED flag in wd_data->status, set under
   wd_data->lock during suspend and cleared on resume, and check it in
   watchdog_worker_should_ping(), watchdog_need_worker(), and
   __watchdog_ping() so an in-flight worker or deferred ping cannot
   re-arm wd_data->timer while suspended.

4. watchdog: core: Stop pretimeout hrtimer on suspend
   Stop the software pretimeout hrtimer in watchdog_dev_suspend(),
   prevent watchdog_hrtimer_pretimeout_start() from arming it while
   _WDOG_SUSPENDED is set, and restart it in watchdog_dev_resume() if
   the hardware watchdog is running.

5. watchdog: core: Restore WDOG_HW_RUNNING if stopping watchdog fails
   In watchdog_stop(), WDOG_HW_RUNNING is cleared before calling
   wdd->ops->stop(). Restore WDOG_HW_RUNNING if wdd->ops->stop() returns
   an error so the core continues to track the running hardware state
   and retains its module and device references.

   v2: Move restore of WDOG_HW_RUNNING to stop() error handler

6. watchdog: core: Cancel timer if cdev_device_add() fails
   Initialize wd_data and acquire running-watchdog module and device
   references prior to calling misc_register(). On registration failure
   (and in watchdog_cdev_unregister()), stop the watchdog if appropriate,
   stop the pretimeout hrtimer, release unclaimed running-watchdog
   references if the device is not open, and cancel wd_data->timer and
   wd_data->work before dropping the initial device reference.

7. watchdog: core: Fix unbalanced module_put() in watchdog_open()
   In watchdog_open(), try_module_get() is skipped when hw_running is
   true because the module reference was already taken when the running
   watchdog was registered. Guard module_put() on the watchdog_start()
   error path with !hw_running to avoid dropping a reference that
   watchdog_open() did not acquire.

8. watchdog: core: Update last_keepalive in watchdog_start()
   When starting a watchdog whose hardware is already running, set
   WDOG_ACTIVE and update wd_data->last_keepalive to started_at before
   calling __watchdog_ping() so that watchdog_get_timeleft() reports the
   correct remaining time and watchdog_update_worker() inside
   __watchdog_ping() evaluates the active state and new keepalive
   timestamp rather than a potentially expired open_deadline. If
   __watchdog_ping() fails, clear WDOG_ACTIVE and update the worker.

Disclaimer: I started this series to fix a number of bugs reported
by Sashiko in the watchdog core. After several fix-review rounds, I did
not get closer to fixing all issues reported by Sashiko; either my patches
turned out to be incomplete or buggy. I finally gave up and fed Sashiko's
review feedback into an AI engine, asking it to fix the reported problems.
It still took some 10+ rounds of review/fix, but the resulting patches
should fix at least the most critical race conditions in the watchdog core.

Given the complexity of the changes, the plan is to apply the series
during the next commit window, to be released with v7.5, and to
eventually back-port it to older kernel branches.

----------------------------------------------------------------
Guenter Roeck (8):
      watchdog: core: Clear wd_data pointer on errors
      watchdog: core: Add missing locks
      watchdog: core: Prevent ping worker from re-arming timer on suspend
      watchdog: core: Stop pretimeout hrtimer on suspend
      watchdog: core: Restore WDOG_HW_RUNNING if stopping watchdog fails
      watchdog: core: Cancel timer if cdev_device_add() fails
      watchdog: core: Fix unbalanced module_put() in watchdog_open()
      watchdog: core: Update last_keepalive in watchdog_start()

 drivers/watchdog/watchdog_core.h               |   1 +
 drivers/watchdog/watchdog_dev.c                | 155 ++++++++++++++++++-------
 drivers/watchdog/watchdog_hrtimer_pretimeout.c |   3 +-
 3 files changed, 117 insertions(+), 42 deletions(-)

[PATCH v2 1/8] watchdog: core: Clear wd_data pointer on errors

[Guenter Roeck]

In watchdog_cdev_register(), if device registration fails after the core
watchdog_core_data structure is allocated and assigned to the persistent
watchdog_device structure, the function frees the data but leaves a
dangling pointer.

During device registration, watchdog_cdev_register() links the newly
allocated wd_data to wdd->wd_data. If a subsequent initialization step
fails, such as the watchdog_kworker validation, dev_set_name(),
misc_register(), or cdev_device_add(), the function cleans up by freeing
wd_data via kfree() or put_device(). However, it fails to clear the
wdd->wd_data pointer before returning. Furthermore, if cdev_device_add()
fails after misc_register() exposed /dev/watchdog to userspace, a
concurrent open may hold a reference to wd_data while the caller frees wdd,
leaving wd_data->wdd dangling.

Fix the problem by clearing wdd->wd_data on all error paths during
watchdog registration, and clearing wd_data->wdd under wd_data->lock if
cdev_device_add() fails.

Fixes: b4ffb1909843 ("watchdog: Separate and maintain variables based on variable lifetime")
Assisted-by: LLM
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Added Reviewed-by: tag

 drivers/watchdog/watchdog_dev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index e8c1fcb53c56..ed5b35184271 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -1033,6 +1033,7 @@ static int watchdog_cdev_register(struct watchdog_device *wdd)
 	wdd->wd_data = wd_data;
 
 	if (IS_ERR_OR_NULL(watchdog_kworker)) {
+		wdd->wd_data = NULL;
 		kfree(wd_data);
 		return -ENODEV;
 	}
@@ -1046,6 +1047,7 @@ static int watchdog_cdev_register(struct watchdog_device *wdd)
 	dev_set_drvdata(&wd_data->dev, wdd);
 	err = dev_set_name(&wd_data->dev, "watchdog%d", wdd->id);
 	if (err) {
+		wdd->wd_data = NULL;
 		put_device(&wd_data->dev);
 		return err;
 	}
@@ -1066,6 +1068,7 @@ static int watchdog_cdev_register(struct watchdog_device *wdd)
 				pr_err("%s: a legacy watchdog module is probably present.\n",
 					wdd->info->identity);
 			old_wd_data = NULL;
+			wdd->wd_data = NULL;
 			put_device(&wd_data->dev);
 			return err;
 		}
@@ -1084,6 +1087,10 @@ static int watchdog_cdev_register(struct watchdog_device *wdd)
 			misc_deregister(&watchdog_miscdev);
 			old_wd_data = NULL;
 		}
+		mutex_lock(&wd_data->lock);
+		wd_data->wdd = NULL;
+		wdd->wd_data = NULL;
+		mutex_unlock(&wd_data->lock);
 		put_device(&wd_data->dev);
 		return err;
 	}
-- 
2.45.2

[PATCH v2 2/8] watchdog: core: Add missing locks

[Guenter Roeck]

Add missing locks to watchdog_open(), watchdog_cdev_unregister(), and
watchdog_set_last_hw_keepalive(). Also protect old_wd_data with its own
lock to prevent its modification while in use, acquiring wd_data->lock
before releasing old_wd_data_lock in watchdog_open() to avoid a TOCTOU
race, and keeping misc_deregister() outside old_wd_data_lock to avoid
lock ordering inversion with misc_mtx.

Fixes: b4ffb1909843 ("watchdog: Separate and maintain variables based on variable lifetime")
Assisted-by: LLM
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Added Reviewed-by: tag

 drivers/watchdog/watchdog_dev.c | 42 ++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index ed5b35184271..50224877ff49 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -55,6 +55,7 @@
 static dev_t watchdog_devt;
 /* Reference to watchdog device behind /dev/watchdog */
 static struct watchdog_core_data *old_wd_data;
+static DEFINE_MUTEX(old_wd_data_lock);
 
 static struct kthread_worker *watchdog_kworker;
 
@@ -869,17 +870,32 @@ static int watchdog_open(struct inode *inode, struct file *file)
 	int err;
 
 	/* Get the corresponding watchdog device */
-	if (imajor(inode) == MISC_MAJOR)
+	if (imajor(inode) == MISC_MAJOR) {
+		mutex_lock(&old_wd_data_lock);
 		wd_data = old_wd_data;
-	else
+		if (!wd_data) {
+			mutex_unlock(&old_wd_data_lock);
+			return -ENODEV;
+		}
+		mutex_lock(&wd_data->lock);
+		mutex_unlock(&old_wd_data_lock);
+	} else {
 		wd_data = container_of(inode->i_cdev, struct watchdog_core_data,
 				       cdev);
+		mutex_lock(&wd_data->lock);
+	}
 
 	/* the watchdog is single open! */
-	if (test_and_set_bit(_WDOG_DEV_OPEN, &wd_data->status))
+	if (test_and_set_bit(_WDOG_DEV_OPEN, &wd_data->status)) {
+		mutex_unlock(&wd_data->lock);
 		return -EBUSY;
+	}
 
 	wdd = wd_data->wdd;
+	if (!wdd) {
+		err = -ENODEV;
+		goto out_clear;
+	}
 
 	/*
 	 * If the /dev/watchdog device is open, we don't want the module
@@ -909,6 +925,8 @@ static int watchdog_open(struct inode *inode, struct file *file)
 	 */
 	wd_data->open_deadline = KTIME_MAX;
 
+	mutex_unlock(&wd_data->lock);
+
 	/* dev/watchdog is a virtual (and thus non-seekable) filesystem */
 	return stream_open(inode, file);
 
@@ -916,6 +934,7 @@ static int watchdog_open(struct inode *inode, struct file *file)
 	module_put(wd_data->wdd->ops->owner);
 out_clear:
 	clear_bit(_WDOG_DEV_OPEN, &wd_data->status);
+	mutex_unlock(&wd_data->lock);
 	return err;
 }
 
@@ -1085,7 +1104,9 @@ static int watchdog_cdev_register(struct watchdog_device *wdd)
 			wdd->id,  MAJOR(watchdog_devt), wdd->id);
 		if (wdd->id == 0) {
 			misc_deregister(&watchdog_miscdev);
+			mutex_lock(&old_wd_data_lock);
 			old_wd_data = NULL;
+			mutex_unlock(&old_wd_data_lock);
 		}
 		mutex_lock(&wd_data->lock);
 		wd_data->wdd = NULL;
@@ -1131,9 +1152,12 @@ static void watchdog_cdev_unregister(struct watchdog_device *wdd)
 	cdev_device_del(&wd_data->cdev, &wd_data->dev);
 	if (wdd->id == 0) {
 		misc_deregister(&watchdog_miscdev);
+		mutex_lock(&old_wd_data_lock);
 		old_wd_data = NULL;
+		mutex_unlock(&old_wd_data_lock);
 	}
 
+	mutex_lock(&wd_data->lock);
 	if (watchdog_active(wdd) &&
 	    test_bit(WDOG_STOP_ON_UNREGISTER, &wdd->status)) {
 		watchdog_stop(wdd);
@@ -1141,7 +1165,6 @@ static void watchdog_cdev_unregister(struct watchdog_device *wdd)
 
 	watchdog_hrtimer_pretimeout_stop(wdd);
 
-	mutex_lock(&wd_data->lock);
 	wd_data->wdd = NULL;
 	wdd->wd_data = NULL;
 	mutex_unlock(&wd_data->lock);
@@ -1208,20 +1231,27 @@ int watchdog_set_last_hw_keepalive(struct watchdog_device *wdd,
 {
 	struct watchdog_core_data *wd_data;
 	ktime_t now;
+	int ret = 0;
 
 	if (!wdd)
 		return -EINVAL;
 
 	wd_data = wdd->wd_data;
+	if (!wd_data)
+		return -ENODEV;
+
+	mutex_lock(&wd_data->lock);
 
 	now = ktime_get();
 
 	wd_data->last_hw_keepalive = ktime_sub(now, ms_to_ktime(last_ping_ms));
 
 	if (watchdog_hw_running(wdd) && handle_boot_enabled)
-		return __watchdog_ping(wdd);
+		ret = __watchdog_ping(wdd);
 
-	return 0;
+	mutex_unlock(&wd_data->lock);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(watchdog_set_last_hw_keepalive);
 
-- 
2.45.2

[PATCH v2 3/8] watchdog: core: Prevent ping worker from re-arming timer on suspend

[Guenter Roeck]

In watchdog_dev_suspend(), hrtimer_cancel() is called before
kthread_cancel_work_sync(). If the timer expired just before
hrtimer_cancel() and queued watchdog_ping_work(), the worker can run
and acquire wd_data->lock while kthread_cancel_work_sync() waits for it
to finish. Because no state flag indicates that the watchdog is
suspended, watchdog_worker_should_ping() returns true and
__watchdog_ping() re-arms wd_data->timer while the device is suspended.

Add a _WDOG_SUSPENDED internal status bit to wd_data->status, set it
under wd_data->lock in watchdog_dev_suspend(), clear it under
wd_data->lock in watchdog_dev_resume(), and check it in
watchdog_worker_should_ping(), watchdog_need_worker(), and
__watchdog_ping() so the timer cannot be armed while suspended.

Fixes: 60bcd91aafd2 ("watchdog: introduce watchdog_dev_suspend/resume")
Assisted-by: LLM
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Added Reviewed-by: tag

 drivers/watchdog/watchdog_core.h |  1 +
 drivers/watchdog/watchdog_dev.c  | 15 +++++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/watchdog_core.h b/drivers/watchdog/watchdog_core.h
index ab825d9f9248..c6df5a584357 100644
--- a/drivers/watchdog/watchdog_core.h
+++ b/drivers/watchdog/watchdog_core.h
@@ -60,6 +60,7 @@ struct watchdog_core_data {
 #define _WDOG_DEV_OPEN		0	/* Opened ? */
 #define _WDOG_ALLOW_RELEASE	1	/* Did we receive the magic char ? */
 #define _WDOG_KEEPALIVE		2	/* Did we receive a keepalive ? */
+#define _WDOG_SUSPENDED		3	/* Suspended ? */
 };
 
 /*
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 50224877ff49..e38f1ef8e683 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -81,6 +81,9 @@ static inline bool watchdog_need_worker(struct watchdog_device *wdd)
 	unsigned int hm = wdd->max_hw_heartbeat_ms;
 	unsigned int t = wdd->timeout * 1000;
 
+	if (test_bit(_WDOG_SUSPENDED, &wdd->wd_data->status))
+		return false;
+
 	/*
 	 * A worker to generate heartbeat requests is needed if all of the
 	 * following conditions are true.
@@ -154,9 +157,10 @@ static int __watchdog_ping(struct watchdog_device *wdd)
 	now = ktime_get();
 
 	if (ktime_after(earliest_keepalive, now)) {
-		hrtimer_start(&wd_data->timer,
-			      ktime_sub(earliest_keepalive, now),
-			      HRTIMER_MODE_REL_HARD);
+		if (!test_bit(_WDOG_SUSPENDED, &wd_data->status))
+			hrtimer_start(&wd_data->timer,
+				      ktime_sub(earliest_keepalive, now),
+				      HRTIMER_MODE_REL_HARD);
 		return 0;
 	}
 
@@ -207,7 +211,7 @@ static bool watchdog_worker_should_ping(struct watchdog_core_data *wd_data)
 {
 	struct watchdog_device *wdd = wd_data->wdd;
 
-	if (!wdd)
+	if (!wdd || test_bit(_WDOG_SUSPENDED, &wd_data->status))
 		return false;
 
 	if (watchdog_active(wdd))
@@ -1318,6 +1322,8 @@ int watchdog_dev_suspend(struct watchdog_device *wdd)
 	mutex_lock(&wd_data->lock);
 	if (watchdog_worker_should_ping(wd_data))
 		ret = __watchdog_ping(wd_data->wdd);
+	if (!ret)
+		set_bit(_WDOG_SUSPENDED, &wd_data->status);
 	mutex_unlock(&wd_data->lock);
 
 	if (ret)
@@ -1346,6 +1352,7 @@ int watchdog_dev_resume(struct watchdog_device *wdd)
 	 * ping worker if needed.
 	 */
 	mutex_lock(&wd_data->lock);
+	clear_bit(_WDOG_SUSPENDED, &wd_data->status);
 	if (watchdog_worker_should_ping(wd_data))
 		ret = __watchdog_ping(wd_data->wdd);
 	mutex_unlock(&wd_data->lock);
-- 
2.45.2

[PATCH v2 4/8] watchdog: core: Stop pretimeout hrtimer on suspend

[Guenter Roeck]

During system suspend, watchdog_dev_suspend() pings the watchdog (which
restarts the software pretimeout hrtimer) and stops the keepalive ping
worker, but fails to stop the pretimeout hrtimer. Because
CLOCK_MONOTONIC continues ticking until the system fully suspends, a
suspend process taking longer than the configured pretimeout will
trigger a spurious pretimeout event.

Stop the pretimeout hrtimer in watchdog_dev_suspend(), prevent
watchdog_hrtimer_pretimeout_start() from starting it while the watchdog
is suspended, and restart it in watchdog_dev_resume() if the hardware
watchdog is running.

Fixes: 60bcd91aafd2 ("watchdog: introduce watchdog_dev_suspend/resume")
Assisted-by: LLM
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Added Reviewed-by: tag

 drivers/watchdog/watchdog_dev.c                | 3 +++
 drivers/watchdog/watchdog_hrtimer_pretimeout.c | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index e38f1ef8e683..af756ee228b2 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -1334,6 +1334,7 @@ int watchdog_dev_suspend(struct watchdog_device *wdd)
 	 * suspended
 	 */
 	hrtimer_cancel(&wd_data->timer);
+	watchdog_hrtimer_pretimeout_stop(wdd);
 	kthread_cancel_work_sync(&wd_data->work);
 
 	return 0;
@@ -1355,6 +1356,8 @@ int watchdog_dev_resume(struct watchdog_device *wdd)
 	clear_bit(_WDOG_SUSPENDED, &wd_data->status);
 	if (watchdog_worker_should_ping(wd_data))
 		ret = __watchdog_ping(wd_data->wdd);
+	if (watchdog_hw_running(wdd) && !ret)
+		watchdog_hrtimer_pretimeout_start(wdd);
 	mutex_unlock(&wd_data->lock);
 
 	return ret;
diff --git a/drivers/watchdog/watchdog_hrtimer_pretimeout.c b/drivers/watchdog/watchdog_hrtimer_pretimeout.c
index 49a05ea60c97..5c88e6467994 100644
--- a/drivers/watchdog/watchdog_hrtimer_pretimeout.c
+++ b/drivers/watchdog/watchdog_hrtimer_pretimeout.c
@@ -31,7 +31,8 @@ void watchdog_hrtimer_pretimeout_start(struct watchdog_device *wdd)
 {
 	if (!(wdd->info->options & WDIOF_PRETIMEOUT) &&
 	    wdd->pretimeout &&
-	    !watchdog_pretimeout_invalid(wdd, wdd->pretimeout))
+	    !watchdog_pretimeout_invalid(wdd, wdd->pretimeout) &&
+	    !test_bit(_WDOG_SUSPENDED, &wdd->wd_data->status))
 		hrtimer_start(&wdd->wd_data->pretimeout_timer,
 			      ktime_set(wdd->timeout - wdd->pretimeout, 0),
 			      HRTIMER_MODE_REL);
-- 
2.45.2

[PATCH v2 5/8] watchdog: core: Restore WDOG_HW_RUNNING if stopping watchdog fails

[Guenter Roeck]

In watchdog_stop(), WDOG_HW_RUNNING is cleared before calling
wdd->ops->stop(). If wdd->ops->stop() returns an error, WDOG_ACTIVE
remains set, but WDOG_HW_RUNNING is left cleared. As a result,
subsequent calls to watchdog_ping() become no-ops, and
watchdog_release() sees watchdog_hw_running() as false and drops the
module and device references while the ping worker (guided by
WDOG_ACTIVE) continues to run.

Restore WDOG_HW_RUNNING if wdd->ops->stop() fails.

Fixes: 3c10bbde10fe ("watchdog: core: Clear WDOG_HW_RUNNING before calling the stop function")
Assisted-by: LLM
Cc: Tzung-Bi Shih <tzungbi@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Moved setting WDOG_HW_RUNNING to error handling after stop() call

 drivers/watchdog/watchdog_dev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index af756ee228b2..47b0735ed6ac 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -312,6 +312,7 @@ static int watchdog_stop(struct watchdog_device *wdd)
 		clear_bit(WDOG_HW_RUNNING, &wdd->status);
 		err = wdd->ops->stop(wdd);
 		if (err < 0) {
+			set_bit(WDOG_HW_RUNNING, &wdd->status);
 			pr_err("watchdog%d: Failed to stop watchdog: %pe\n",
 			       wdd->id, ERR_PTR(err));
 		}
-- 
2.45.2

Re: [PATCH v2 5/8] watchdog: core: Restore WDOG_HW_RUNNING if stopping watchdog fails

[Tzung-Bi Shih]

On Tue, Oct 27, 2026 at 09:02:59AM -0700, Guenter Roeck wrote:
> In watchdog_stop(), WDOG_HW_RUNNING is cleared before calling
> wdd->ops->stop(). If wdd->ops->stop() returns an error, WDOG_ACTIVE
> remains set, but WDOG_HW_RUNNING is left cleared. As a result,
> subsequent calls to watchdog_ping() become no-ops, and
> watchdog_release() sees watchdog_hw_running() as false and drops the
> module and device references while the ping worker (guided by
> WDOG_ACTIVE) continues to run.
> 
> Restore WDOG_HW_RUNNING if wdd->ops->stop() fails.
> 
> Fixes: 3c10bbde10fe ("watchdog: core: Clear WDOG_HW_RUNNING before calling the stop function")
> Assisted-by: LLM
> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH v2 5/8] watchdog: core: Restore WDOG_HW_RUNNING if stopping watchdog fails

[Guenter Roeck]

On 10/27/26 20:14, Tzung-Bi Shih wrote:
> On Tue, Oct 27, 2026 at 09:02:59AM -0700, Guenter Roeck wrote:
>> In watchdog_stop(), WDOG_HW_RUNNING is cleared before calling
>> wdd->ops->stop(). If wdd->ops->stop() returns an error, WDOG_ACTIVE
>> remains set, but WDOG_HW_RUNNING is left cleared. As a result,
>> subsequent calls to watchdog_ping() become no-ops, and
>> watchdog_release() sees watchdog_hw_running() as false and drops the
>> module and device references while the ping worker (guided by
>> WDOG_ACTIVE) continues to run.
>>
>> Restore WDOG_HW_RUNNING if wdd->ops->stop() fails.
>>
>> Fixes: 3c10bbde10fe ("watchdog: core: Clear WDOG_HW_RUNNING before calling the stop function")
>> Assisted-by: LLM
>> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> 
> Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Thanks a lot for the reviews!

Guenter