diff mbox series

[v4] selftests/landlock: skip overlayfs test when kernel not support it

Message ID 20220823010216.2653012-1-jeffxu@google.com (mailing list archive)
State Handled Elsewhere
Headers show
Series [v4] selftests/landlock: skip overlayfs test when kernel not support it | expand

Commit Message

Jeff Xu Aug. 23, 2022, 1:02 a.m. UTC
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@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
---
 tools/testing/selftests/landlock/fs_test.c | 53 ++++++++++++++++++++--
 1 file changed, 50 insertions(+), 3 deletions(-)


base-commit: 50cd95ac46548429e5bba7ca75cc97d11a697947

Comments

Mickaël Salaün Aug. 23, 2022, 3:28 p.m. UTC | #1
This looks good overall. You'll find some nitpicking review below.

I found that there is an issue with the skipped test, and especially the 
teardown parts:

> #  RUN           layout2_overlay.no_restriction ...
> #      SKIP      overlayfs is not supported
> # fs_test.c:3537:no_restriction:Expected 0 (0) == test_open(merge_fl1, O_RDONLY) (2)
> # fs_test.c:3512:no_restriction:Expected 0 (0) == umount(MERGE_DATA) (-1)

These messages seem OK…

> # fs_test.c:3496:no_restriction:Expected 0 (0) == umount(LOWER_BASE) (-1)
> # fs_test.c:3507:no_restriction:Expected 0 (0) == umount(UPPER_BASE) (-1)

…but these two should not happen because the tmpfs should still be mounted.


> #            OK  layout2_overlay.no_restriction
> ok 58 # SKIP overlayfs is not supported
> #  RUN           layout2_overlay.same_content_different_file ...
> # fs_test.c:3512:no_restriction:Expected 0 (0) == umount(MERGE_DATA) (-1)

These is some inconsistencies here too.

> # fs_test.c:230:no_restriction:Expected 0 (0) == umount(TMP_DIR) (-1)
> #      SKIP      overlayfs is not supported
> # fs_test.c:3723:same_content_different_file:Expected 0 (0) == test_open(path_entry, O_RDWR) (2)
> # fs_test.c:3496:same_content_different_file:Expected 0 (0) == umount(LOWER_BASE) (-1)
> # fs_test.c:3498:same_content_different_file:Expected 0 (0) == remove_path(LOWER_BASE) (16)
> # fs_test.c:3507:same_content_different_file:Expected 0 (0) == umount(UPPER_BASE) (-1)
> # fs_test.c:3509:same_content_different_file:Expected 0 (0) == remove_path(UPPER_BASE) (16)
> # fs_test.c:3512:same_content_different_file:Expected 0 (0) == umount(MERGE_DATA) (-1)
> # fs_test.c:230:same_content_different_file:Expected 0 (0) == umount(TMP_DIR) (-1)
> # fs_test.c:232:same_content_different_file:Expected 0 (0) == remove_path(TMP_DIR) (16)
> # fs_test.c:3512:same_content_different_file:Expected 0 (0) == umount(MERGE_DATA) (-1)

I guess this is because of the TEST_F_FORK() helper which doesn't handle 
well the skipped with error tests, hence the parent test process trying 
to execute the teardown and unmounting something which is not mounted it 
its namespace, and the duplicated messages.

