Please use "selftests/landlock:" as subject prefix and without a final dot.
On 07/07/2022 22:06, Günther Noack wrote:
These tests exercise the following scenarios:
* File with Read, Write, Truncate rights.
Should we use a capital for access right names or does it come from Go? ;)
* File with Read, Write rights.
* File with Truncate rights.
* File with no rights.
* Directory with Truncate rights.
For each of the scenarios, both truncate() and the open() +
ftruncate() syscalls get exercised and their results checked.
In particular, the test demonstrates that opening a file for writing
is not enough to call truncate().
Looks good! According to my previous comment, O_TRUNC should be tested
if it is checked by the kernel.
Signed-off-by: Günther Noack <gnoack3000@xxxxxxxxx>
---
tools/testing/selftests/landlock/fs_test.c | 80 ++++++++++++++++++++++
1 file changed, 80 insertions(+)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index cb77eaa01c91..c3e48fd12b2b 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -2237,6 +2237,86 @@ TEST_F_FORK(layout1, reparent_rename)
ASSERT_EQ(EXDEV, errno);
}
+TEST_F_FORK(layout1, truncate)
Please move this test after the proc_pipe one.
+{
+ const struct rule rules[] = {
You can add a first layer of rules to check truncate and ftruncate with
a ruleset not handling LANDLOCK_ACCESS_FS_TRUNCATE.
+ {
+ .path = file1_s1d1,
+ .access = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_WRITE_FILE |
+ LANDLOCK_ACCESS_FS_TRUNCATE,
+ },
+ {
+ .path = file2_s1d2,
+ .access = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_WRITE_FILE,
+ },
+ {
+ .path = file1_s1d2,
+ .access = LANDLOCK_ACCESS_FS_TRUNCATE,
+ },
Move this entry before file2_s1d2 to keep the paths sorted and make this
easier to read. You can change the access rights per path to also keep
their ordering though.
+ {
+ .path = dir_s2d3,
+ .access = LANDLOCK_ACCESS_FS_TRUNCATE,
+ },
+ // Implicitly: No access rights for file2_s1d1.
Comment to move after the use of file1_s1d1.
+ {},
+ };
+ const int ruleset_fd = create_ruleset(_metadata, ACCESS_ALL, rules);
Don't use ACCESS_ALL because it will change over time and we want tests
to be deterministic. You can use rules[0].access instead.
+ int reg_fd;
+
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Read, write and truncate permissions => truncate and ftruncate work. */
It would be nice to have consistent comments such as: "Checks read,
write and truncate access rights: truncate and ftruncate work."
+ reg_fd = open(file1_s1d1, O_RDWR | O_CLOEXEC);
+ ASSERT_LE(0, reg_fd);
+ EXPECT_EQ(0, ftruncate(reg_fd, 10));
You should not use EXPECT but ASSERT here. I use EXPECT when an error
could block a test or when it could stop a cleanup (i.e. teardown).
+ EXPECT_EQ(0, ftruncate64(reg_fd, 20));
+ ASSERT_EQ(0, close(reg_fd));
+
+ EXPECT_EQ(0, truncate(file1_s1d1, 10));
+ EXPECT_EQ(0, truncate64(file1_s1d1, 20));
+
+ /* Just read and write permissions => no truncate variant works. */
+ reg_fd = open(file2_s1d2, O_RDWR | O_CLOEXEC);
+ ASSERT_LE(0, reg_fd);
+ EXPECT_EQ(-1, ftruncate(reg_fd, 10));
+ EXPECT_EQ(EACCES, errno);
+ EXPECT_EQ(-1, ftruncate64(reg_fd, 20));
+ EXPECT_EQ(EACCES, errno);
+ ASSERT_EQ(0, close(reg_fd));
+
+ EXPECT_EQ(-1, truncate(file2_s1d2, 10));
+ EXPECT_EQ(EACCES, errno);
+ EXPECT_EQ(-1, truncate64(file2_s1d2, 20));
+ EXPECT_EQ(EACCES, errno);
+
+ /* Just truncate permissions => truncate(64) works, but can't open file. */
+ ASSERT_EQ(-1, open(file1_s1d2, O_RDWR | O_CLOEXEC));
+ ASSERT_EQ(EACCES, errno);
+
+ EXPECT_EQ(0, truncate(file1_s1d2, 10));
+ EXPECT_EQ(0, truncate64(file1_s1d2, 20));
+
+ /* Just truncate permission on directory => truncate(64) works, but can't open file. */
+ ASSERT_EQ(-1, open(file1_s2d3, O_RDWR | O_CLOEXEC));
+ ASSERT_EQ(EACCES, errno);
+
+ EXPECT_EQ(0, truncate(file1_s2d3, 10));
+ EXPECT_EQ(0, truncate64(file1_s2d3, 20));
+
+ /* No permissions => Neither truncate nor ftruncate work. */
+ ASSERT_EQ(-1, open(file2_s1d1, O_RDWR | O_CLOEXEC));
+ ASSERT_EQ(EACCES, errno);
+
+ EXPECT_EQ(-1, truncate(file2_s1d1, 10));
+ EXPECT_EQ(EACCES, errno);
+ EXPECT_EQ(-1, truncate64(file2_s1d1, 20));
+ EXPECT_EQ(EACCES, errno);
These tests are good!
+}
+
static void
reparent_exdev_layers_enforce1(struct __test_metadata *const _metadata)
{
