| Message ID | 20220824015852.32257-1-jeffxu@chromium.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v5] selftests/landlock: Skip overlayfs tests not supported | expand |
As discussed for the v4, the next version of this patch needs a TEST_F_FORK() fix. Please add a link to the previous patch (lore.kernel.org) for each new version. On 24/08/2022 03:58, jeffxu@chromium.org wrote: > From: Jeff Xu <jeffxu@chromium.org> > > overlayfs can be disabled in the kernel configuration (which is the case > for chromeOS), causing related tests to fail. Skip such tests when an > overlayfs mount operation failed because the running kernel doesn't > support this file system. > > Signed-off-by: Jeff Xu <jeffxu@chromium.org> > Reviewed-by: Guenter Roeck <groeck@chromium.org> > --- > tools/testing/selftests/landlock/fs_test.c | 54 ++++++++++++++++++++-- > 1 file changed, 51 insertions(+), 3 deletions(-) > > diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c > index 21a2ce8fa739..645304d9fe98 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> > @@ -169,6 +170,42 @@ static int remove_path(const char *const path) > return err; > } > > +static bool fgrep(FILE *file, const char *str) > +{ > + char line[32]; > + int str_len = strlen(str); > + > + while (!feof(file)) { > + if (!fgets(line, sizeof(line), file)) > + break; > + if (strncmp(line, str, str_len)) > + continue; > + > + return true; > + } > + > + return false; > +} > + > +static bool supports_overlayfs(void) > +{ > + bool ret; > + FILE *file = fopen("/proc/filesystems", "r"); > + > + /* > + * A failed attempt to open /proc/filesystems > + * implies that the file system is supported (default > + * behavior). This can help detect such unattended issue > + * (which should not happen)." > + */ > + if (!file) > + return true; > + > + ret = fgrep(file, "nodev\toverlay\n"); > + fclose(file); > + return ret; > +} > + > static void prepare_layout(struct __test_metadata *const _metadata) > { > disable_caps(_metadata); > @@ -3404,6 +3441,8 @@ FIXTURE(layout2_overlay) {}; > > FIXTURE_SETUP(layout2_overlay) > { > + int ret, err; > + > prepare_layout(_metadata); > > create_directory(_metadata, LOWER_BASE); > @@ -3431,11 +3470,20 @@ FIXTURE_SETUP(layout2_overlay) > create_directory(_metadata, MERGE_DATA); > set_cap(_metadata, CAP_SYS_ADMIN); > set_cap(_metadata, CAP_DAC_OVERRIDE); > - ASSERT_EQ(0, mount("overlay", MERGE_DATA, "overlay", 0, > - "lowerdir=" LOWER_DATA ",upperdir=" UPPER_DATA > - ",workdir=" UPPER_WORK)); > + > + ret = mount("overlay", MERGE_DATA, "overlay", 0, > + "lowerdir=" LOWER_DATA ",upperdir=" UPPER_DATA > + ",workdir=" UPPER_WORK); > + err = errno; > clear_cap(_metadata, CAP_DAC_OVERRIDE); > clear_cap(_metadata, CAP_SYS_ADMIN); > + > + if (ret == -1) { > + ASSERT_EQ(ENODEV, err); > + ASSERT_FALSE(supports_overlayfs()); > + SKIP(return, "overlayfs is not supported"); > + } > + ASSERT_EQ(0, ret); > } > > FIXTURE_TEARDOWN(layout2_overlay) > > base-commit: 50cd95ac46548429e5bba7ca75cc97d11a697947
On Thu, Aug 25, 2022 at 1:23 AM Mickaël Salaün <mic@digikod.net> wrote: > > As discussed for the v4, the next version of this patch needs a > TEST_F_FORK() fix. > I can make TEST_F_FORK() to be skipped when SKIP() is called in FIXTURE_SETUP(), but this makes FIXTURE_TEARDOWN() complicated, because SKIP() can be called after any resource creation failure in the FIXTURE_SETUP(). Another (better) option: add generic FIXTURE_CONFIG_CHECK() FIXTURE_CONFIG_CHECK() checks the runtime configuration for current FIXTURE, if the configuration is not met, the whole test will be skipped, including FIXTURE_SETUP()/TEARDOWN(), TEST_F_FORK(), so there is no resource clear up issue after test. > Please add a link to the previous patch (lore.kernel.org) for each new > version. > > > On 24/08/2022 03:58, jeffxu@chromium.org wrote: > > From: Jeff Xu <jeffxu@chromium.org> > > > > overlayfs can be disabled in the kernel configuration (which is the case > > for chromeOS), causing related tests to fail. Skip such tests when an > > overlayfs mount operation failed because the running kernel doesn't > > support this file system. > > > > Signed-off-by: Jeff Xu <jeffxu@chromium.org> > > Reviewed-by: Guenter Roeck <groeck@chromium.org> > > --- > > tools/testing/selftests/landlock/fs_test.c | 54 ++++++++++++++++++++-- > > 1 file changed, 51 insertions(+), 3 deletions(-) > > > > diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c > > index 21a2ce8fa739..645304d9fe98 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> > > @@ -169,6 +170,42 @@ static int remove_path(const char *const path) > > return err; > > } > > > > +static bool fgrep(FILE *file, const char *str) > > +{ > > + char line[32]; > > + int str_len = strlen(str); > > + > > + while (!feof(file)) { > > + if (!fgets(line, sizeof(line), file)) > > + break; > > + if (strncmp(line, str, str_len)) > > + continue; > > + > > + return true; > > + } > > + > > + return false; > > +} > > + > > +static bool supports_overlayfs(void) > > +{ > > + bool ret; > > + FILE *file = fopen("/proc/filesystems", "r"); > > + > > + /* > > + * A failed attempt to open /proc/filesystems > > + * implies that the file system is supported (default > > + * behavior). This can help detect such unattended issue > > + * (which should not happen)." > > + */ > > + if (!file) > > + return true; > > + > > + ret = fgrep(file, "nodev\toverlay\n"); > > + fclose(file); > > + return ret; > > +} > > + > > static void prepare_layout(struct __test_metadata *const _metadata) > > { > > disable_caps(_metadata); > > @@ -3404,6 +3441,8 @@ FIXTURE(layout2_overlay) {}; > > > > FIXTURE_SETUP(layout2_overlay) > > { > > + int ret, err; > > + > > prepare_layout(_metadata); > > > > create_directory(_metadata, LOWER_BASE); > > @@ -3431,11 +3470,20 @@ FIXTURE_SETUP(layout2_overlay) > > create_directory(_metadata, MERGE_DATA); > > set_cap(_metadata, CAP_SYS_ADMIN); > > set_cap(_metadata, CAP_DAC_OVERRIDE); > > - ASSERT_EQ(0, mount("overlay", MERGE_DATA, "overlay", 0, > > - "lowerdir=" LOWER_DATA ",upperdir=" UPPER_DATA > > - ",workdir=" UPPER_WORK)); > > + > > + ret = mount("overlay", MERGE_DATA, "overlay", 0, > > + "lowerdir=" LOWER_DATA ",upperdir=" UPPER_DATA > > + ",workdir=" UPPER_WORK); > > + err = errno; > > clear_cap(_metadata, CAP_DAC_OVERRIDE); > > clear_cap(_metadata, CAP_SYS_ADMIN); > > + > > + if (ret == -1) { > > + ASSERT_EQ(ENODEV, err); > > + ASSERT_FALSE(supports_overlayfs()); > > + SKIP(return, "overlayfs is not supported"); > > + } > > + ASSERT_EQ(0, ret); > > } > > > > FIXTURE_TEARDOWN(layout2_overlay) > > > > base-commit: 50cd95ac46548429e5bba7ca75cc97d11a697947
On 26/09/2022 16:23, Jeff Xu wrote: > On Thu, Aug 25, 2022 at 1:23 AM Mickaël Salaün <mic@digikod.net> wrote: >> >> As discussed for the v4, the next version of this patch needs a >> TEST_F_FORK() fix. >> > I can make TEST_F_FORK() to be skipped when SKIP() is called > in FIXTURE_SETUP(), but this makes FIXTURE_TEARDOWN() > complicated, because SKIP() can be called after any resource > creation failure in the FIXTURE_SETUP(). > > Another (better) option: add generic FIXTURE_CONFIG_CHECK() > FIXTURE_CONFIG_CHECK() checks the runtime configuration for > current FIXTURE, if the configuration is not met, the whole test will be > skipped, including FIXTURE_SETUP()/TEARDOWN(), TEST_F_FORK(), > so there is no resource clear up issue after test. This looks like a good idea. What do you think Shuah? > >> Please add a link to the previous patch (lore.kernel.org) for each new >> version. >> >> >> On 24/08/2022 03:58, jeffxu@chromium.org wrote: >>> From: Jeff Xu <jeffxu@chromium.org> >>> >>> overlayfs can be disabled in the kernel configuration (which is the case >>> for chromeOS), causing related tests to fail. Skip such tests when an >>> overlayfs mount operation failed because the running kernel doesn't >>> support this file system. >>> >>> Signed-off-by: Jeff Xu <jeffxu@chromium.org> >>> Reviewed-by: Guenter Roeck <groeck@chromium.org> >>> --- >>> tools/testing/selftests/landlock/fs_test.c | 54 ++++++++++++++++++++-- >>> 1 file changed, 51 insertions(+), 3 deletions(-) >>> >>> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c >>> index 21a2ce8fa739..645304d9fe98 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> >>> @@ -169,6 +170,42 @@ static int remove_path(const char *const path) >>> return err; >>> } >>> >>> +static bool fgrep(FILE *file, const char *str) >>> +{ >>> + char line[32]; >>> + int str_len = strlen(str); >>> + >>> + while (!feof(file)) { >>> + if (!fgets(line, sizeof(line), file)) >>> + break; >>> + if (strncmp(line, str, str_len)) >>> + continue; >>> + >>> + return true; >>> + } >>> + >>> + return false; >>> +} >>> + >>> +static bool supports_overlayfs(void) >>> +{ >>> + bool ret; >>> + FILE *file = fopen("/proc/filesystems", "r"); >>> + >>> + /* >>> + * A failed attempt to open /proc/filesystems >>> + * implies that the file system is supported (default >>> + * behavior). This can help detect such unattended issue >>> + * (which should not happen)." >>> + */ >>> + if (!file) >>> + return true; >>> + >>> + ret = fgrep(file, "nodev\toverlay\n"); >>> + fclose(file); >>> + return ret; >>> +} >>> + >>> static void prepare_layout(struct __test_metadata *const _metadata) >>> { >>> disable_caps(_metadata); >>> @@ -3404,6 +3441,8 @@ FIXTURE(layout2_overlay) {}; >>> >>> FIXTURE_SETUP(layout2_overlay) >>> { >>> + int ret, err; >>> + >>> prepare_layout(_metadata); >>> >>> create_directory(_metadata, LOWER_BASE); >>> @@ -3431,11 +3470,20 @@ FIXTURE_SETUP(layout2_overlay) >>> create_directory(_metadata, MERGE_DATA); >>> set_cap(_metadata, CAP_SYS_ADMIN); >>> set_cap(_metadata, CAP_DAC_OVERRIDE); >>> - ASSERT_EQ(0, mount("overlay", MERGE_DATA, "overlay", 0, >>> - "lowerdir=" LOWER_DATA ",upperdir=" UPPER_DATA >>> - ",workdir=" UPPER_WORK)); >>> + >>> + ret = mount("overlay", MERGE_DATA, "overlay", 0, >>> + "lowerdir=" LOWER_DATA ",upperdir=" UPPER_DATA >>> + ",workdir=" UPPER_WORK); >>> + err = errno; >>> clear_cap(_metadata, CAP_DAC_OVERRIDE); >>> clear_cap(_metadata, CAP_SYS_ADMIN); >>> + >>> + if (ret == -1) { >>> + ASSERT_EQ(ENODEV, err); >>> + ASSERT_FALSE(supports_overlayfs()); >>> + SKIP(return, "overlayfs is not supported"); >>> + } >>> + ASSERT_EQ(0, ret); >>> } >>> >>> FIXTURE_TEARDOWN(layout2_overlay) >>> >>> base-commit: 50cd95ac46548429e5bba7ca75cc97d11a697947
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 21a2ce8fa739..645304d9fe98 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> @@ -169,6 +170,42 @@ static int remove_path(const char *const path) return err; } +static bool fgrep(FILE *file, const char *str) +{ + char line[32]; + int str_len = strlen(str); + + while (!feof(file)) { + if (!fgets(line, sizeof(line), file)) + break; + if (strncmp(line, str, str_len)) + continue; + + return true; + } + + return false; +} + +static bool supports_overlayfs(void) +{ + bool ret; + FILE *file = fopen("/proc/filesystems", "r"); + + /* + * A failed attempt to open /proc/filesystems + * implies that the file system is supported (default + * behavior). This can help detect such unattended issue + * (which should not happen)." + */ + if (!file) + return true; + + ret = fgrep(file, "nodev\toverlay\n"); + fclose(file); + return ret; +} + static void prepare_layout(struct __test_metadata *const _metadata) { disable_caps(_metadata); @@ -3404,6 +3441,8 @@ FIXTURE(layout2_overlay) {}; FIXTURE_SETUP(layout2_overlay) { + int ret, err; + prepare_layout(_metadata); create_directory(_metadata, LOWER_BASE); @@ -3431,11 +3470,20 @@ FIXTURE_SETUP(layout2_overlay) create_directory(_metadata, MERGE_DATA); set_cap(_metadata, CAP_SYS_ADMIN); set_cap(_metadata, CAP_DAC_OVERRIDE); - ASSERT_EQ(0, mount("overlay", MERGE_DATA, "overlay", 0, - "lowerdir=" LOWER_DATA ",upperdir=" UPPER_DATA - ",workdir=" UPPER_WORK)); + + ret = mount("overlay", MERGE_DATA, "overlay", 0, + "lowerdir=" LOWER_DATA ",upperdir=" UPPER_DATA + ",workdir=" UPPER_WORK); + err = errno; clear_cap(_metadata, CAP_DAC_OVERRIDE); clear_cap(_metadata, CAP_SYS_ADMIN); + + if (ret == -1) { + ASSERT_EQ(ENODEV, err); + ASSERT_FALSE(supports_overlayfs()); + SKIP(return, "overlayfs is not supported"); + } + ASSERT_EQ(0, ret); } FIXTURE_TEARDOWN(layout2_overlay)