This may be related to commit 63e6b2a42342 ("selftests/harness: Run 
TEARDOWN for ASSERT failures").

Can you fix TEST_F_FORK() for skipped tests please?

We should merge TEST_F_FORK() within kselftest_harness.h with a follow 
up patch though.


On 23/08/2022 03:02, jeffxu@google.com wrote:
> From: Jeff Xu <jeffxu@google.com>

This is not consistent with your Signed-off-by email.

> 
> 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.

selftests/landlock: Skip overlayfs tests when not supported

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 | 53 ++++++++++++++++++++--
>   1 file changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 21a2ce8fa739..aaece185579d 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";

No need for this global variable, just use the string in the fopen() call.


>   /*
>    * layout1 hierarchy:
>    *
> @@ -169,6 +171,42 @@ static int remove_path(const char *const path)
>   	return err;
>   }
>   
> +static bool fgrep(FILE *inf, const char *str)

const char *const str

s/inf/file/


> +{
> +	char line[32];
> +	int slen = strlen(str);

s/slen/str_len/

> +
> +	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 ret = false;

No need to set it to "false" (which should be "true" BTW).


> +	FILE *inf = fopen(proc_filesystems, "r");

s/inf/filename/

> +
> +	/*
> +	 * If fopen fails, return supported.
> +	 * This helps to detect missing file (shall not
> +	 * happen).

"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 (!inf)
> +		return true;
> +
> +	ret = fgrep(inf, "nodev\toverlay\n");
> +	fclose(inf);
> +

You can remove this newline.

> +	return ret;
> +}
> +
>   static void prepare_layout(struct __test_metadata *const _metadata)
>   {
>   	disable_caps(_metadata);
> @@ -3404,6 +3442,8 @@ FIXTURE(layout2_overlay) {};
>   
>   FIXTURE_SETUP(layout2_overlay)
>   {
> +	int rc;

s/rc/ret/

int ret, err;

> +
>   	prepare_layout(_metadata);
>   
>   	create_directory(_metadata, LOWER_BASE);
> @@ -3431,11 +3471,18 @@ 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));
> +
> +	rc = 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 (rc < 0) {

if (ret == -1) {

> +		ASSERT_EQ(ENODEV, errno);

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
Jeff Xu Aug. 23, 2022, 11:47 p.m. UTC | #2
> I guess this is because of the TEST_F_FORK() helper which doesn't handle
> well the skipped with error tests, hence the parent test process trying
> to execute the teardown and unmounting something which is not mounted it
> its namespace, and the duplicated messages.

> This may be related to commit 63e6b2a42342 ("selftests/harness: Run
> TEARDOWN for ASSERT failures").

> Can you fix TEST_F_FORK() for skipped tests please?

> We should merge TEST_F_FORK() within kselftest_harness.h with a follow
> up patch though.

OK. That can be after I worked on pt_test.

Would you like me to move supports_overlayfs() to the beginning of
layout2_overlay in this Patch ?
then there is nothing to cleanup.

> On 23/08/2022 03:02, jeffxu@google.com wrote:
> > From: Jeff Xu <jeffxu@google.com>
>
> This is not consistent with your Signed-off-by email.

Sure, will try to switch to send patch via jeffxu@chromium.org


On Tue, Aug 23, 2022 at 8:28 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> This looks good overall. You'll find some nitpicking review below.
>
> I found that there is an issue with the skipped test, and especially the
> teardown parts:
>
> > #  RUN           layout2_overlay.no_restriction ...
> > #      SKIP      overlayfs is not supported
> > # fs_test.c:3537:no_restriction:Expected 0 (0) == test_open(merge_fl1, O_RDONLY) (2)
> > # fs_test.c:3512:no_restriction:Expected 0 (0) == umount(MERGE_DATA) (-1)
>
> These messages seem OK…
>
> > # fs_test.c:3496:no_restriction:Expected 0 (0) == umount(LOWER_BASE) (-1)
> > # fs_test.c:3507:no_restriction:Expected 0 (0) == umount(UPPER_BASE) (-1)
>
> …but these two should not happen because the tmpfs should still be mounted.
>
>
> > #            OK  layout2_overlay.no_restriction
> > ok 58 # SKIP overlayfs is not supported
> > #  RUN           layout2_overlay.same_content_different_file ...
> > # fs_test.c:3512:no_restriction:Expected 0 (0) == umount(MERGE_DATA) (-1)
>
> These is some inconsistencies here too.
>
> > # fs_test.c:230:no_restriction:Expected 0 (0) == umount(TMP_DIR) (-1)
> > #      SKIP      overlayfs is not supported
> > # fs_test.c:3723:same_content_different_file:Expected 0 (0) == test_open(path_entry, O_RDWR) (2)
> > # fs_test.c:3496:same_content_different_file:Expected 0 (0) == umount(LOWER_BASE) (-1)
> > # fs_test.c:3498:same_content_different_file:Expected 0 (0) == remove_path(LOWER_BASE) (16)
> > # fs_test.c:3507:same_content_different_file:Expected 0 (0) == umount(UPPER_BASE) (-1)
> > # fs_test.c:3509:same_content_different_file:Expected 0 (0) == remove_path(UPPER_BASE) (16)
> > # fs_test.c:3512:same_content_different_file:Expected 0 (0) == umount(MERGE_DATA) (-1)
> > # fs_test.c:230:same_content_different_file:Expected 0 (0) == umount(TMP_DIR) (-1)
> > # fs_test.c:232:same_content_different_file:Expected 0 (0) == remove_path(TMP_DIR) (16)
> > # fs_test.c:3512:same_content_different_file:Expected 0 (0) == umount(MERGE_DATA) (-1)
>
> I guess this is because of the TEST_F_FORK() helper which doesn't handle
> well the skipped with error tests, hence the parent test process trying
> to execute the teardown and unmounting something which is not mounted it
> its namespace, and the duplicated messages.
>
> This may be related to commit 63e6b2a42342 ("selftests/harness: Run
> TEARDOWN for ASSERT failures").
>
> Can you fix TEST_F_FORK() for skipped tests please?
>
> We should merge TEST_F_FORK() within kselftest_harness.h with a follow
> up patch though.
>
>
> On 23/08/2022 03:02, jeffxu@google.com wrote:
> > From: Jeff Xu <jeffxu@google.com>
>
> This is not consistent with your Signed-off-by email.
>
> >
> > 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.
>
> selftests/landlock: Skip overlayfs tests when not supported
>
> 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 | 53 ++++++++++++++++++++--
> >   1 file changed, 50 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> > index 21a2ce8fa739..aaece185579d 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";
>
> No need for this global variable, just use the string in the fopen() call.
>
>
> >   /*
> >    * layout1 hierarchy:
> >    *
> > @@ -169,6 +171,42 @@ static int remove_path(const char *const path)
> >       return err;
> >   }
> >
> > +static bool fgrep(FILE *inf, const char *str)
>
> const char *const str
>
> s/inf/file/
>
>
> > +{
> > +     char line[32];
> > +     int slen = strlen(str);
>
> s/slen/str_len/
>
> > +
> > +     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 ret = false;
>
> No need to set it to "false" (which should be "true" BTW).
>
>
> > +     FILE *inf = fopen(proc_filesystems, "r");
>
> s/inf/filename/
>
> > +
> > +     /*
> > +      * If fopen fails, return supported.
> > +      * This helps to detect missing file (shall not
> > +      * happen).
>
> "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 (!inf)
> > +             return true;
> > +
> > +     ret = fgrep(inf, "nodev\toverlay\n");
> > +     fclose(inf);
> > +
>
> You can remove this newline.
>
> > +     return ret;
> > +}
> > +
> >   static void prepare_layout(struct __test_metadata *const _metadata)
> >   {
> >       disable_caps(_metadata);
> > @@ -3404,6 +3442,8 @@ FIXTURE(layout2_overlay) {};
> >
> >   FIXTURE_SETUP(layout2_overlay)
> >   {
> > +     int rc;
>
> s/rc/ret/
>
> int ret, err;
>
> > +
> >       prepare_layout(_metadata);
> >
> >       create_directory(_metadata, LOWER_BASE);
> > @@ -3431,11 +3471,18 @@ 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));
> > +
> > +     rc = 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 (rc < 0) {
>
> if (ret == -1) {
>
> > +             ASSERT_EQ(ENODEV, errno);
>
> 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
Mickaël Salaün Aug. 24, 2022, 9:08 a.m. UTC | #3
On 24/08/2022 01:47, Jeff Xu wrote:
>> I guess this is because of the TEST_F_FORK() helper which doesn't handle
>> well the skipped with error tests, hence the parent test process trying
>> to execute the teardown and unmounting something which is not mounted it
>> its namespace, and the duplicated messages.
> 
>> This may be related to commit 63e6b2a42342 ("selftests/harness: Run
>> TEARDOWN for ASSERT failures").
> 
>> Can you fix TEST_F_FORK() for skipped tests please?
> 
>> We should merge TEST_F_FORK() within kselftest_harness.h with a follow
>> up patch though.
> 
> OK. That can be after I worked on pt_test.

Right, but that is independent from the TEST_F_FORK() fix which should 
be easier to do and make backport possible.


> 
> Would you like me to move supports_overlayfs() to the beginning of
> layout2_overlay in this Patch ?
> then there is nothing to cleanup.

I'm not sure this would be enough, and the current approach is good. 
TEST_F_FORK() needs some investigation.


> 
>> On 23/08/2022 03:02, jeffxu@google.com wrote:
>>> From: Jeff Xu <jeffxu@google.com>
>>
>> This is not consistent with your Signed-off-by email.
> 
> Sure, will try to switch to send patch via jeffxu@chromium.org
> 
> 
> On Tue, Aug 23, 2022 at 8:28 AM Mickaël Salaün <mic@digikod.net> wrote:
>>
>> This looks good overall. You'll find some nitpicking review below.
>>
>> I found that there is an issue with the skipped test, and especially the
>> teardown parts:
>>
>>> #  RUN           layout2_overlay.no_restriction ...
>>> #      SKIP      overlayfs is not supported
>>> # fs_test.c:3537:no_restriction:Expected 0 (0) == test_open(merge_fl1, O_RDONLY) (2)
>>> # fs_test.c:3512:no_restriction:Expected 0 (0) == umount(MERGE_DATA) (-1)
>>
>> These messages seem OK…
>>
>>> # fs_test.c:3496:no_restriction:Expected 0 (0) == umount(LOWER_BASE) (-1)
>>> # fs_test.c:3507:no_restriction:Expected 0 (0) == umount(UPPER_BASE) (-1)
>>
>> …but these two should not happen because the tmpfs should still be mounted.
>>
>>
>>> #            OK  layout2_overlay.no_restriction
>>> ok 58 # SKIP overlayfs is not supported
>>> #  RUN           layout2_overlay.same_content_different_file ...
>>> # fs_test.c:3512:no_restriction:Expected 0 (0) == umount(MERGE_DATA) (-1)
>>
>> These is some inconsistencies here too.
>>
>>> # fs_test.c:230:no_restriction:Expected 0 (0) == umount(TMP_DIR) (-1)
>>> #      SKIP      overlayfs is not supported
>>> # fs_test.c:3723:same_content_different_file:Expected 0 (0) == test_open(path_entry, O_RDWR) (2)
>>> # fs_test.c:3496:same_content_different_file:Expected 0 (0) == umount(LOWER_BASE) (-1)
>>> # fs_test.c:3498:same_content_different_file:Expected 0 (0) == remove_path(LOWER_BASE) (16)
>>> # fs_test.c:3507:same_content_different_file:Expected 0 (0) == umount(UPPER_BASE) (-1)
>>> # fs_test.c:3509:same_content_different_file:Expected 0 (0) == remove_path(UPPER_BASE) (16)
>>> # fs_test.c:3512:same_content_different_file:Expected 0 (0) == umount(MERGE_DATA) (-1)
>>> # fs_test.c:230:same_content_different_file:Expected 0 (0) == umount(TMP_DIR) (-1)
>>> # fs_test.c:232:same_content_different_file:Expected 0 (0) == remove_path(TMP_DIR) (16)
>>> # fs_test.c:3512:same_content_different_file:Expected 0 (0) == umount(MERGE_DATA) (-1)
>>
>> I guess this is because of the TEST_F_FORK() helper which doesn't handle
>> well the skipped with error tests, hence the parent test process trying
>> to execute the teardown and unmounting something which is not mounted it
>> its namespace, and the duplicated messages.
>>
>> This may be related to commit 63e6b2a42342 ("selftests/harness: Run
>> TEARDOWN for ASSERT failures").
>>
>> Can you fix TEST_F_FORK() for skipped tests please?
>>
>> We should merge TEST_F_FORK() within kselftest_harness.h with a follow
>> up patch though.
>>
>>
>> On 23/08/2022 03:02, jeffxu@google.com wrote:
>>> From: Jeff Xu <jeffxu@google.com>
>>
>> This is not consistent with your Signed-off-by email.
>>
>>>
>>> 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.
>>
>> selftests/landlock: Skip overlayfs tests when not supported
>>
>> 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 | 53 ++++++++++++++++++++--
>>>    1 file changed, 50 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
>>> index 21a2ce8fa739..aaece185579d 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";
>>
>> No need for this global variable, just use the string in the fopen() call.
>>
>>
>>>    /*
>>>     * layout1 hierarchy:
>>>     *
>>> @@ -169,6 +171,42 @@ static int remove_path(const char *const path)
>>>        return err;
>>>    }
>>>
>>> +static bool fgrep(FILE *inf, const char *str)
>>
>> const char *const str
>>
>> s/inf/file/
>>
>>
>>> +{
>>> +     char line[32];
>>> +     int slen = strlen(str);
>>
>> s/slen/str_len/
>>
>>> +
>>> +     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 ret = false;
>>
>> No need to set it to "false" (which should be "true" BTW).
>>
>>
>>> +     FILE *inf = fopen(proc_filesystems, "r");
>>
>> s/inf/filename/
>>
>>> +
>>> +     /*
>>> +      * If fopen fails, return supported.
>>> +      * This helps to detect missing file (shall not
>>> +      * happen).
>>
>> "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 (!inf)
>>> +             return true;
>>> +
>>> +     ret = fgrep(inf, "nodev\toverlay\n");
>>> +     fclose(inf);
>>> +
>>
>> You can remove this newline.
>>
>>> +     return ret;
>>> +}
>>> +
>>>    static void prepare_layout(struct __test_metadata *const _metadata)
>>>    {
>>>        disable_caps(_metadata);
>>> @@ -3404,6 +3442,8 @@ FIXTURE(layout2_overlay) {};
>>>
>>>    FIXTURE_SETUP(layout2_overlay)
>>>    {
>>> +     int rc;
>>
>> s/rc/ret/
>>
>> int ret, err;
>>
>>> +
>>>        prepare_layout(_metadata);
>>>
>>>        create_directory(_metadata, LOWER_BASE);
>>> @@ -3431,11 +3471,18 @@ 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));
>>> +
>>> +     rc = 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 (rc < 0) {
>>
>> if (ret == -1) {
>>
>>> +             ASSERT_EQ(ENODEV, errno);
>>
>> 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 mbox series

Patch

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 21a2ce8fa739..aaece185579d 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,42 @@  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 ret = false;
+	FILE *inf = fopen(proc_filesystems, "r");
+
+	/*
+	 * If fopen fails, return supported.
+	 * This helps to detect missing file (shall not
+	 * happen).
+	 */
+	if (!inf)
+		return true;
+
+	ret = fgrep(inf, "nodev\toverlay\n");
+	fclose(inf);
+
+	return ret;
+}
+
 static void prepare_layout(struct __test_metadata *const _metadata)
 {
 	disable_caps(_metadata);
@@ -3404,6 +3442,8 @@  FIXTURE(layout2_overlay) {};
 
 FIXTURE_SETUP(layout2_overlay)
 {
+	int rc;
+
 	prepare_layout(_metadata);
 
 	create_directory(_metadata, LOWER_BASE);
@@ -3431,11 +3471,18 @@  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));
+
+	rc = mount("overlay", MERGE_DATA, "overlay", 0,
+		   "lowerdir=" LOWER_DATA ",upperdir=" UPPER_DATA
+		   ",workdir=" UPPER_WORK);
 	clear_cap(_metadata, CAP_DAC_OVERRIDE);
 	clear_cap(_metadata, CAP_SYS_ADMIN);
+
+	if (rc < 0) {
+		ASSERT_EQ(ENODEV, errno);
+		ASSERT_FALSE(supports_overlayfs());
+		SKIP(return, "overlayfs is not supported");
+	}
 }
 
 FIXTURE_TEARDOWN(layout2_overlay)