On Fri, Sep 16, 2022 at 07:05:44PM +0200, Mickaël Salaün wrote: > I'd like to have tests similar to base_test.c:ruleset_fd_transfer to check > ftruncate with different kind of file descriptors and not-sandboxed > processes. That would require some code refactoring to reuse the FD passing > code. Done. I factored out the FD sending and receiving into helper function in common.h. > On 08/09/2022 21:58, Günther Noack wrote: > > diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c > > index 87b28d14a1aa..ddc8c7e57e86 100644 > > --- a/tools/testing/selftests/landlock/fs_test.c > > +++ b/tools/testing/selftests/landlock/fs_test.c > > ... > > +TEST_F_FORK(layout1, truncate) > > +{ > > + const char *const file_rwt = file1_s1d1; > > + const char *const file_rw = file2_s1d1; > > + const char *const file_rt = file1_s1d2; > > + const char *const file_t = file2_s1d2; > > + const char *const file_none = file1_s1d3; > > + const char *const dir_t = dir_s2d1; > > + const char *const file_in_dir_t = file1_s2d1; > > + const char *const dir_w = dir_s3d1; > > + const char *const file_in_dir_w = file1_s3d1; > > + int file_rwt_fd, file_rw_fd; > > These variables are unused now. Good catch, done. > > +TEST_F_FORK(layout1, ftruncate) > > Great! > > > +{ > > + /* > > + * This test opens a new file descriptor at different stages of > > + * Landlock restriction: > > + * > > + * without restriction: ftruncate works > > + * something else but truncate restricted: ftruncate works > > + * truncate restricted and permitted: ftruncate works > > + * truncate restricted and not permitted: ftruncate fails > > + * > > + * Whether this works or not is expected to depend on the time when the > > + * FD was opened, not to depend on the time when ftruncate() was > > + * called. > > + */ > > + const char *const path = file1_s1d1; > > + int fd0, fd1, fd2, fd3; > > You can rename them fd_layer0, fd_layer1… Done. > > + fd0 = open(path, O_WRONLY); > > + EXPECT_EQ(0, test_ftruncate(fd0)); > > + > > + landlock_single_path(_metadata, path, > > + LANDLOCK_ACCESS_FS_READ_FILE | > > + LANDLOCK_ACCESS_FS_WRITE_FILE, > > + LANDLOCK_ACCESS_FS_WRITE_FILE); > > I'd prefer to follow the current way to write rule layers: write all struct > rule at first and then call each enforcement steps. It is a bit more verbose > but easier to understand errors. The list of test_ftruncate checks are > straightforward to follow. Done. > > + fd1 = open(path, O_WRONLY); > > + EXPECT_EQ(0, test_ftruncate(fd0)); > > + EXPECT_EQ(0, test_ftruncate(fd1)); > > + > > + landlock_single_path(_metadata, path, LANDLOCK_ACCESS_FS_TRUNCATE, > > + LANDLOCK_ACCESS_FS_TRUNCATE); > > + > > + fd2 = open(path, O_WRONLY); > > + EXPECT_EQ(0, test_ftruncate(fd0)); > > + EXPECT_EQ(0, test_ftruncate(fd1)); > > + EXPECT_EQ(0, test_ftruncate(fd2)); > > + > > + landlock_single_path(_metadata, path, > > + LANDLOCK_ACCESS_FS_TRUNCATE | > > + LANDLOCK_ACCESS_FS_WRITE_FILE, > > + LANDLOCK_ACCESS_FS_WRITE_FILE); > > + > > + fd3 = open(path, O_WRONLY); > > + EXPECT_EQ(0, test_ftruncate(fd0)); > > + EXPECT_EQ(0, test_ftruncate(fd1)); > > + EXPECT_EQ(0, test_ftruncate(fd2)); > > + EXPECT_EQ(EACCES, test_ftruncate(fd3)); > > + > > + ASSERT_EQ(0, close(fd0)); > > + ASSERT_EQ(0, close(fd1)); > > + ASSERT_EQ(0, close(fd2)); > > + ASSERT_EQ(0, close(fd3)); > > +} > > + > > /* clang-format off */ > > FIXTURE(layout1_bind) {}; > > /* clang-format on */ --