fs/read_all: Filter /dev/watchdog*

PATCH New yang xu 3 participants 7 messages Mar 14, 2018 – Mar 20, 2018

Share & tools

yang xu

author

2018-03-14 11:07 UTC #0 lore raw

PATCH 1/1 fs/read_all: Filter /dev/watchdog*

On some distros with Magic Close feature or built-in CONFIG_WATCHDOG_NOWAYOUT, just closing /dev/watchdog* enabled by open leads to system reboot as expected.

If Magic Close feature is supported, just writing a specific magic character 'V' into /dev/watchdog* before closing it can disable the watchdog.

If CONFIG_WATCHDOG_NOWAYOUT is built-in, there is no way to disable the watchdog.

Magic Close feature is introduced by:
commit 017cf080("watchDog Timer Driver Core - Add Magic Close feature")

Please see the following url for detailed watchdog info:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/watchdog/watchdog-api.txt

Signed-off-by yang xu <[email protected]>

Signed-off-by Xiao Yang <[email protected]>

---
 testcases/kernel/fs/read_all/read_all.c | 3 +++
 1 file changed, 3 insertions(+)

testcases/kernel/fs/read_all/read_all.c +3 -0

index 81806e7..a841b88 100644
@@ -393,6 +393,9 @@ static void visit_dir(const char *path)
 		snprintf(dent_path, MAX_PATH,
 			 "%s/%s", path, dent->d_name);
 
