linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] watchdog: Device must be opened for writing
@ 2021-08-14 19:13 Stanislav German-Evtushenko
  2021-08-14 19:56 ` Guenter Roeck
  0 siblings, 1 reply; 4+ messages in thread
From: Stanislav German-Evtushenko @ 2021-08-14 19:13 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck
  Cc: linux-watchdog, Stanislav German-Evtushenko

If userspace opens the watchdog device self-feeding stops. Sometimes
opening the device happens by accident, e.g. by mistakenly running grep
recursively in a wrong directory which leads to the server being reset.

Watchdog device does not handle read operation therefore the issue can be
prevented by requiring the device to be opened for writing:

- Prevent opening the device without FMODE_WRITE

Signed-off-by: Stanislav German-Evtushenko <ginermail@gmail.com>
---
 drivers/watchdog/watchdog_dev.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 3bab32485273..28b88542a4d0 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -835,6 +835,10 @@ static int watchdog_open(struct inode *inode, struct file *file)
 	bool hw_running;
 	int err;
 
+	/* Must be open for writing */
+	if (!(file->f_mode & FMODE_WRITE))
+		return -EINVAL;
+
 	/* Get the corresponding watchdog device */
 	if (imajor(inode) == MISC_MAJOR)
 		wd_data = old_wd_data;

base-commit: cf813c67d9619fd474c785698cbed543b94209dd
-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] watchdog: Device must be opened for writing
  2021-08-14 19:13 [PATCH] watchdog: Device must be opened for writing Stanislav German-Evtushenko
@ 2021-08-14 19:56 ` Guenter Roeck
       [not found]   ` <CAPeRZrV+HwdmBKFB3tmG_tEHjsR8h3WLxKQYGDLOLgw178oq=A@mail.gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2021-08-14 19:56 UTC (permalink / raw)
  To: Stanislav German-Evtushenko; +Cc: Wim Van Sebroeck, linux-watchdog

On Sun, Aug 15, 2021 at 04:13:45AM +0900, Stanislav German-Evtushenko wrote:
> If userspace opens the watchdog device self-feeding stops. Sometimes
> opening the device happens by accident, e.g. by mistakenly running grep
> recursively in a wrong directory which leads to the server being reset.
> 
> Watchdog device does not handle read operation therefore the issue can be
> prevented by requiring the device to be opened for writing:
> 
> - Prevent opening the device without FMODE_WRITE
> 
> Signed-off-by: Stanislav German-Evtushenko <ginermail@gmail.com>

NACK. That would be a major undocumented ABI change. Opening /dev/watchdog
for reading can be and is used today to test a watchdog.

Guenter

> ---
>  drivers/watchdog/watchdog_dev.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index 3bab32485273..28b88542a4d0 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -835,6 +835,10 @@ static int watchdog_open(struct inode *inode, struct file *file)
>  	bool hw_running;
>  	int err;
>  
> +	/* Must be open for writing */
> +	if (!(file->f_mode & FMODE_WRITE))
> +		return -EINVAL;
> +
>  	/* Get the corresponding watchdog device */
>  	if (imajor(inode) == MISC_MAJOR)
>  		wd_data = old_wd_data;
> 
> base-commit: cf813c67d9619fd474c785698cbed543b94209dd
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] watchdog: Device must be opened for writing
       [not found]   ` <CAPeRZrV+HwdmBKFB3tmG_tEHjsR8h3WLxKQYGDLOLgw178oq=A@mail.gmail.com>
@ 2021-08-15  1:33     ` Guenter Roeck
  2021-08-15  4:32       ` Stanislav German-Evtushenko
  0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2021-08-15  1:33 UTC (permalink / raw)
  To: Stanislav German-Evtushenko; +Cc: Wim Van Sebroeck, linux-watchdog

