Re: [PATCH v4 12/30] selftests/landlock: Add test to check partial access in a mount tree

["Mickaël Salaün" <mic@digikod.net>]

On Wed, Jan 08, 2025 at 04:43:20PM +0100, Mickaël Salaün wrote:
> Add layout1.refer_part_mount_tree_is_allowed to test the masked logical
> issue regarding collect_domain_accesses() calls followed by the
> is_access_to_paths_allowed() check in current_check_refer_path().  See
> previous commit.
> 
> This test should work without the previous fix as well, but it enables
> us to make sure future changes will not have impact regarding this
> behavior.
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20250108154338.1129069-13-mic@digikod.net

Pushed in my next tree to simplify next patch series.

> ---
> 
> Changes since v2:
> - New patch.
> ---
>  tools/testing/selftests/landlock/fs_test.c | 54 ++++++++++++++++++++--
>  1 file changed, 50 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 6788762188fe..42ce1e79ba82 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -85,6 +85,9 @@ static const char file1_s3d1[] = TMP_DIR "/s3d1/f1";
>  /* dir_s3d2 is a mount point. */
>  static const char dir_s3d2[] = TMP_DIR "/s3d1/s3d2";
>  static const char dir_s3d3[] = TMP_DIR "/s3d1/s3d2/s3d3";
> +static const char file1_s3d3[] = TMP_DIR "/s3d1/s3d2/s3d3/f1";
> +static const char dir_s3d4[] = TMP_DIR "/s3d1/s3d2/s3d4";
> +static const char file1_s3d4[] = TMP_DIR "/s3d1/s3d2/s3d4/f1";
>  
>  /*
>   * layout1 hierarchy:
> @@ -108,8 +111,11 @@ static const char dir_s3d3[] = TMP_DIR "/s3d1/s3d2/s3d3";
>   * │           └── f2
>   * └── s3d1
>   *     ├── f1
> - *     └── s3d2
> - *         └── s3d3
> + *     └── s3d2 [mount point]
> + *         ├── s3d3
> + *         │   └── f1
> + *         └── s3d4
> + *             └── f1
>   */
>  
>  static bool fgrep(FILE *const inf, const char *const str)
> @@ -358,7 +364,8 @@ static void create_layout1(struct __test_metadata *const _metadata)
>  	ASSERT_EQ(0, mount_opt(&mnt_tmp, dir_s3d2));
>  	clear_cap(_metadata, CAP_SYS_ADMIN);
>  
> -	ASSERT_EQ(0, mkdir(dir_s3d3, 0700));
> +	create_file(_metadata, file1_s3d3);
> +	create_file(_metadata, file1_s3d4);
>  }
>  
>  static void remove_layout1(struct __test_metadata *const _metadata)
> @@ -378,7 +385,8 @@ static void remove_layout1(struct __test_metadata *const _metadata)
>  	EXPECT_EQ(0, remove_path(dir_s2d2));
>  
>  	EXPECT_EQ(0, remove_path(file1_s3d1));
> -	EXPECT_EQ(0, remove_path(dir_s3d3));
> +	EXPECT_EQ(0, remove_path(file1_s3d3));
> +	EXPECT_EQ(0, remove_path(file1_s3d4));
>  	set_cap(_metadata, CAP_SYS_ADMIN);
>  	umount(dir_s3d2);
>  	clear_cap(_metadata, CAP_SYS_ADMIN);
> @@ -2444,6 +2452,44 @@ TEST_F_FORK(layout1, refer_mount_root_deny)
>  	EXPECT_EQ(0, close(root_fd));
>  }
>  
> +TEST_F_FORK(layout1, refer_part_mount_tree_is_allowed)
> +{
> +	const struct rule layer1[] = {
> +		{
> +			/* Parent mount point. */
> +			.path = dir_s3d1,
> +			.access = LANDLOCK_ACCESS_FS_REFER |
> +				  LANDLOCK_ACCESS_FS_MAKE_REG,
> +		},
> +		{
> +			/*
> +			 * Removing the source file is allowed because its
> +			 * access rights are already a superset of the
> +			 * destination.
> +			 */
> +			.path = dir_s3d4,
> +			.access = LANDLOCK_ACCESS_FS_REFER |
> +				  LANDLOCK_ACCESS_FS_MAKE_REG |
> +				  LANDLOCK_ACCESS_FS_REMOVE_FILE,
> +		},
> +		{},
> +	};
> +	int ruleset_fd;
> +
> +	ASSERT_EQ(0, unlink(file1_s3d3));
> +	ruleset_fd = create_ruleset(_metadata,
> +				    LANDLOCK_ACCESS_FS_REFER |
> +					    LANDLOCK_ACCESS_FS_MAKE_REG |
> +					    LANDLOCK_ACCESS_FS_REMOVE_FILE,
> +				    layer1);
> +
> +	ASSERT_LE(0, ruleset_fd);
> +	enforce_ruleset(_metadata, ruleset_fd);
> +	ASSERT_EQ(0, close(ruleset_fd));
> +
> +	ASSERT_EQ(0, rename(file1_s3d4, file1_s3d3));
> +}
> +
>  TEST_F_FORK(layout1, reparent_link)
>  {
>  	const struct rule layer1[] = {
> -- 
> 2.47.1
> 
>