+		if (!strncmp(dent_path, "/dev/watchdog", 13))
+			continue;
+
 		if (act == DA_UNKNOWN) {
 			if (lstat(dent_path, &dent_st))
 				tst_res(TINFO | TERRNO, "lstat(%s)", path);

Signature · 1 line

1.8.3.1

Richard Palethorpe

2018-03-15 12:25 UTC #1 lore raw

PATCH 1/1 fs/read_all: Filter /dev/watchdog*

Hello,

Quoted 25 lines · yang xu

On some distros with Magic Close feature or built-in CONFIG_WATCHDOG_NOWAYOUT, just closing /dev/watchdog* enabled by open leads to system reboot as expected.
If Magic Close feature is supported, just writing a specific magic character 'V' into /dev/watchdog* before closing it can disable the watchdog.
If CONFIG_WATCHDOG_NOWAYOUT is built-in, there is no way to disable the watchdog.
Magic Close feature is introduced by:
commit 017cf080("watchDog Timer Driver Core - Add Magic Close feature")
Please see the following url for detailed watchdog info:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/watchdog/watchdog-api.txt
Signed-off-by yang xu <[email protected]>
Signed-off-by Xiao Yang <[email protected]>
---
 testcases/kernel/fs/read_all/read_all.c | 3 +++
 1 file changed, 3 insertions(+)
testcases/kernel/fs/read_all/read_all.c +0 -0
index 81806e7..a841b88 100644

testcases/kernel/fs/read_all/read_all.c static void visit_dir(const char *path) · line 398

Quoted 6 lines

testcases/kernel/fs/read_all/read_all.c +3 -0

@@ -393,6 +393,9 @@ static void visit_dir(const char *path)
 		snprintf(dent_path, MAX_PATH,
 			 "%s/%s", path, dent->d_name);
 
+		if (!strncmp(dent_path, "/dev/watchdog", 13))
+			continue;
+

I don't think this should be hardcoded because it is OK to read this file on some systems. Unfortunately there does not seem to be a reliable way to find out if it is OK (/sys/class/watchdog/watchdogn/nowayout is not even present on my system)

However perhaps we could set the default for the exclude parameter (-e) to /dev/watchdog? Then the user can easily override it, but we won't reboot some people's systems by default. What do you think?

Quoted 5 lines

testcases/kernel/fs/read_all/read_all.c +0 -1

 		if (act == DA_UNKNOWN) {
 			if (lstat(dent_path, &dent_st))
 				tst_res(TINFO | TERRNO, "lstat(%s)", path);
--

1.8.3.1

--
Thank you,
Richard.

xuyang.jy

author

2018-03-16 05:18 UTC #2 lore raw

PATCH 1/1 fs/read_all: Filter /dev/watchdog*

2018/3/15 20:25, Richard Palethorpe write:

Quoted 40 lines

Hello,
Quoted 25 lines · yang xu
On some distros with Magic Close feature or built-in CONFIG_WATCHDOG_NOWAYOUT, just closing /dev/watchdog* enabled by open leads to system reboot as expected.
If Magic Close feature is supported, just writing a specific magic character 'V' into /dev/watchdog* before closing it can disable the watchdog.
If CONFIG_WATCHDOG_NOWAYOUT is built-in, there is no way to disable the watchdog.
Magic Close feature is introduced by:
commit 017cf080("watchDog Timer Driver Core - Add Magic Close feature")
Please see the following url for detailed watchdog info:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/watchdog/watchdog-api.txt
Signed-off-by yang xu<[email protected]>
Signed-off-by Xiao Yang<[email protected]>
---
  testcases/kernel/fs/read_all/read_all.c | 3 +++
  1 file changed, 3 insertions(+)
testcases/kernel/fs/read_all/read_all.c +0 -0
index 81806e7..a841b88 100644
testcases/kernel/fs/read_all/read_all.c static void visit_dir(const char *path) · line 398
Quoted 6 lines
testcases/kernel/fs/read_all/read_all.c +3 -0
@@ -393,6 +393,9 @@ static void visit_dir(const char *path)
  		snprintf(dent_path, MAX_PATH,
  			 "%s/%s", path, dent->d_name);
 
+		if (!strncmp(dent_path, "/dev/watchdog", 13))
+			continue;
+
I don't think this should be hardcoded because it is OK to read this file on some systems. Unfortunately there does not seem to be a reliable way to find out if it is OK (/sys/class/watchdog/watchdogn/nowayout is not even present on my system)

Quoted 3 lines

However perhaps we could set the default for the exclude parameter (-e) to /dev/watchdog? Then the user can easily override it, but we won't reboot some people's systems by default. What do you think?

Hi Richard
Agreed. We can run read_all with the -e option to exclude the /dev/watchdog*, as bleow:

runtest/fs +0 -0

index a595edb..42a9bfc 100644
@@ -69,7 +69,7 @@ fs_di fs_di -d $TMPDIR

# Was not sure why it should reside in runtest/crashme and wonÂ´t get tested ever
proc01 proc01 -m 128

-read_all_dev read_all -d /dev -q -r 10 +read_all_dev read_all -d /dev -e '/dev/watchdog?(0)' -q -r 10 read_all_proc read_all -d /proc -q -r 10 read_all_sys read_all -d /sys -q -r 10

How about this modification?

Best Regards,
Yang Xu

Quoted 13 lines

Quoted 5 lines
testcases/kernel/fs/read_all/read_all.c +0 -1
  		if (act == DA_UNKNOWN) {
  			if (lstat(dent_path,&dent_st))
  				tst_res(TINFO | TERRNO, "lstat(%s)", path);
--
1.8.3.1
--
Thank you,
Richard.
.

Richard Palethorpe

2018-03-19 09:14 UTC #3 lore raw

PATCH 1/1 fs/read_all: Filter /dev/watchdog*

Hello,

Quoted 63 lines · xuyang.jy

2018/3/15 20:25, Richard Palethorpe write:
Quoted 40 lines
Hello,
Quoted 25 lines · yang xu
On some distros with Magic Close feature or built-in CONFIG_WATCHDOG_NOWAYOUT, just closing /dev/watchdog* enabled by open leads to system reboot as expected.
If Magic Close feature is supported, just writing a specific magic character 'V' into /dev/watchdog* before closing it can disable the watchdog.
If CONFIG_WATCHDOG_NOWAYOUT is built-in, there is no way to disable the watchdog.
Magic Close feature is introduced by:
commit 017cf080("watchDog Timer Driver Core - Add Magic Close feature")
Please see the following url for detailed watchdog info:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/watchdog/watchdog-api.txt
Signed-off-by yang xu<[email protected]>
Signed-off-by Xiao Yang<[email protected]>
---
  testcases/kernel/fs/read_all/read_all.c | 3 +++
  1 file changed, 3 insertions(+)
testcases/kernel/fs/read_all/read_all.c +0 -0
index 81806e7..a841b88 100644
testcases/kernel/fs/read_all/read_all.c static void visit_dir(const char *path) · line 398
Quoted 6 lines
testcases/kernel/fs/read_all/read_all.c +3 -0
@@ -393,6 +393,9 @@ static void visit_dir(const char *path)
  		snprintf(dent_path, MAX_PATH,
  			 "%s/%s", path, dent->d_name);
 
+		if (!strncmp(dent_path, "/dev/watchdog", 13))
+			continue;
+
I don't think this should be hardcoded because it is OK to read this file on some systems. Unfortunately there does not seem to be a reliable way to find out if it is OK (/sys/class/watchdog/watchdogn/nowayout is not even present on my system)
Quoted 3 lines
However perhaps we could set the default for the exclude parameter (-e) to /dev/watchdog? Then the user can easily override it, but we won't reboot some people's systems by default. What do you think?
Hi Richard
Agreed. We can run read_all with the -e option to exclude the /dev/watchdog*, as bleow:
runtest/fs +0 -0
index a595edb..42a9bfc 100644
@@ -69,7 +69,7 @@ fs_di fs_di -d $TMPDIR
 # Was not sure why it should reside in runtest/crashme and wonÂ´t get tested
ever
proc01 proc01 -m 128
runtest/fs +1 -1
-read_all_dev read_all -d /dev -q -r 10
+read_all_dev read_all -d /dev -e '/dev/watchdog?(0)' -q -r 10
read_all_proc read_all -d /proc -q -r 10
read_all_sys read_all -d /sys -q -r 10

Quoted 1 line

How about this modification?

I think that is OK, but Cyril would prefer to make it the default value of the exclude variable so that it does not cause a restart when the user runs read_all manually.

I think I still prefer putting it in the runtest file because the exception is only relevant to /dev and I don't want to have exceptions hardcoded.

Quoted 17 lines

Best Regards,
Yang Xu
Quoted 13 lines
Quoted 5 lines
testcases/kernel/fs/read_all/read_all.c +0 -1
  		if (act == DA_UNKNOWN) {
  			if (lstat(dent_path,&dent_st))
  				tst_res(TINFO | TERRNO, "lstat(%s)", path);
--
1.8.3.1
--
Thank you,
Richard.
.

Signature · 2 lines

Thank you,
Richard.

yang xu

author

2018-03-19 10:38 UTC #4 lore raw

PATCH v2 1/1 runtest/fs: filter /dev/watchdog* for read_all_dev by default

On some distros with Magic Close feature or built-in CONFIG_WATCHDOG_NOWAYOUT, just closing /dev/watchdog* enabled by open leads to system reboot as expected.

If Magic Close feature is supported, just writing a specific magic character 'V' into /dev/watchdog* before closing it can disable the watchdog.

If CONFIG_WATCHDOG_NOWAYOUT is built-in, there is no way to disable the watchdog.

Magic Close feature is introduced by:
commit 017cf080("watchDog Timer Driver Core - Add Magic Close feature")

Please see the following url for detailed watchdog info:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/watchdog/watchdog-api.txt

Signed-off-by yang xu <[email protected]>

Signed-off-by Xiao Yang <[email protected]>

---
 runtest/fs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

runtest/fs +1 -1

index a595edb..42a9bfc 100644
@@ -69,7 +69,7 @@ fs_di fs_di -d $TMPDIR
 # Was not sure why it should reside in runtest/crashme and wonÂ´t get tested ever
 proc01 proc01 -m 128
 
-read_all_dev read_all -d /dev -q -r 10
+read_all_dev read_all -d /dev -e '/dev/watchdog?(0)' -q -r 10
 read_all_proc read_all -d /proc -q -r 10
 read_all_sys read_all -d /sys -q -r 10

Signature · 1 line

1.8.3.1

Richard Palethorpe

2018-03-19 14:29 UTC #5 lore raw

PATCH v2 1/1 runtest/fs: filter /dev/watchdog* for read_all_dev by default

Hello,

Quoted 25 lines · yang xu

On some distros with Magic Close feature or built-in CONFIG_WATCHDOG_NOWAYOUT, just closing /dev/watchdog* enabled by open leads to system reboot as expected.
If Magic Close feature is supported, just writing a specific magic character 'V' into /dev/watchdog* before closing it can disable the watchdog.
If CONFIG_WATCHDOG_NOWAYOUT is built-in, there is no way to disable the watchdog.
Magic Close feature is introduced by:
commit 017cf080("watchDog Timer Driver Core - Add Magic Close feature")
Please see the following url for detailed watchdog info:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/watchdog/watchdog-api.txt
Signed-off-by yang xu <[email protected]>
Signed-off-by Xiao Yang <[email protected]>
---
 runtest/fs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
runtest/fs +0 -0
index a595edb..42a9bfc 100644

runtest/fs fs_di fs_di -d $TMPDIR · line 74

Quoted 7 lines

runtest/fs +1 -1

@@ -69,7 +69,7 @@ fs_di fs_di -d $TMPDIR
 # Was not sure why it should reside in runtest/crashme and wonÂ´t get tested ever
 proc01 proc01 -m 128
 
-read_all_dev read_all -d /dev -q -r 10
+read_all_dev read_all -d /dev -e '/dev/watchdog?(0)' -q -r 10
 read_all_proc read_all -d /proc -q -r 10
 read_all_sys read_all -d /sys -q -r 10

LGTM, thanks!

Signature · 2 lines

Thank you,
Richard.

Cyril Hrubis

2018-03-20 11:51 UTC #6 lore raw

PATCH v2 1/1 runtest/fs: filter /dev/watchdog* for read_all_dev by default

In reply to yang xu

Hi!
Applied, thanks.

Signature · 1 line

[email protected]