2022-05-09 03:18:59

by Mickaël Salaün

[permalink] [raw]
Subject: [PATCH v2 06/10] selftests/landlock: Fully test file rename with "remove" access

These tests were missing to check the check_access_path() call with all
combinations of maybe_remove(old_dentry) and maybe_remove(new_dentry).

Extend layout1.link with a new complementary test and check that
REMOVE_FILE is not required to link a file.

Cc: Shuah Khan <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---

Changes since v1:
* Extend layout1.link with a second rule layer to check that REMOVE_FILE
is not required to link a file.
* Format with clang-format and rebase.
---
tools/testing/selftests/landlock/fs_test.c | 41 +++++++++++++++++++---
1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 75f9358512df..9165f6adf7b9 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -1659,15 +1659,21 @@ TEST_F_FORK(layout1, execute)

TEST_F_FORK(layout1, link)
{
- const struct rule rules[] = {
+ const struct rule layer1[] = {
{
.path = dir_s1d2,
.access = LANDLOCK_ACCESS_FS_MAKE_REG,
},
{},
};
- const int ruleset_fd =
- create_ruleset(_metadata, rules[0].access, rules);
+ const struct rule layer2[] = {
+ {
+ .path = dir_s1d3,
+ .access = LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ },
+ {},
+ };
+ int ruleset_fd = create_ruleset(_metadata, layer1[0].access, layer1);

ASSERT_LE(0, ruleset_fd);

@@ -1680,14 +1686,30 @@ TEST_F_FORK(layout1, link)

ASSERT_EQ(-1, link(file2_s1d1, file1_s1d1));
ASSERT_EQ(EACCES, errno);
+
/* Denies linking because of reparenting. */
ASSERT_EQ(-1, link(file1_s2d1, file1_s1d2));
ASSERT_EQ(EXDEV, errno);
ASSERT_EQ(-1, link(file2_s1d2, file1_s1d3));
ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(-1, link(file2_s1d3, file1_s1d2));
+ ASSERT_EQ(EXDEV, errno);

ASSERT_EQ(0, link(file2_s1d2, file1_s1d2));
ASSERT_EQ(0, link(file2_s1d3, file1_s1d3));
+
+ /* Prepares for next unlinks. */
+ ASSERT_EQ(0, unlink(file2_s1d2));
+ ASSERT_EQ(0, unlink(file2_s1d3));
+
+ ruleset_fd = create_ruleset(_metadata, layer2[0].access, layer2);
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Checks that linkind doesn't require the ability to delete a file. */
+ ASSERT_EQ(0, link(file1_s1d2, file2_s1d2));
+ ASSERT_EQ(0, link(file1_s1d3, file2_s1d3));
}

TEST_F_FORK(layout1, rename_file)
@@ -1708,7 +1730,6 @@ TEST_F_FORK(layout1, rename_file)

ASSERT_LE(0, ruleset_fd);

- ASSERT_EQ(0, unlink(file1_s1d1));
ASSERT_EQ(0, unlink(file1_s1d2));

enforce_ruleset(_metadata, ruleset_fd);
@@ -1744,9 +1765,15 @@ TEST_F_FORK(layout1, rename_file)
ASSERT_EQ(-1, renameat2(AT_FDCWD, dir_s2d2, AT_FDCWD, file1_s2d1,
RENAME_EXCHANGE));
ASSERT_EQ(EACCES, errno);
+ /* Checks that file1_s2d1 cannot be removed (instead of ENOTDIR). */
+ ASSERT_EQ(-1, rename(dir_s2d2, file1_s2d1));
+ ASSERT_EQ(EACCES, errno);
ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s2d1, AT_FDCWD, dir_s2d2,
RENAME_EXCHANGE));
ASSERT_EQ(EACCES, errno);
+ /* Checks that file1_s1d1 cannot be removed (instead of EISDIR). */
+ ASSERT_EQ(-1, rename(file1_s1d1, dir_s1d2));
+ ASSERT_EQ(EACCES, errno);

/* Renames files with different parents. */
ASSERT_EQ(-1, rename(file1_s2d2, file1_s1d2));
@@ -1809,9 +1836,15 @@ TEST_F_FORK(layout1, rename_dir)
ASSERT_EQ(-1, renameat2(AT_FDCWD, dir_s1d1, AT_FDCWD, dir_s2d1,
RENAME_EXCHANGE));
ASSERT_EQ(EACCES, errno);
+ /* Checks that dir_s1d2 cannot be removed (instead of ENOTDIR). */
+ ASSERT_EQ(-1, rename(dir_s1d2, file1_s1d1));
+ ASSERT_EQ(EACCES, errno);
ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s1d1, AT_FDCWD, dir_s1d2,
RENAME_EXCHANGE));
ASSERT_EQ(EACCES, errno);
+ /* Checks that dir_s1d2 cannot be removed (instead of EISDIR). */
+ ASSERT_EQ(-1, rename(file1_s1d1, dir_s1d2));
+ ASSERT_EQ(EACCES, errno);

/*
* Exchanges and renames directory to the same parent, which allows
--
2.35.1