* [PATCH v7 0/1] selftests/landlock: fix test when overlayfs is @ 2022-12-29 21:02 jeffxu 2022-12-29 21:02 ` [PATCH v7 1/1] selftests/landlock: skip overlayfs test when kernel not support it jeffxu 0 siblings, 1 reply; 6+ messages in thread From: jeffxu @ 2022-12-29 21:02 UTC (permalink / raw) To: mic; +Cc: jorgelo, keescook, linux-security-module, groeck, gnoack, Jeff Xu From: Jeff Xu <jeffxu@google.com> Overlayfs can be disabled in kernel config, causing related tests to fail. Adding a check for overlayfs’s supportability at runtime, so we can call SKIP() when needed. v7: Fix bug in supports_overlayfs(). Manual test with kernel with and without overlayfs. v6: https://lore.kernel.org/all/20221229201215.3006512-1-jeffxu@google.com/ In v4, the SKIP() was applied at FIXTURE_SETUP() after mount() fail, however, FIXTURE_TEARDOWN() will fail. It might be complicated for test infra or testcase itself to have cleanup code handing the success/failure of steps in SETUP(). This patch changes the approach, it calls supports_overlay() and SKIP() at the beginning of FIXTURE_SETUP(), FIX_TEARDOWN(), TEST_F_FORK(). Because no modification of system is done by the test, cleanup is not needed. v4: https://lore.kernel.org/all/20220823010216.2653012-1-jeffxu@google.com/ *** BLURB HERE *** Jeff Xu (1): selftests/landlock: skip overlayfs test when kernel not support it tools/testing/selftests/landlock/fs_test.c | 51 ++++++++++++++++++++++ 1 file changed, 51 insertions(+) base-commit: 963a70bee5880640d0fd83ed29dc1e7ec0d2bd4a -- 2.39.0.314.g84b9a713c41-goog ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v7 1/1] selftests/landlock: skip overlayfs test when kernel not support it 2022-12-29 21:02 [PATCH v7 0/1] selftests/landlock: fix test when overlayfs is jeffxu @ 2022-12-29 21:02 ` jeffxu 2022-12-29 21:41 ` Guenter Roeck 0 siblings, 1 reply; 6+ messages in thread From: jeffxu @ 2022-12-29 21:02 UTC (permalink / raw) To: mic; +Cc: jorgelo, keescook, linux-security-module, groeck, gnoack, Jeff Xu From: Jeff Xu <jeffxu@google.com> Overlayfs can be disabled in kernel config, causing related tests to fail. Add check for overlayfs’s supportability at runtime, so we can call SKIP() when needed. Signed-off-by: Jeff Xu <jeffxu@google.com> --- tools/testing/selftests/landlock/fs_test.c | 51 ++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 21a2ce8fa739..34095fe2419b 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -11,6 +11,7 @@ #include <fcntl.h> #include <linux/landlock.h> #include <sched.h> +#include <stdio.h> #include <string.h> #include <sys/capability.h> #include <sys/mount.h> @@ -62,6 +63,7 @@ static const char dir_s3d1[] = TMP_DIR "/s3d1"; static const char dir_s3d2[] = TMP_DIR "/s3d1/s3d2"; static const char dir_s3d3[] = TMP_DIR "/s3d1/s3d2/s3d3"; +static const char proc_filesystems[] = "/proc/filesystems"; /* * layout1 hierarchy: * @@ -169,6 +171,43 @@ static int remove_path(const char *const path) return err; } +static bool fgrep(FILE *inf, const char *str) +{ + char line[32]; + int slen = strlen(str); + + while (!feof(inf)) { + if (!fgets(line, sizeof(line), inf)) + break; + if (strncmp(line, str, slen)) + continue; + + return true; + } + + return false; +} + +static bool supports_overlayfs(void) +{ + bool res; + FILE *inf = fopen(proc_filesystems, "r"); + + /* + * If fopen failed, return supported. + * This help detect missing file (shall not + * happen). + */ + if (!inf) + return true; + + res = fgrep(inf, "nodev\toverlay\n"); + fclose(inf); + + return res; +} + + static void prepare_layout(struct __test_metadata *const _metadata) { disable_caps(_metadata); @@ -3404,6 +3443,9 @@ FIXTURE(layout2_overlay) {}; FIXTURE_SETUP(layout2_overlay) { + if (!supports_overlayfs()) + SKIP(return, "overlayfs is not supported"); + prepare_layout(_metadata); create_directory(_metadata, LOWER_BASE); @@ -3440,6 +3482,9 @@ FIXTURE_SETUP(layout2_overlay) FIXTURE_TEARDOWN(layout2_overlay) { + if (!supports_overlayfs()) + SKIP(return, "overlayfs is not supported"); + EXPECT_EQ(0, remove_path(lower_do1_fl3)); EXPECT_EQ(0, remove_path(lower_dl1_fl2)); EXPECT_EQ(0, remove_path(lower_fl1)); @@ -3471,6 +3516,9 @@ FIXTURE_TEARDOWN(layout2_overlay) TEST_F_FORK(layout2_overlay, no_restriction) { + if (!supports_overlayfs()) + SKIP(return, "overlayfs is not supported"); + ASSERT_EQ(0, test_open(lower_fl1, O_RDONLY)); ASSERT_EQ(0, test_open(lower_dl1, O_RDONLY)); ASSERT_EQ(0, test_open(lower_dl1_fl2, O_RDONLY)); @@ -3634,6 +3682,9 @@ TEST_F_FORK(layout2_overlay, same_content_different_file) size_t i; const char *path_entry; + if (!supports_overlayfs()) + SKIP(return, "overlayfs is not supported"); + /* Sets rules on base directories (i.e. outside overlay scope). */ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer1_base); ASSERT_LE(0, ruleset_fd); -- 2.39.0.314.g84b9a713c41-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v7 1/1] selftests/landlock: skip overlayfs test when kernel not support it 2022-12-29 21:02 ` [PATCH v7 1/1] selftests/landlock: skip overlayfs test when kernel not support it jeffxu @ 2022-12-29 21:41 ` Guenter Roeck 2023-01-09 16:05 ` Mickaël Salaün 0 siblings, 1 reply; 6+ messages in thread From: Guenter Roeck @ 2022-12-29 21:41 UTC (permalink / raw) To: jeffxu Cc: mic, jorgelo, keescook, linux-security-module, groeck, gnoack, Jeff Xu On Thu, Dec 29, 2022 at 1:02 PM <jeffxu@chromium.org> wrote: > > From: Jeff Xu <jeffxu@google.com> > > Overlayfs can be disabled in kernel config, causing related tests to > fail. Add check for overlayfs’s supportability at runtime, > so we can call SKIP() when needed. > > Signed-off-by: Jeff Xu <jeffxu@google.com> Reviewed-by: Guenter Roeck <groeck@chromium.org> > --- > tools/testing/selftests/landlock/fs_test.c | 51 ++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c > index 21a2ce8fa739..34095fe2419b 100644 > --- a/tools/testing/selftests/landlock/fs_test.c > +++ b/tools/testing/selftests/landlock/fs_test.c > @@ -11,6 +11,7 @@ > #include <fcntl.h> > #include <linux/landlock.h> > #include <sched.h> > +#include <stdio.h> > #include <string.h> > #include <sys/capability.h> > #include <sys/mount.h> > @@ -62,6 +63,7 @@ static const char dir_s3d1[] = TMP_DIR "/s3d1"; > static const char dir_s3d2[] = TMP_DIR "/s3d1/s3d2"; > static const char dir_s3d3[] = TMP_DIR "/s3d1/s3d2/s3d3"; > > +static const char proc_filesystems[] = "/proc/filesystems"; > /* > * layout1 hierarchy: > * > @@ -169,6 +171,43 @@ static int remove_path(const char *const path) > return err; > } > > +static bool fgrep(FILE *inf, const char *str) > +{ > + char line[32]; > + int slen = strlen(str); > + > + while (!feof(inf)) { > + if (!fgets(line, sizeof(line), inf)) > + break; > + if (strncmp(line, str, slen)) > + continue; > + > + return true; > + } > + > + return false; > +} > + > +static bool supports_overlayfs(void) > +{ > + bool res; > + FILE *inf = fopen(proc_filesystems, "r"); > + > + /* > + * If fopen failed, return supported. > + * This help detect missing file (shall not > + * happen). > + */ > + if (!inf) > + return true; > + > + res = fgrep(inf, "nodev\toverlay\n"); > + fclose(inf); > + > + return res; > +} > + > + > static void prepare_layout(struct __test_metadata *const _metadata) > { > disable_caps(_metadata); > @@ -3404,6 +3443,9 @@ FIXTURE(layout2_overlay) {}; > > FIXTURE_SETUP(layout2_overlay) > { > + if (!supports_overlayfs()) > + SKIP(return, "overlayfs is not supported"); > + > prepare_layout(_metadata); > > create_directory(_metadata, LOWER_BASE); > @@ -3440,6 +3482,9 @@ FIXTURE_SETUP(layout2_overlay) > > FIXTURE_TEARDOWN(layout2_overlay) > { > + if (!supports_overlayfs()) > + SKIP(return, "overlayfs is not supported"); > + > EXPECT_EQ(0, remove_path(lower_do1_fl3)); > EXPECT_EQ(0, remove_path(lower_dl1_fl2)); > EXPECT_EQ(0, remove_path(lower_fl1)); > @@ -3471,6 +3516,9 @@ FIXTURE_TEARDOWN(layout2_overlay) > > TEST_F_FORK(layout2_overlay, no_restriction) > { > + if (!supports_overlayfs()) > + SKIP(return, "overlayfs is not supported"); > + > ASSERT_EQ(0, test_open(lower_fl1, O_RDONLY)); > ASSERT_EQ(0, test_open(lower_dl1, O_RDONLY)); > ASSERT_EQ(0, test_open(lower_dl1_fl2, O_RDONLY)); > @@ -3634,6 +3682,9 @@ TEST_F_FORK(layout2_overlay, same_content_different_file) > size_t i; > const char *path_entry; > > + if (!supports_overlayfs()) > + SKIP(return, "overlayfs is not supported"); > + > /* Sets rules on base directories (i.e. outside overlay scope). */ > ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer1_base); > ASSERT_LE(0, ruleset_fd); > -- > 2.39.0.314.g84b9a713c41-goog > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 1/1] selftests/landlock: skip overlayfs test when kernel not support it 2022-12-29 21:41 ` Guenter Roeck @ 2023-01-09 16:05 ` Mickaël Salaün 2023-01-09 21:59 ` Jeff Xu 0 siblings, 1 reply; 6+ messages in thread From: Mickaël Salaün @ 2023-01-09 16:05 UTC (permalink / raw) To: Guenter Roeck, jeffxu, Shuah Khan Cc: jorgelo, keescook, linux-security-module, groeck, gnoack, Jeff Xu Please refresh with clang-format-14. You might want to update the subject to: selftests/landlock: Skip overlayfs tests when not supported On 29/12/2022 22:41, Guenter Roeck wrote: > On Thu, Dec 29, 2022 at 1:02 PM <jeffxu@chromium.org> wrote: >> >> From: Jeff Xu <jeffxu@google.com> >> >> Overlayfs can be disabled in kernel config, causing related tests to >> fail. Add check for overlayfs’s supportability at runtime, >> so we can call SKIP() when needed. >> >> Signed-off-by: Jeff Xu <jeffxu@google.com> > > Reviewed-by: Guenter Roeck <groeck@chromium.org> > >> --- >> tools/testing/selftests/landlock/fs_test.c | 51 ++++++++++++++++++++++ >> 1 file changed, 51 insertions(+) >> >> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c >> index 21a2ce8fa739..34095fe2419b 100644 >> --- a/tools/testing/selftests/landlock/fs_test.c >> +++ b/tools/testing/selftests/landlock/fs_test.c >> @@ -11,6 +11,7 @@ >> #include <fcntl.h> >> #include <linux/landlock.h> >> #include <sched.h> >> +#include <stdio.h> >> #include <string.h> >> #include <sys/capability.h> >> #include <sys/mount.h> >> @@ -62,6 +63,7 @@ static const char dir_s3d1[] = TMP_DIR "/s3d1"; >> static const char dir_s3d2[] = TMP_DIR "/s3d1/s3d2"; >> static const char dir_s3d3[] = TMP_DIR "/s3d1/s3d2/s3d3"; >> >> +static const char proc_filesystems[] = "/proc/filesystems"; You can inline this string in the fopen() call for now. >> /* >> * layout1 hierarchy: >> * >> @@ -169,6 +171,43 @@ static int remove_path(const char *const path) >> return err; >> } >> >> +static bool fgrep(FILE *inf, const char *str) >> +{ >> + char line[32]; >> + int slen = strlen(str); >> + >> + while (!feof(inf)) { >> + if (!fgets(line, sizeof(line), inf)) >> + break; >> + if (strncmp(line, str, slen)) >> + continue; >> + >> + return true; >> + } >> + >> + return false; >> +} >> + >> +static bool supports_overlayfs(void) You can move this two functions just before mkdir_parents(). >> +{ >> + bool res; >> + FILE *inf = fopen(proc_filesystems, "r"); >> + >> + /* >> + * If fopen failed, return supported. >> + * This help detect missing file (shall not >> + * happen). You can make this comment fit in two lines, with 80 columns. >> + */ >> + if (!inf) >> + return true; >> + >> + res = fgrep(inf, "nodev\toverlay\n"); >> + fclose(inf); >> + >> + return res; >> +} >> + >> + >> static void prepare_layout(struct __test_metadata *const _metadata) >> { >> disable_caps(_metadata); >> @@ -3404,6 +3443,9 @@ FIXTURE(layout2_overlay) {}; >> >> FIXTURE_SETUP(layout2_overlay) >> { >> + if (!supports_overlayfs()) >> + SKIP(return, "overlayfs is not supported"); >> + >> prepare_layout(_metadata); >> >> create_directory(_metadata, LOWER_BASE); >> @@ -3440,6 +3482,9 @@ FIXTURE_SETUP(layout2_overlay) >> >> FIXTURE_TEARDOWN(layout2_overlay) >> { >> + if (!supports_overlayfs()) >> + SKIP(return, "overlayfs is not supported"); This looks good to me except the multiple supports_overlayfs() calls. Only the FIXTURE_SETUP() should be required. I guess some modifications of kselftest_harness.h are need to support that. I'd like to avoid touching TEST_F_FORK() which should be part of kselftest_harness.h >> + >> EXPECT_EQ(0, remove_path(lower_do1_fl3)); >> EXPECT_EQ(0, remove_path(lower_dl1_fl2)); >> EXPECT_EQ(0, remove_path(lower_fl1)); >> @@ -3471,6 +3516,9 @@ FIXTURE_TEARDOWN(layout2_overlay) >> >> TEST_F_FORK(layout2_overlay, no_restriction) >> { >> + if (!supports_overlayfs()) >> + SKIP(return, "overlayfs is not supported"); >> + >> ASSERT_EQ(0, test_open(lower_fl1, O_RDONLY)); >> ASSERT_EQ(0, test_open(lower_dl1, O_RDONLY)); >> ASSERT_EQ(0, test_open(lower_dl1_fl2, O_RDONLY)); >> @@ -3634,6 +3682,9 @@ TEST_F_FORK(layout2_overlay, same_content_different_file) >> size_t i; >> const char *path_entry; >> >> + if (!supports_overlayfs()) >> + SKIP(return, "overlayfs is not supported"); >> + >> /* Sets rules on base directories (i.e. outside overlay scope). */ >> ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer1_base); >> ASSERT_LE(0, ruleset_fd); >> -- >> 2.39.0.314.g84b9a713c41-goog >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 1/1] selftests/landlock: skip overlayfs test when kernel not support it 2023-01-09 16:05 ` Mickaël Salaün @ 2023-01-09 21:59 ` Jeff Xu 2023-01-10 18:54 ` Mickaël Salaün 0 siblings, 1 reply; 6+ messages in thread From: Jeff Xu @ 2023-01-09 21:59 UTC (permalink / raw) To: Mickaël Salaün Cc: Guenter Roeck, jeffxu, Shuah Khan, jorgelo, keescook, linux-security-module, groeck, gnoack Hi Mickaël Please see inline. On Mon, Jan 9, 2023 at 8:05 AM Mickaël Salaün <mic@digikod.net> wrote: > > Please refresh with clang-format-14. > My installation has clang-format version 15, but changes are quite big if I use it, do you still want me to use it ? > You might want to update the subject to: > selftests/landlock: Skip overlayfs tests when not supported > OK. > > On 29/12/2022 22:41, Guenter Roeck wrote: > > On Thu, Dec 29, 2022 at 1:02 PM <jeffxu@chromium.org> wrote: > >> > >> From: Jeff Xu <jeffxu@google.com> > >> > >> Overlayfs can be disabled in kernel config, causing related tests to > >> fail. Add check for overlayfs’s supportability at runtime, > >> so we can call SKIP() when needed. > >> > >> Signed-off-by: Jeff Xu <jeffxu@google.com> > > > > Reviewed-by: Guenter Roeck <groeck@chromium.org> > > > >> --- > >> tools/testing/selftests/landlock/fs_test.c | 51 ++++++++++++++++++++++ > >> 1 file changed, 51 insertions(+) > >> > >> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c > >> index 21a2ce8fa739..34095fe2419b 100644 > >> --- a/tools/testing/selftests/landlock/fs_test.c > >> +++ b/tools/testing/selftests/landlock/fs_test.c > >> @@ -11,6 +11,7 @@ > >> #include <fcntl.h> > >> #include <linux/landlock.h> > >> #include <sched.h> > >> +#include <stdio.h> > >> #include <string.h> > >> #include <sys/capability.h> > >> #include <sys/mount.h> > >> @@ -62,6 +63,7 @@ static const char dir_s3d1[] = TMP_DIR "/s3d1"; > >> static const char dir_s3d2[] = TMP_DIR "/s3d1/s3d2"; > >> static const char dir_s3d3[] = TMP_DIR "/s3d1/s3d2/s3d3"; > >> > >> +static const char proc_filesystems[] = "/proc/filesystems"; > > You can inline this string in the fopen() call for now. > Done. > > >> /* > >> * layout1 hierarchy: > >> * > >> @@ -169,6 +171,43 @@ static int remove_path(const char *const path) > >> return err; > >> } > >> > >> +static bool fgrep(FILE *inf, const char *str) > >> +{ > >> + char line[32]; > >> + int slen = strlen(str); > >> + > >> + while (!feof(inf)) { > >> + if (!fgets(line, sizeof(line), inf)) > >> + break; > >> + if (strncmp(line, str, slen)) > >> + continue; > >> + > >> + return true; > >> + } > >> + > >> + return false; > >> +} > >> + > >> +static bool supports_overlayfs(void) > > You can move this two functions just before mkdir_parents(). > Done. > > >> +{ > >> + bool res; > >> + FILE *inf = fopen(proc_filesystems, "r"); > >> + > >> + /* > >> + * If fopen failed, return supported. > >> + * This help detect missing file (shall not > >> + * happen). > > You can make this comment fit in two lines, with 80 columns. > Done. > >> + */ > >> + if (!inf) > >> + return true; > >> + > >> + res = fgrep(inf, "nodev\toverlay\n"); > >> + fclose(inf); > >> + > >> + return res; > >> +} > >> + > >> + > >> static void prepare_layout(struct __test_metadata *const _metadata) > >> { > >> disable_caps(_metadata); > >> @@ -3404,6 +3443,9 @@ FIXTURE(layout2_overlay) {}; > >> > >> FIXTURE_SETUP(layout2_overlay) > >> { > >> + if (!supports_overlayfs()) > >> + SKIP(return, "overlayfs is not supported"); > >> + > >> prepare_layout(_metadata); > >> > >> create_directory(_metadata, LOWER_BASE); > >> @@ -3440,6 +3482,9 @@ FIXTURE_SETUP(layout2_overlay) > >> > >> FIXTURE_TEARDOWN(layout2_overlay) > >> { > >> + if (!supports_overlayfs()) > >> + SKIP(return, "overlayfs is not supported"); > > This looks good to me except the multiple supports_overlayfs() calls. > Only the FIXTURE_SETUP() should be required. I guess some modifications > of kselftest_harness.h are need to support that. I'd like to avoid > touching TEST_F_FORK() which should be part of kselftest_harness.h > > In kselftest_harness.h, SKIP() only applies within the function scope ( FIXTURE_SETUP(), TEST_F_FORK(), FIXTURE_TEARDOWN()) If we want to apply the skip logic to all remaining steps of the testcase, I think we should do it with a dedicated environment check hook (FIXTURE_ENV_CHECK), called before FIXTURE_SETUP(), if the environment check fails, all of the remaining test steps will be skipped. In this way, once the env check pass, the remaining test case should also be passing, or if env check fails, there is no need to delete the resource since no setup is called. However, this requires change to the kselftest_harness.h, I do think it needs to be a separate feature and commit (we can adopt fs_test to be the first user) Best regards, Jeff > >> + > >> EXPECT_EQ(0, remove_path(lower_do1_fl3)); > >> EXPECT_EQ(0, remove_path(lower_dl1_fl2)); > >> EXPECT_EQ(0, remove_path(lower_fl1)); > >> @@ -3471,6 +3516,9 @@ FIXTURE_TEARDOWN(layout2_overlay) > >> > >> TEST_F_FORK(layout2_overlay, no_restriction) > >> { > >> + if (!supports_overlayfs()) > >> + SKIP(return, "overlayfs is not supported"); > >> + > >> ASSERT_EQ(0, test_open(lower_fl1, O_RDONLY)); > >> ASSERT_EQ(0, test_open(lower_dl1, O_RDONLY)); > >> ASSERT_EQ(0, test_open(lower_dl1_fl2, O_RDONLY)); > >> @@ -3634,6 +3682,9 @@ TEST_F_FORK(layout2_overlay, same_content_different_file) > >> size_t i; > >> const char *path_entry; > >> > >> + if (!supports_overlayfs()) > >> + SKIP(return, "overlayfs is not supported"); > >> + > >> /* Sets rules on base directories (i.e. outside overlay scope). */ > >> ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer1_base); > >> ASSERT_LE(0, ruleset_fd); > >> -- > >> 2.39.0.314.g84b9a713c41-goog > >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 1/1] selftests/landlock: skip overlayfs test when kernel not support it 2023-01-09 21:59 ` Jeff Xu @ 2023-01-10 18:54 ` Mickaël Salaün 0 siblings, 0 replies; 6+ messages in thread From: Mickaël Salaün @ 2023-01-10 18:54 UTC (permalink / raw) To: Jeff Xu, Shuah Khan Cc: Guenter Roeck, jeffxu, Shuah Khan, jorgelo, keescook, linux-security-module, groeck, gnoack On 09/01/2023 22:59, Jeff Xu wrote: > Hi Mickaël > Please see inline. > > On Mon, Jan 9, 2023 at 8:05 AM Mickaël Salaün <mic@digikod.net> wrote: >> >> Please refresh with clang-format-14. >> > My installation has clang-format version 15, but changes are quite big > if I use it, > do you still want me to use it ? That's fine, this patch is almost good, I'll run clang-format-14 on the final one. > >> You might want to update the subject to: >> selftests/landlock: Skip overlayfs tests when not supported >> > OK. > >> >> On 29/12/2022 22:41, Guenter Roeck wrote: >>> On Thu, Dec 29, 2022 at 1:02 PM <jeffxu@chromium.org> wrote: >>>> >>>> From: Jeff Xu <jeffxu@google.com> >>>> >>>> Overlayfs can be disabled in kernel config, causing related tests to >>>> fail. Add check for overlayfs’s supportability at runtime, >>>> so we can call SKIP() when needed. >>>> >>>> Signed-off-by: Jeff Xu <jeffxu@google.com> >>> >>> Reviewed-by: Guenter Roeck <groeck@chromium.org> >>> >>>> --- >>>> tools/testing/selftests/landlock/fs_test.c | 51 ++++++++++++++++++++++ >>>> 1 file changed, 51 insertions(+) >>>> >>>> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c >>>> index 21a2ce8fa739..34095fe2419b 100644 >>>> --- a/tools/testing/selftests/landlock/fs_test.c >>>> +++ b/tools/testing/selftests/landlock/fs_test.c >>>> @@ -11,6 +11,7 @@ >>>> #include <fcntl.h> >>>> #include <linux/landlock.h> >>>> #include <sched.h> >>>> +#include <stdio.h> >>>> #include <string.h> >>>> #include <sys/capability.h> >>>> #include <sys/mount.h> >>>> @@ -62,6 +63,7 @@ static const char dir_s3d1[] = TMP_DIR "/s3d1"; >>>> static const char dir_s3d2[] = TMP_DIR "/s3d1/s3d2"; >>>> static const char dir_s3d3[] = TMP_DIR "/s3d1/s3d2/s3d3"; >>>> >>>> +static const char proc_filesystems[] = "/proc/filesystems"; >> >> You can inline this string in the fopen() call for now. >> > Done. > >> >>>> /* >>>> * layout1 hierarchy: >>>> * >>>> @@ -169,6 +171,43 @@ static int remove_path(const char *const path) >>>> return err; >>>> } >>>> >>>> +static bool fgrep(FILE *inf, const char *str) >>>> +{ >>>> + char line[32]; >>>> + int slen = strlen(str); >>>> + >>>> + while (!feof(inf)) { >>>> + if (!fgets(line, sizeof(line), inf)) >>>> + break; >>>> + if (strncmp(line, str, slen)) >>>> + continue; >>>> + >>>> + return true; >>>> + } >>>> + >>>> + return false; >>>> +} >>>> + >>>> +static bool supports_overlayfs(void) >> >> You can move this two functions just before mkdir_parents(). >> > Done. > >> >>>> +{ >>>> + bool res; >>>> + FILE *inf = fopen(proc_filesystems, "r"); >>>> + >>>> + /* >>>> + * If fopen failed, return supported. >>>> + * This help detect missing file (shall not >>>> + * happen). >> >> You can make this comment fit in two lines, with 80 columns. >> > Done. > >>>> + */ >>>> + if (!inf) >>>> + return true; >>>> + >>>> + res = fgrep(inf, "nodev\toverlay\n"); >>>> + fclose(inf); >>>> + >>>> + return res; >>>> +} >>>> + >>>> + >>>> static void prepare_layout(struct __test_metadata *const _metadata) >>>> { >>>> disable_caps(_metadata); >>>> @@ -3404,6 +3443,9 @@ FIXTURE(layout2_overlay) {}; >>>> >>>> FIXTURE_SETUP(layout2_overlay) >>>> { >>>> + if (!supports_overlayfs()) >>>> + SKIP(return, "overlayfs is not supported"); >>>> + >>>> prepare_layout(_metadata); >>>> >>>> create_directory(_metadata, LOWER_BASE); >>>> @@ -3440,6 +3482,9 @@ FIXTURE_SETUP(layout2_overlay) >>>> >>>> FIXTURE_TEARDOWN(layout2_overlay) >>>> { >>>> + if (!supports_overlayfs()) >>>> + SKIP(return, "overlayfs is not supported"); >> >> This looks good to me except the multiple supports_overlayfs() calls. >> Only the FIXTURE_SETUP() should be required. I guess some modifications >> of kselftest_harness.h are need to support that. I'd like to avoid >> touching TEST_F_FORK() which should be part of kselftest_harness.h >> >> > In kselftest_harness.h, SKIP() only applies within the function scope ( > FIXTURE_SETUP(), TEST_F_FORK(), FIXTURE_TEARDOWN()) > > If we want to apply the skip logic to all remaining steps of the testcase, > I think we should do it with a dedicated environment check hook > (FIXTURE_ENV_CHECK), > called before FIXTURE_SETUP(), if the environment check fails, all of > the remaining > test steps will be skipped. In this way, once the env check pass, > the remaining test case should also be passing, or if env check fails, > there is no need to > delete the resource since no setup is called. > > However, this requires change to the kselftest_harness.h, I do think it needs > to be a separate feature and commit (we can adopt fs_test to be the > first user) Looks good to me, implementing this FIXTURE_ENV_CHECK (or something similar) will be cleaner. Shuah, what do you think about that? > > Best regards, > Jeff > >>>> + >>>> EXPECT_EQ(0, remove_path(lower_do1_fl3)); >>>> EXPECT_EQ(0, remove_path(lower_dl1_fl2)); >>>> EXPECT_EQ(0, remove_path(lower_fl1)); >>>> @@ -3471,6 +3516,9 @@ FIXTURE_TEARDOWN(layout2_overlay) >>>> >>>> TEST_F_FORK(layout2_overlay, no_restriction) >>>> { >>>> + if (!supports_overlayfs()) >>>> + SKIP(return, "overlayfs is not supported"); >>>> + >>>> ASSERT_EQ(0, test_open(lower_fl1, O_RDONLY)); >>>> ASSERT_EQ(0, test_open(lower_dl1, O_RDONLY)); >>>> ASSERT_EQ(0, test_open(lower_dl1_fl2, O_RDONLY)); >>>> @@ -3634,6 +3682,9 @@ TEST_F_FORK(layout2_overlay, same_content_different_file) >>>> size_t i; >>>> const char *path_entry; >>>> >>>> + if (!supports_overlayfs()) >>>> + SKIP(return, "overlayfs is not supported"); >>>> + >>>> /* Sets rules on base directories (i.e. outside overlay scope). */ >>>> ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer1_base); >>>> ASSERT_LE(0, ruleset_fd); >>>> -- >>>> 2.39.0.314.g84b9a713c41-goog >>>> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-10 18:56 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-12-29 21:02 [PATCH v7 0/1] selftests/landlock: fix test when overlayfs is jeffxu 2022-12-29 21:02 ` [PATCH v7 1/1] selftests/landlock: skip overlayfs test when kernel not support it jeffxu 2022-12-29 21:41 ` Guenter Roeck 2023-01-09 16:05 ` Mickaël Salaün 2023-01-09 21:59 ` Jeff Xu 2023-01-10 18:54 ` Mickaël Salaün
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).