linux-kernel - Re: [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 21 Apr 2021 09:42:28 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Francesco Zanella <francesco.zanella@...ar.com>
Cc:     linux-watchdog@...r.kernel.org, wim@...ux-watchdog.org,
        devicetree@...r.kernel.org, robh+dt@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature

On Wed, Apr 21, 2021 at 06:26:21PM +0200, Francesco Zanella wrote:
> If "start-at-boot" property is present in the device tree, start pinging
> hw watchdog at probe, in order to take advantage of kernel configs:
> - WATCHDOG_HANDLE_BOOT_ENABLED: Avoid possible reboot if hw watchdog was
>   been enabled before the kernel (by uboot for example) and userspace
>   doesn't take control of /dev/watchdog in time;
> - WATCHDOG_OPEN_TIMEOUT: Reboot if userspace doesn't take control of
>   /dev/watchdog within the timeout.
> 
> Signed-off-by: Francesco Zanella <francesco.zanella@...ar.com>
> ---
>  drivers/watchdog/gpio_wdt.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
> index 0923201ce874..1e6f0322ab7a 100644
> --- a/drivers/watchdog/gpio_wdt.c
> +++ b/drivers/watchdog/gpio_wdt.c
> @@ -31,6 +31,7 @@ struct gpio_wdt_priv {
>  	struct gpio_desc	*gpiod;
>  	bool			state;
>  	bool			always_running;
> +	bool			start_at_boot;
>  	unsigned int		hw_algo;
>  	struct watchdog_device	wdd;
>  };
> @@ -147,6 +148,9 @@ static int gpio_wdt_probe(struct platform_device *pdev)
>  	priv->always_running = of_property_read_bool(np,
>  						     "always-running");
>  
> +	priv->start_at_boot = of_property_read_bool(np,
> +						    "start-at-boot");
> +
>  	watchdog_set_drvdata(&priv->wdd, priv);
>  
>  	priv->wdd.info		= &gpio_wdt_ident;
> @@ -161,7 +165,7 @@ static int gpio_wdt_probe(struct platform_device *pdev)
>  
>  	watchdog_stop_on_reboot(&priv->wdd);
>  
> -	if (priv->always_running)
> +	if (priv->always_running || priv->start_at_boot)
>  		gpio_wdt_start(&priv->wdd);

So the only real difference to always_running is that always_running
doesn't stop the watchdog on close but keeps it running.

Does that really warrant another property ? Why not just use
always-running ?

The special use case of being able to stop the watchdog doesn't seem
to be worth the trouble. Please explain your use case.

Guenter