* [PATCH] selftests: Improves rtctest error handling. @ 2023-04-08 7:40 Atul Kumar Pant 2023-04-08 11:50 ` Alexandre Belloni via Linux-kernel-mentees 0 siblings, 1 reply; 4+ messages in thread From: Atul Kumar Pant @ 2023-04-08 7:40 UTC (permalink / raw) To: a.zummo, alexandre.belloni; +Cc: shuah, Atul Kumar Pant, linux-kernel-mentees When running the rtctest without root privileges the test fails expectedly, but prints the logs that are not useful to point to the issue. Similarly, if we pass wrong rtc device file as an argument, the test output failure logs do not point to the issue that the rtc file is invalid. To handle these issues, this patch adds checks to verify uid with which the test is run and also if the rtc_file is valid. Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com> --- tools/testing/selftests/rtc/rtctest.c | 34 +++++++++++++++++++-------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c index 2b9d929a24ed..9564346c63eb 100644 --- a/tools/testing/selftests/rtc/rtctest.c +++ b/tools/testing/selftests/rtc/rtctest.c @@ -388,16 +388,30 @@ __constructor_order_last(void) int main(int argc, char **argv) { - switch (argc) { - case 2: - rtc_file = argv[1]; - /* FALLTHROUGH */ - case 1: - break; - default: - fprintf(stderr, "usage: %s [rtcdev]\n", argv[0]); - return 1; + int ret = -1; + + // Verify if the test is run as root + if (getuid() != 0) { + ksft_exit_skip("[ERROR]: Please run the test as root - Exiting.\n"); + } + + switch (argc) { + case 2: + rtc_file = argv[1]; + /* FALLTHROUGH */ + case 1: + break; + default: + fprintf(stderr, "usage: %s [rtcdev]\n", argv[0]); + return 1; } - return test_harness_run(argc, argv); + // Run the test if rtc_file is valid + if (access(rtc_file, F_OK) == 0) { + ret = test_harness_run(argc, argv); + } else { + ksft_exit_skip("[ERROR]: %s : File does not exists - Exiting\n", rtc_file); + } + + return ret; } -- 2.25.1 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] selftests: Improves rtctest error handling. 2023-04-08 7:40 [PATCH] selftests: Improves rtctest error handling Atul Kumar Pant @ 2023-04-08 11:50 ` Alexandre Belloni via Linux-kernel-mentees 2023-04-11 4:51 ` Atul Kumar Pant 0 siblings, 1 reply; 4+ messages in thread From: Alexandre Belloni via Linux-kernel-mentees @ 2023-04-08 11:50 UTC (permalink / raw) To: Atul Kumar Pant; +Cc: a.zummo, shuah, linux-kernel-mentees On 08/04/2023 13:10:59+0530, Atul Kumar Pant wrote: > When running the rtctest without root privileges the test fails > expectedly, but prints the logs that are not useful to point to the issue. > Similarly, if we pass wrong rtc device file as an argument, the test > output failure logs do not point to the issue that the rtc file is > invalid. > To handle these issues, this patch adds checks to verify uid with which > the test is run and also if the rtc_file is valid. > > Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com> > --- > tools/testing/selftests/rtc/rtctest.c | 34 +++++++++++++++++++-------- > 1 file changed, 24 insertions(+), 10 deletions(-) > Please run your patch through checkpatch.pl, the indentation and comment style are not correct. > diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c > index 2b9d929a24ed..9564346c63eb 100644 > --- a/tools/testing/selftests/rtc/rtctest.c > +++ b/tools/testing/selftests/rtc/rtctest.c > @@ -388,16 +388,30 @@ __constructor_order_last(void) > > int main(int argc, char **argv) > { > - switch (argc) { > - case 2: > - rtc_file = argv[1]; > - /* FALLTHROUGH */ > - case 1: > - break; > - default: > - fprintf(stderr, "usage: %s [rtcdev]\n", argv[0]); > - return 1; > + int ret = -1; > + > + // Verify if the test is run as root > + if (getuid() != 0) { > + ksft_exit_skip("[ERROR]: Please run the test as root - Exiting.\n"); If this is an error, should we really return skip? This may end up being silently ignored whereas the expectation is to test the rtc. Also, you may use the rtc as non root so I guess the access() test should be enough. > + } > + > + switch (argc) { > + case 2: > + rtc_file = argv[1]; > + /* FALLTHROUGH */ > + case 1: > + break; > + default: > + fprintf(stderr, "usage: %s [rtcdev]\n", argv[0]); > + return 1; > } > > - return test_harness_run(argc, argv); > + // Run the test if rtc_file is valid > + if (access(rtc_file, F_OK) == 0) { > + ret = test_harness_run(argc, argv); > + } else { > + ksft_exit_skip("[ERROR]: %s : File does not exists - Exiting\n", rtc_file); > + } > + > + return ret; > } > -- > 2.25.1 > -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] selftests: Improves rtctest error handling. 2023-04-08 11:50 ` Alexandre Belloni via Linux-kernel-mentees @ 2023-04-11 4:51 ` Atul Kumar Pant 2023-04-18 8:01 ` Alexandre Belloni via Linux-kernel-mentees 0 siblings, 1 reply; 4+ messages in thread From: Atul Kumar Pant @ 2023-04-11 4:51 UTC (permalink / raw) To: Alexandre Belloni; +Cc: a.zummo, shuah, linux-kernel-mentees On Sat, Apr 08, 2023 at 01:50:15PM +0200, Alexandre Belloni wrote: > On 08/04/2023 13:10:59+0530, Atul Kumar Pant wrote: > > When running the rtctest without root privileges the test fails > > expectedly, but prints the logs that are not useful to point to the issue. > > Similarly, if we pass wrong rtc device file as an argument, the test > > output failure logs do not point to the issue that the rtc file is > > invalid. > > To handle these issues, this patch adds checks to verify uid with which > > the test is run and also if the rtc_file is valid. > > > > Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com> > > --- > > tools/testing/selftests/rtc/rtctest.c | 34 +++++++++++++++++++-------- > > 1 file changed, 24 insertions(+), 10 deletions(-) > > > > Please run your patch through checkpatch.pl, the indentation and comment > style are not correct. Sure I'll fix this is next patch. > > > diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c > > index 2b9d929a24ed..9564346c63eb 100644 > > --- a/tools/testing/selftests/rtc/rtctest.c > > +++ b/tools/testing/selftests/rtc/rtctest.c > > @@ -388,16 +388,30 @@ __constructor_order_last(void) > > > > int main(int argc, char **argv) > > { > > - switch (argc) { > > - case 2: > > - rtc_file = argv[1]; > > - /* FALLTHROUGH */ > > - case 1: > > - break; > > - default: > > - fprintf(stderr, "usage: %s [rtcdev]\n", argv[0]); > > - return 1; > > + int ret = -1; > > + > > + // Verify if the test is run as root > > + if (getuid() != 0) { > > + ksft_exit_skip("[ERROR]: Please run the test as root - Exiting.\n"); > > If this is an error, should we really return skip? This may end up being > silently ignored whereas the expectation is to test the rtc. > Yes, we should not skip the test. In that case we can use 'ksft_exit_fail_msg'. I'll fix this in next patch. > Also, you may use the rtc as non root so I guess the access() test > should be enough. I added this check for the use case when we use the rtc file from /dev/ which will require root permissions. Should I keep this check ? > > > + } > > + > > + switch (argc) { > > + case 2: > > + rtc_file = argv[1]; > > + /* FALLTHROUGH */ > > + case 1: > > + break; > > + default: > > + fprintf(stderr, "usage: %s [rtcdev]\n", argv[0]); > > + return 1; > > } > > > > - return test_harness_run(argc, argv); > > + // Run the test if rtc_file is valid > > + if (access(rtc_file, F_OK) == 0) { > > + ret = test_harness_run(argc, argv); > > + } else { > > + ksft_exit_skip("[ERROR]: %s : File does not exists - Exiting\n", rtc_file); > > > > + } > > + > > + return ret; > > } > > -- > > 2.25.1 > > > > -- > Alexandre Belloni, co-owner and COO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] selftests: Improves rtctest error handling. 2023-04-11 4:51 ` Atul Kumar Pant @ 2023-04-18 8:01 ` Alexandre Belloni via Linux-kernel-mentees 0 siblings, 0 replies; 4+ messages in thread From: Alexandre Belloni via Linux-kernel-mentees @ 2023-04-18 8:01 UTC (permalink / raw) To: Atul Kumar Pant; +Cc: a.zummo, shuah, linux-kernel-mentees On 11/04/2023 10:21:15+0530, Atul Kumar Pant wrote: > On Sat, Apr 08, 2023 at 01:50:15PM +0200, Alexandre Belloni wrote: > > On 08/04/2023 13:10:59+0530, Atul Kumar Pant wrote: > > > When running the rtctest without root privileges the test fails > > > expectedly, but prints the logs that are not useful to point to the issue. > > > Similarly, if we pass wrong rtc device file as an argument, the test > > > output failure logs do not point to the issue that the rtc file is > > > invalid. > > > To handle these issues, this patch adds checks to verify uid with which > > > the test is run and also if the rtc_file is valid. > > > > > > Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com> > > > --- > > > tools/testing/selftests/rtc/rtctest.c | 34 +++++++++++++++++++-------- > > > 1 file changed, 24 insertions(+), 10 deletions(-) > > > > > > > Please run your patch through checkpatch.pl, the indentation and comment > > style are not correct. > > Sure I'll fix this is next patch. > > > > > > diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c > > > index 2b9d929a24ed..9564346c63eb 100644 > > > --- a/tools/testing/selftests/rtc/rtctest.c > > > +++ b/tools/testing/selftests/rtc/rtctest.c > > > @@ -388,16 +388,30 @@ __constructor_order_last(void) > > > > > > int main(int argc, char **argv) > > > { > > > - switch (argc) { > > > - case 2: > > > - rtc_file = argv[1]; > > > - /* FALLTHROUGH */ > > > - case 1: > > > - break; > > > - default: > > > - fprintf(stderr, "usage: %s [rtcdev]\n", argv[0]); > > > - return 1; > > > + int ret = -1; > > > + > > > + // Verify if the test is run as root > > > + if (getuid() != 0) { > > > + ksft_exit_skip("[ERROR]: Please run the test as root - Exiting.\n"); > > > > If this is an error, should we really return skip? This may end up being > > silently ignored whereas the expectation is to test the rtc. > > > > Yes, we should not skip the test. In that case we can use 'ksft_exit_fail_msg'. > I'll fix this in next patch. > > > Also, you may use the rtc as non root so I guess the access() test > > should be enough. > > I added this check for the use case when we use the rtc file from /dev/ which will require root > permissions. Should I keep this check ? You don't need to be root, you need CAP_SYS_TIME and CAP_SYS_RESOURCE, I wouldn't check for uid 0. > > > > > > + } > > > + > > > + switch (argc) { > > > + case 2: > > > + rtc_file = argv[1]; > > > + /* FALLTHROUGH */ > > > + case 1: > > > + break; > > > + default: > > > + fprintf(stderr, "usage: %s [rtcdev]\n", argv[0]); > > > + return 1; > > > } > > > > > > - return test_harness_run(argc, argv); > > > + // Run the test if rtc_file is valid > > > + if (access(rtc_file, F_OK) == 0) { > > > + ret = test_harness_run(argc, argv); > > > + } else { > > > + ksft_exit_skip("[ERROR]: %s : File does not exists - Exiting\n", rtc_file); > > > > > > > + } > > > + > > > + return ret; > > > } > > > -- > > > 2.25.1 > > > > > > > -- > > Alexandre Belloni, co-owner and COO, Bootlin > > Embedded Linux and Kernel engineering > > https://bootlin.com -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-04-18 8:01 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-04-08 7:40 [PATCH] selftests: Improves rtctest error handling Atul Kumar Pant 2023-04-08 11:50 ` Alexandre Belloni via Linux-kernel-mentees 2023-04-11 4:51 ` Atul Kumar Pant 2023-04-18 8:01 ` Alexandre Belloni via Linux-kernel-mentees
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox