[PATCH v1 2/5] selftests/landlock: Fix FS tests when run on a private mount point

Mickaël Salaün posted 5 patches 1 year, 9 months ago
There is a newer version of this series
[PATCH v1 2/5] selftests/landlock: Fix FS tests when run on a private mount point
Posted by Mickaël Salaün 1 year, 9 months ago
According to the test environment, the mount point of the test's working
directory may be shared or not, which changes the visibility of the
nested "tmp" mount point for the test's parent process calling
umount("tmp").

This was spotted while running tests on different Linux distributions,
with different mount point configurations.

Cc: Günther Noack <gnoack@google.com>
Fixes: 41cca0542d7c ("selftests/harness: Fix TEST_F()'s vfork handling")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20240426172252.1862930-3-mic@digikod.net
---
 tools/testing/selftests/landlock/fs_test.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 9a6036fbf289..46b9effd53e4 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -293,7 +293,15 @@ static void prepare_layout(struct __test_metadata *const _metadata)
 static void cleanup_layout(struct __test_metadata *const _metadata)
 {
 	set_cap(_metadata, CAP_SYS_ADMIN);
-	EXPECT_EQ(0, umount(TMP_DIR));
+	if (umount(TMP_DIR)) {
+		/*
+		 * According to the test environment, the mount point of the
+		 * current directory may be shared or not, which changes the
+		 * visibility of the nested TMP_DIR mount point for the test's
+		 * parent process doing this cleanup.
+		 */
+		ASSERT_EQ(EINVAL, errno);
+	}
 	clear_cap(_metadata, CAP_SYS_ADMIN);
 	EXPECT_EQ(0, remove_path(TMP_DIR));
 }
-- 
2.44.0

Re: [PATCH v1 2/5] selftests/landlock: Fix FS tests when run on a private mount point
Posted by Kees Cook 1 year, 9 months ago
On Fri, Apr 26, 2024 at 07:22:49PM +0200, Mickaël Salaün wrote:
> According to the test environment, the mount point of the test's working
> directory may be shared or not, which changes the visibility of the
> nested "tmp" mount point for the test's parent process calling
> umount("tmp").
> 
> This was spotted while running tests on different Linux distributions,
> with different mount point configurations.

Which distros did what?

> 
> Cc: Günther Noack <gnoack@google.com>
> Fixes: 41cca0542d7c ("selftests/harness: Fix TEST_F()'s vfork handling")
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20240426172252.1862930-3-mic@digikod.net

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  tools/testing/selftests/landlock/fs_test.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 9a6036fbf289..46b9effd53e4 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -293,7 +293,15 @@ static void prepare_layout(struct __test_metadata *const _metadata)
>  static void cleanup_layout(struct __test_metadata *const _metadata)
>  {
>  	set_cap(_metadata, CAP_SYS_ADMIN);
> -	EXPECT_EQ(0, umount(TMP_DIR));
> +	if (umount(TMP_DIR)) {
> +		/*
> +		 * According to the test environment, the mount point of the
> +		 * current directory may be shared or not, which changes the
> +		 * visibility of the nested TMP_DIR mount point for the test's
> +		 * parent process doing this cleanup.
> +		 */
> +		ASSERT_EQ(EINVAL, errno);
> +	}
>  	clear_cap(_metadata, CAP_SYS_ADMIN);
>  	EXPECT_EQ(0, remove_path(TMP_DIR));
>  }
> -- 
> 2.44.0
> 

-- 
Kees Cook
Re: [PATCH v1 2/5] selftests/landlock: Fix FS tests when run on a private mount point
Posted by Mickaël Salaün 1 year, 9 months ago
On Fri, Apr 26, 2024 at 12:38:17PM -0700, Kees Cook wrote:
> On Fri, Apr 26, 2024 at 07:22:49PM +0200, Mickaël Salaün wrote:
> > According to the test environment, the mount point of the test's working
> > directory may be shared or not, which changes the visibility of the
> > nested "tmp" mount point for the test's parent process calling
> > umount("tmp").
> > 
> > This was spotted while running tests on different Linux distributions,
> > with different mount point configurations.
> 
> Which distros did what?

Actually it's not related to distros, but rather container runtime
(Docker) vs. non-container environment.  With Docker (at least on my
environment) all mount points are private, which is not the case (by
default) when running the same UML environment not in a container. See
https://github.com/landlock-lsm/landlock-test-tools/pull/4

I'll update the description.