Re: [PATCH v2 8/9] watchdog: msc313e: Propagate error code in resume()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver manually clears core-managed watchdog flags (WDOG_HW_RUNNING and WDOG_ACTIVE) upon resume failure, which violates subsystem boundaries and causes userspace watchdog pings to silently succeed while the hardware watchdog is actually stopped.
--

commit 671369d67bb1efa18f6207e680185bff41e40d0e
Author: Tzung-Bi Shih <tzungbi@kernel.org>

watchdog: msc313e: Propagate error code in resume()

If msc313e_wdt_start() fails during system resume, the error is
currently ignored. Consequently, the watchdog isn't running without the
user's knowledge. This commit propagates the error code, prints a
message, and explicitly clears both the WDOG_HW_RUNNING and WDOG_ACTIVE
flags if start fails.

> diff --git a/drivers/watchdog/msc313e_wdt.c b/drivers/watchdog/msc313e_wdt.c
> index eea26c6c95d84..eff90ddeb3634 100644
> --- a/drivers/watchdog/msc313e_wdt.c
> +++ b/drivers/watchdog/msc313e_wdt.c
> @@ -192,11 +192,19 @@ static int __maybe_unused msc313e_wdt_suspend(struct device *dev)
>  static int __maybe_unused msc313e_wdt_resume(struct device *dev)
>  {
>  	struct msc313e_wdt_priv *priv = dev_get_drvdata(dev);
> +	int ret = 0;
>  
> -	if (watchdog_active(&priv->wdev) || watchdog_hw_running(&priv->wdev))
> -		msc313e_wdt_start(&priv->wdev);
> +	if (watchdog_active(&priv->wdev) || watchdog_hw_running(&priv->wdev)) {
> +		ret = msc313e_wdt_start(&priv->wdev);
> +		if (ret) {
> +			dev_err(dev, "Failed to restart watchdog (err=%d)\n", ret);
>  
> -	return 0;
> +			clear_bit(WDOG_HW_RUNNING, &priv->wdev.status);
> +			clear_bit(WDOG_ACTIVE, &priv->wdev.status);

[Severity: High]
Does manually clearing these core-managed flags cause userspace watchdog
pings to silently succeed while the hardware watchdog is actually stopped?

If msc313e_wdt_start() fails during system resume in msc313e_wdt_resume(),
the driver manually clears the WDOG_HW_RUNNING and WDOG_ACTIVE flags.

Later, when the userspace daemon writes to /dev/watchdog, the core
watchdog_ping() function could immediately return 0 (success) because
watchdog_hw_running() evaluates to false. This bypasses the driver's .ping
callback and returns success to userspace, hiding the dead state from
monitoring software.

> +		}
> +	}
> +
> +	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828161348.13212-1-tzungbi@kernel.org?part=8