On 8/14/21 5:19 PM, Stanislav German-Evtushenko wrote:
> On Saturday, August 14, 2021, Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>> wrote:
>  > On Sun, Aug 15, 2021 at 04:13:45AM +0900, Stanislav German-Evtushenko wrote:
>  >> If userspace opens the watchdog device self-feeding stops. Sometimes
>  >> opening the device happens by accident, e.g. by mistakenly running grep
>  >> recursively in a wrong directory which leads to the server being reset.
>  >>
>  >> Watchdog device does not handle read operation therefore the issue can be
>  >> prevented by requiring the device to be opened for writing:
>  >>
>  >> - Prevent opening the device without FMODE_WRITE
>  >>
>  >> Signed-off-by: Stanislav German-Evtushenko <ginermail@gmail.com <mailto:ginermail@gmail.com>>
>  >
>  > NACK. That would be a major undocumented ABI change. Opening /dev/watchdog
>  > for reading can be and is used today to test a watchdog.
>  >
>  > Guenter
>  >
> 
> I see. This is unfortunate.
> 
> In this case I'll try to find the right place in the documentation and make it more explicit unless it is already there and I've overlooked.
> 
No. That isn't the point. This is used and must not be changed.
Again, people (including me( are _using_ this to test watchdogs.
We are not going to disable that because some people are not
careful when executing commands as root.

Guenter

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] watchdog: Device must be opened for writing
  2021-08-15  1:33     ` Guenter Roeck
@ 2021-08-15  4:32       ` Stanislav German-Evtushenko
  0 siblings, 0 replies; 4+ messages in thread
From: Stanislav German-Evtushenko @ 2021-08-15  4:32 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Wim Van Sebroeck, linux-watchdog

On 8/15/21, Guenter Roeck <linux@roeck-us.net> wrote:
> On 8/14/21 5:19 PM, Stanislav German-Evtushenko wrote:
>> On Saturday, August 14, 2021, Guenter Roeck <linux@roeck-us.net
>> <mailto:linux@roeck-us.net>> wrote:
>>  > On Sun, Aug 15, 2021 at 04:13:45AM +0900, Stanislav German-Evtushenko
>> wrote:
>>  >> If userspace opens the watchdog device self-feeding stops. Sometimes
>>  >> opening the device happens by accident, e.g. by mistakenly running
>> grep
>>  >> recursively in a wrong directory which leads to the server being
>> reset.
>>  >>
>>  >> Watchdog device does not handle read operation therefore the issue can
>> be
>>  >> prevented by requiring the device to be opened for writing:
>>  >>
>>  >> - Prevent opening the device without FMODE_WRITE
>>  >>
>>  >> Signed-off-by: Stanislav German-Evtushenko <ginermail@gmail.com
>> <mailto:ginermail@gmail.com>>
>>  >
>>  > NACK. That would be a major undocumented ABI change. Opening
>> /dev/watchdog
>>  > for reading can be and is used today to test a watchdog.
>>  >
>>  > Guenter
>>  >
>>
>> I see. This is unfortunate.
>>
>> In this case I'll try to find the right place in the documentation and
>> make it more explicit unless it is already there and I've overlooked.
>>
> No. That isn't the point. This is used and must not be changed.
> Again, people (including me( are _using_ this to test watchdogs.
> We are not going to disable that because some people are not
> careful when executing commands as root.
>
> Guenter
>

I mean I would describe the existing behavior as this is not obvious.

As for testing, opening for writing is not harder than for reading but
I understand that it would be a breaking change now.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-15  4:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-14 19:13 [PATCH] watchdog: Device must be opened for writing Stanislav German-Evtushenko
2021-08-14 19:56 ` Guenter Roeck
     [not found]   ` <CAPeRZrV+HwdmBKFB3tmG_tEHjsR8h3WLxKQYGDLOLgw178oq=A@mail.gmail.com>
2021-08-15  1:33     ` Guenter Roeck
2021-08-15  4:32       ` Stanislav German-Evtushenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).