| Message ID | 20230814172816.3907299-3-gnoack@google.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Landlock: IOCTL support | expand |
On Mon, Aug 14, 2023 at 07:28:13PM +0200, Günther Noack wrote: > Exercises Landlock's IOCTL feature: If the LANDLOCK_ACCESS_FS_IOCTL > right is restricted, the use of IOCTL fails with a freshly opened > file. > > Irrespective of the LANDLOCK_ACCESS_FS_IOCTL right, IOCTL continues to > work with a selected set of known harmless IOCTL commands. > > Signed-off-by: Günther Noack <gnoack@google.com> > --- > tools/testing/selftests/landlock/fs_test.c | 96 +++++++++++++++++++++- > 1 file changed, 93 insertions(+), 3 deletions(-) > > diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c > index 09dd1eaac8a9..456bd681091d 100644 > --- a/tools/testing/selftests/landlock/fs_test.c > +++ b/tools/testing/selftests/landlock/fs_test.c > @@ -3329,7 +3329,7 @@ TEST_F_FORK(layout1, truncate_unhandled) > LANDLOCK_ACCESS_FS_WRITE_FILE; > int ruleset_fd; > > - /* Enable Landlock. */ > + /* Enables Landlock. */ > ruleset_fd = create_ruleset(_metadata, handled, rules); > > ASSERT_LE(0, ruleset_fd); > @@ -3412,7 +3412,7 @@ TEST_F_FORK(layout1, truncate) > LANDLOCK_ACCESS_FS_TRUNCATE; > int ruleset_fd; > > - /* Enable Landlock. */ > + /* Enables Landlock. */ > ruleset_fd = create_ruleset(_metadata, handled, rules); > > ASSERT_LE(0, ruleset_fd); > @@ -3639,7 +3639,7 @@ TEST_F_FORK(ftruncate, open_and_ftruncate) > }; > int fd, ruleset_fd; > > - /* Enable Landlock. */ > + /* Enables Landlock. */ > ruleset_fd = create_ruleset(_metadata, variant->handled, rules); > ASSERT_LE(0, ruleset_fd); > enforce_ruleset(_metadata, ruleset_fd); > @@ -3732,6 +3732,96 @@ TEST(memfd_ftruncate) > ASSERT_EQ(0, close(fd)); > } We should also check with O_PATH to make sure the correct error is returned (and not EACCES). > > +/* Invokes the FIOQSIZE ioctl(2) and returns its errno or 0. */ > +static int test_fioqsize_ioctl(int fd) > +{ > + loff_t size; > + > + if (ioctl(fd, FIOQSIZE, &size) < 0) > + return errno; > + return 0; > +} > + > +/* > + * Attempt ioctls on regular files, with file descriptors opened before and > + * after landlocking. > + */ > +TEST_F_FORK(layout1, ioctl) > +{ > + const struct rule rules[] = { > + { > + .path = file1_s1d1, > + .access = LANDLOCK_ACCESS_FS_IOCTL, > + }, > + { > + .path = dir_s2d1, > + .access = LANDLOCK_ACCESS_FS_IOCTL, > + }, > + {}, > + }; > + const __u64 handled = LANDLOCK_ACCESS_FS_IOCTL; > + int ruleset_fd; > + int dir_s1d1_fd, file1_s1d1_fd, dir_s2d1_fd; > + > + /* Enables Landlock. */ > + ruleset_fd = create_ruleset(_metadata, handled, rules); > + ASSERT_LE(0, ruleset_fd); > + enforce_ruleset(_metadata, ruleset_fd); > + ASSERT_EQ(0, close(ruleset_fd)); > + > + dir_s1d1_fd = open(dir_s1d1, O_RDONLY); You can use O_CLOEXEC everywhere. > + ASSERT_LE(0, dir_s1d1_fd); > + file1_s1d1_fd = open(file1_s1d1, O_RDONLY); > + ASSERT_LE(0, file1_s1d1_fd); > + dir_s2d1_fd = open(dir_s2d1, O_RDONLY); > + ASSERT_LE(0, dir_s2d1_fd); > + > + /* > + * Checks that FIOQSIZE works on files where LANDLOCK_ACCESS_FS_IOCTL is > + * permitted. > + */ > + EXPECT_EQ(EACCES, test_fioqsize_ioctl(dir_s1d1_fd)); > + EXPECT_EQ(0, test_fioqsize_ioctl(file1_s1d1_fd)); > + EXPECT_EQ(0, test_fioqsize_ioctl(dir_s2d1_fd)); > + > + /* Closes all file descriptors. */ > + ASSERT_EQ(0, close(dir_s1d1_fd)); > + ASSERT_EQ(0, close(file1_s1d1_fd)); > + ASSERT_EQ(0, close(dir_s2d1_fd)); > +} > + > +TEST_F_FORK(layout1, ioctl_always_allowed) > +{ > + struct landlock_ruleset_attr attr = { const struct landlock_ruleset_attr attr = { > + .handled_access_fs = LANDLOCK_ACCESS_FS_IOCTL, > + }; > + int ruleset_fd, fd; > + int flag = 0; > + int n; const int flag = 0; int ruleset_fd, test_fd, n; > + > + /* Enables Landlock. */ > + ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0); > + ASSERT_LE(0, ruleset_fd); > + enforce_ruleset(_metadata, ruleset_fd); > + ASSERT_EQ(0, close(ruleset_fd)); > + > + fd = open(file1_s1d1, O_RDONLY); > + ASSERT_LE(0, fd); > + > + /* Checks that the restrictable FIOQSIZE is restricted. */ > + EXPECT_EQ(EACCES, test_fioqsize_ioctl(fd)); > + > + /* Checks that unrestrictable commands are unrestricted. */ > + EXPECT_EQ(0, ioctl(fd, FIOCLEX)); > + EXPECT_EQ(0, ioctl(fd, FIONCLEX)); > + EXPECT_EQ(0, ioctl(fd, FIONBIO, &flag)); > + EXPECT_EQ(0, ioctl(fd, FIOASYNC, &flag)); > + EXPECT_EQ(0, ioctl(fd, FIONREAD, &n)); > + EXPECT_EQ(0, n); > + > + ASSERT_EQ(0, close(fd)); > +} > + > /* clang-format off */ > FIXTURE(layout1_bind) {}; > /* clang-format on */ > -- > 2.41.0.694.ge786442a9b-goog >
Hello! On Fri, Aug 18, 2023 at 07:06:07PM +0200, Mickaël Salaün wrote: > On Mon, Aug 14, 2023 at 07:28:13PM +0200, Günther Noack wrote: > > @@ -3639,7 +3639,7 @@ TEST_F_FORK(ftruncate, open_and_ftruncate) > > }; > > int fd, ruleset_fd; > > > > - /* Enable Landlock. */ > > + /* Enables Landlock. */ > > ruleset_fd = create_ruleset(_metadata, variant->handled, rules); > > ASSERT_LE(0, ruleset_fd); > > enforce_ruleset(_metadata, ruleset_fd); > > @@ -3732,6 +3732,96 @@ TEST(memfd_ftruncate) > > ASSERT_EQ(0, close(fd)); > > } > > We should also check with O_PATH to make sure the correct error is > returned (and not EACCES). Is this remark referring to the code before it or after it? My interpretation is that you are asking to test that test_fioqsize_ioctl() will return errnos correctly? Do I understand that correctly? (I think that would be a little bit overdone, IMHO - it's just a test utility of ~10 lines after all, which is below the threshold where it can be verified by staring at it for a bit. :)) > > +/* Invokes the FIOQSIZE ioctl(2) and returns its errno or 0. */ > > +static int test_fioqsize_ioctl(int fd) > > +{ > > + loff_t size; > > + > > + if (ioctl(fd, FIOQSIZE, &size) < 0) > > + return errno; > > + return 0; > > +} > > + dir_s1d1_fd = open(dir_s1d1, O_RDONLY); > > You can use O_CLOEXEC everywhere. Done. > > + ASSERT_LE(0, dir_s1d1_fd); > > + file1_s1d1_fd = open(file1_s1d1, O_RDONLY); > > + ASSERT_LE(0, file1_s1d1_fd); > > + dir_s2d1_fd = open(dir_s2d1, O_RDONLY); > > + ASSERT_LE(0, dir_s2d1_fd); > > + > > + /* > > + * Checks that FIOQSIZE works on files where LANDLOCK_ACCESS_FS_IOCTL is > > + * permitted. > > + */ > > + EXPECT_EQ(EACCES, test_fioqsize_ioctl(dir_s1d1_fd)); > > + EXPECT_EQ(0, test_fioqsize_ioctl(file1_s1d1_fd)); > > + EXPECT_EQ(0, test_fioqsize_ioctl(dir_s2d1_fd)); > > + > > + /* Closes all file descriptors. */ > > + ASSERT_EQ(0, close(dir_s1d1_fd)); > > + ASSERT_EQ(0, close(file1_s1d1_fd)); > > + ASSERT_EQ(0, close(dir_s2d1_fd)); > > +} > > + > > +TEST_F_FORK(layout1, ioctl_always_allowed) > > +{ > > + struct landlock_ruleset_attr attr = { > > const struct landlock_ruleset_attr attr = { Done. I am personally unsure whether "const" is worth it for local variables, but I am happy to abide by whatever the dominant style is. (The kernel style guide doesn't seem to mention it though.) BTW, it's somewhat inconsistent within this file already -- we should maybe clean this up. > > + .handled_access_fs = LANDLOCK_ACCESS_FS_IOCTL, > > + }; > > + int ruleset_fd, fd; > > + int flag = 0; > > + int n; > > const int flag = 0; > int ruleset_fd, test_fd, n; Done. Thanks for the review! —Günther
On Fri, Aug 25, 2023 at 05:51:09PM +0200, Günther Noack wrote: > Hello! > > On Fri, Aug 18, 2023 at 07:06:07PM +0200, Mickaël Salaün wrote: > > On Mon, Aug 14, 2023 at 07:28:13PM +0200, Günther Noack wrote: > > > @@ -3639,7 +3639,7 @@ TEST_F_FORK(ftruncate, open_and_ftruncate) > > > }; > > > int fd, ruleset_fd; > > > > > > - /* Enable Landlock. */ > > > + /* Enables Landlock. */ > > > ruleset_fd = create_ruleset(_metadata, variant->handled, rules); > > > ASSERT_LE(0, ruleset_fd); > > > enforce_ruleset(_metadata, ruleset_fd); > > > @@ -3732,6 +3732,96 @@ TEST(memfd_ftruncate) > > > ASSERT_EQ(0, close(fd)); > > > } > > > > We should also check with O_PATH to make sure the correct error is > > returned (and not EACCES). > > Is this remark referring to the code before it or after it? > > My interpretation is that you are asking to test that test_fioqsize_ioctl() will > return errnos correctly? Do I understand that correctly? (I think that would > be a little bit overdone, IMHO - it's just a test utility of ~10 lines after > all, which is below the threshold where it can be verified by staring at it for > a bit. :)) I was refering to the previous memfd_ftruncate test, which is changed with a next patch. We should check the access rights tied (and checkd) to FD (i.e. truncate and ioctl) opened with O_PATH. > > > > +/* Invokes the FIOQSIZE ioctl(2) and returns its errno or 0. */ > > > +static int test_fioqsize_ioctl(int fd) > > > +{ > > > + loff_t size; > > > + > > > + if (ioctl(fd, FIOQSIZE, &size) < 0) > > > + return errno; > > > + return 0; > > > +} > > > > > > + dir_s1d1_fd = open(dir_s1d1, O_RDONLY); > > > > You can use O_CLOEXEC everywhere. > > Done. > > > > > + ASSERT_LE(0, dir_s1d1_fd); > > > + file1_s1d1_fd = open(file1_s1d1, O_RDONLY); > > > + ASSERT_LE(0, file1_s1d1_fd); > > > + dir_s2d1_fd = open(dir_s2d1, O_RDONLY); > > > + ASSERT_LE(0, dir_s2d1_fd); > > > + > > > + /* > > > + * Checks that FIOQSIZE works on files where LANDLOCK_ACCESS_FS_IOCTL is > > > + * permitted. > > > + */ > > > + EXPECT_EQ(EACCES, test_fioqsize_ioctl(dir_s1d1_fd)); > > > + EXPECT_EQ(0, test_fioqsize_ioctl(file1_s1d1_fd)); > > > + EXPECT_EQ(0, test_fioqsize_ioctl(dir_s2d1_fd)); > > > + > > > + /* Closes all file descriptors. */ > > > + ASSERT_EQ(0, close(dir_s1d1_fd)); > > > + ASSERT_EQ(0, close(file1_s1d1_fd)); > > > + ASSERT_EQ(0, close(dir_s2d1_fd)); > > > +} > > > + > > > +TEST_F_FORK(layout1, ioctl_always_allowed) > > > +{ > > > + struct landlock_ruleset_attr attr = { > > > > const struct landlock_ruleset_attr attr = { > > Done. > > I am personally unsure whether "const" is worth it for local variables, but I am > happy to abide by whatever the dominant style is. (The kernel style guide > doesn't seem to mention it though.) I prefer to constify as much as possible to be notified when a write will be needed for a patch. From a security point of view, it's always good to have as much as possible read-only data, at least in theory (it might not always be enforced in memory). It's also useful as documentation. > > BTW, it's somewhat inconsistent within this file already -- we should maybe > clean this up. I probably missed some, more constification would be good, but not with this patch series. > > > > > + .handled_access_fs = LANDLOCK_ACCESS_FS_IOCTL, > > > + }; > > > + int ruleset_fd, fd; > > > + int flag = 0; > > > + int n; > > > > const int flag = 0; > > int ruleset_fd, test_fd, n; > > Done. > > Thanks for the review! > —Günther > > -- > Sent using Mutt
Hello! On Fri, Aug 25, 2023 at 07:07:01PM +0200, Mickaël Salaün wrote: > On Fri, Aug 25, 2023 at 05:51:09PM +0200, Günther Noack wrote: > > Hello! > > > > On Fri, Aug 18, 2023 at 07:06:07PM +0200, Mickaël Salaün wrote: > > > On Mon, Aug 14, 2023 at 07:28:13PM +0200, Günther Noack wrote: > > > > @@ -3639,7 +3639,7 @@ TEST_F_FORK(ftruncate, open_and_ftruncate) > > > > }; > > > > int fd, ruleset_fd; > > > > > > > > - /* Enable Landlock. */ > > > > + /* Enables Landlock. */ > > > > ruleset_fd = create_ruleset(_metadata, variant->handled, rules); > > > > ASSERT_LE(0, ruleset_fd); > > > > enforce_ruleset(_metadata, ruleset_fd); > > > > @@ -3732,6 +3732,96 @@ TEST(memfd_ftruncate) > > > > ASSERT_EQ(0, close(fd)); > > > > } > > > > > > We should also check with O_PATH to make sure the correct error is > > > returned (and not EACCES). > > > > Is this remark referring to the code before it or after it? > > > > My interpretation is that you are asking to test that test_fioqsize_ioctl() will > > return errnos correctly? Do I understand that correctly? (I think that would > > be a little bit overdone, IMHO - it's just a test utility of ~10 lines after > > all, which is below the threshold where it can be verified by staring at it for > > a bit. :)) > > I was refering to the previous memfd_ftruncate test, which is changed > with a next patch. We should check the access rights tied (and checkd) > to FD (i.e. truncate and ioctl) opened with O_PATH. OK, I added a test that checks ioctl(2) and ftruncate(2) on files that were opened with O_PATH, both before and after enabling Landlock. ftruncate() and ioctl() always give an EBADF error, both before and after enabling Landlock (as described in open(2) in the section about O_PATH). A bit outside of the IOCTL path set scope: I was surprised that it is even possible to successfully open a file with O_PATH, even after Landlock is enabled and restricts all it can in that file hierarchy. This lets you detect that a file exists, even when that file is in a directory whose contents you are otherwise not permitted to list due to Landlock. The logic for that is in the get_required_file_open_access() function. Should we add a "LANDLOCK_ACCESS_FS_PATH_FILE" right, which would work similar to LANDLOCK_ACCESS_FS_READ_FILE and LANDLOCK_ACCESS_FS_WRITE_FILE, so that this can be restricted? —Günther
On Fri, Sep 01, 2023 at 03:35:59PM +0200, Günther Noack wrote: > Hello! > > On Fri, Aug 25, 2023 at 07:07:01PM +0200, Mickaël Salaün wrote: > > On Fri, Aug 25, 2023 at 05:51:09PM +0200, Günther Noack wrote: > > > Hello! > > > > > > On Fri, Aug 18, 2023 at 07:06:07PM +0200, Mickaël Salaün wrote: > > > > On Mon, Aug 14, 2023 at 07:28:13PM +0200, Günther Noack wrote: > > > > > @@ -3639,7 +3639,7 @@ TEST_F_FORK(ftruncate, open_and_ftruncate) > > > > > }; > > > > > int fd, ruleset_fd; > > > > > > > > > > - /* Enable Landlock. */ > > > > > + /* Enables Landlock. */ > > > > > ruleset_fd = create_ruleset(_metadata, variant->handled, rules); > > > > > ASSERT_LE(0, ruleset_fd); > > > > > enforce_ruleset(_metadata, ruleset_fd); > > > > > @@ -3732,6 +3732,96 @@ TEST(memfd_ftruncate) > > > > > ASSERT_EQ(0, close(fd)); > > > > > } > > > > > > > > We should also check with O_PATH to make sure the correct error is > > > > returned (and not EACCES). > > > > > > Is this remark referring to the code before it or after it? > > > > > > My interpretation is that you are asking to test that test_fioqsize_ioctl() will > > > return errnos correctly? Do I understand that correctly? (I think that would > > > be a little bit overdone, IMHO - it's just a test utility of ~10 lines after > > > all, which is below the threshold where it can be verified by staring at it for > > > a bit. :)) > > > > I was refering to the previous memfd_ftruncate test, which is changed > > with a next patch. We should check the access rights tied (and checkd) > > to FD (i.e. truncate and ioctl) opened with O_PATH. > > OK, I added a test that checks ioctl(2) and ftruncate(2) on files that > were opened with O_PATH, both before and after enabling Landlock. > ftruncate() and ioctl() always give an EBADF error, both before and > after enabling Landlock (as described in open(2) in the section about > O_PATH). Good! > > A bit outside of the IOCTL path set scope: > > I was surprised that it is even possible to successfully open a file > with O_PATH, even after Landlock is enabled and restricts all it can > in that file hierarchy. This lets you detect that a file exists, even > when that file is in a directory whose contents you are otherwise not > permitted to list due to Landlock. This is indeed intended and tested with the effective_access test. O_PATH is handled the same way as chdir (and similar path-based syscalls) and always allowed. However, I just realized that the O_PATH case is not explicitly mentioned in the documentation. > > The logic for that is in the get_required_file_open_access() function. > Should we add a "LANDLOCK_ACCESS_FS_PATH_FILE" right, which would work > similar to LANDLOCK_ACCESS_FS_READ_FILE and > LANDLOCK_ACCESS_FS_WRITE_FILE, so that this can be restricted? Having a file descriptor opened with O_PATH should not give any specific access (hence the need to test with IOCTLs). O_PATH is designed to get an explicit reference to the filesystem (without the issues of paths) and use the related FD with *at() syscalls, including landlock_add_rule() with a path_beneath rule. FDs opened with O_PATH should not be an issue by themselves for a sandbox, but the real issue is to discover paths, i.e. the directory's execute access right. This is why I think it would make more sense to have something like a LANDLOCK_ACCESS_FS_WALK right instead. This would enable to definitely deny any access to a file hierarchy. For chdir-like syscalls, we could rely on path_permission(), but for a more holistic approach we need to properly handle the execute permission on directories. See Christian's comment on chdir/path_permission and the limit of what an LSM can infer from paths: https://lore.kernel.org/r/20230530-mietfrei-zynisch-8b63a8566f66@brauner We could rely on the last walked (leaf) dentry to allow or deny such walk, but that would still enable malicious processes to infer path because of intermediate walk that may return ENOENT. We could also return ENOENT instead of EACCES, but this would not handle the case of other access controls returning EACCES. With this in mind, I think the better solution is to properly check each intermediate directory during a path walk. This is not currently possible with path-based LSM hooks but I think that we could add a new LSM hook in filename_lookup(), close to the audit_inode() call, to get the necessary information about the currently walked directory. Simply using this new hook with Landlock would add a significant performance impact because of the way Landlock identifies paths (i.e. walk back). An interesting approach to efficiently check Landlock access right would be to store the collected access rights of a path in a cache, and use it when checking a "child" of this path. According to task_struct, there is only one path walk at a time per thread, which would enable us to have only one cached path per task and opportunistically use it in Landlock's is_access_to_paths_allowed() as a backstop to end the walk. With this trick, I think in most cases no walk back would be required, which would be great for performance. The main challenge would be to efficiently handle the ".." directories. See an initial approach of caching for Landlock: https://lore.kernel.org/r/20210630224856.1313928-1-mic@digikod.net Being able to control path walks would be a way to use (most?) inode-based LSM hooks for Landlock, especially the security_inode_permission(). Indeed. we could then tie an inode to a path (resolution) thanks to the cache (and potentially other caches according to the number of checked inodes). This would be great for new access rights such as LANDLOCK_ACCESS_FS_{READ,WRITE}_METADATA. Xiu, that would be a good opportunity to continue your work, and probably without waiting for IMA to be converted to a proper LSM. What do you think?
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 09dd1eaac8a9..456bd681091d 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -3329,7 +3329,7 @@ TEST_F_FORK(layout1, truncate_unhandled) LANDLOCK_ACCESS_FS_WRITE_FILE; int ruleset_fd; - /* Enable Landlock. */ + /* Enables Landlock. */ ruleset_fd = create_ruleset(_metadata, handled, rules); ASSERT_LE(0, ruleset_fd); @@ -3412,7 +3412,7 @@ TEST_F_FORK(layout1, truncate) LANDLOCK_ACCESS_FS_TRUNCATE; int ruleset_fd; - /* Enable Landlock. */ + /* Enables Landlock. */ ruleset_fd = create_ruleset(_metadata, handled, rules); ASSERT_LE(0, ruleset_fd); @@ -3639,7 +3639,7 @@ TEST_F_FORK(ftruncate, open_and_ftruncate) }; int fd, ruleset_fd; - /* Enable Landlock. */ + /* Enables Landlock. */ ruleset_fd = create_ruleset(_metadata, variant->handled, rules); ASSERT_LE(0, ruleset_fd); enforce_ruleset(_metadata, ruleset_fd); @@ -3732,6 +3732,96 @@ TEST(memfd_ftruncate) ASSERT_EQ(0, close(fd)); } +/* Invokes the FIOQSIZE ioctl(2) and returns its errno or 0. */ +static int test_fioqsize_ioctl(int fd) +{ + loff_t size; + + if (ioctl(fd, FIOQSIZE, &size) < 0) + return errno; + return 0; +} + +/* + * Attempt ioctls on regular files, with file descriptors opened before and + * after landlocking. + */ +TEST_F_FORK(layout1, ioctl) +{ + const struct rule rules[] = { + { + .path = file1_s1d1, + .access = LANDLOCK_ACCESS_FS_IOCTL, + }, + { + .path = dir_s2d1, + .access = LANDLOCK_ACCESS_FS_IOCTL, + }, + {}, + }; + const __u64 handled = LANDLOCK_ACCESS_FS_IOCTL; + int ruleset_fd; + int dir_s1d1_fd, file1_s1d1_fd, dir_s2d1_fd; + + /* Enables Landlock. */ + ruleset_fd = create_ruleset(_metadata, handled, rules); + ASSERT_LE(0, ruleset_fd); + enforce_ruleset(_metadata, ruleset_fd); + ASSERT_EQ(0, close(ruleset_fd)); + + dir_s1d1_fd = open(dir_s1d1, O_RDONLY); + ASSERT_LE(0, dir_s1d1_fd); + file1_s1d1_fd = open(file1_s1d1, O_RDONLY); + ASSERT_LE(0, file1_s1d1_fd); + dir_s2d1_fd = open(dir_s2d1, O_RDONLY); + ASSERT_LE(0, dir_s2d1_fd); + + /* + * Checks that FIOQSIZE works on files where LANDLOCK_ACCESS_FS_IOCTL is + * permitted. + */ + EXPECT_EQ(EACCES, test_fioqsize_ioctl(dir_s1d1_fd)); + EXPECT_EQ(0, test_fioqsize_ioctl(file1_s1d1_fd)); + EXPECT_EQ(0, test_fioqsize_ioctl(dir_s2d1_fd)); + + /* Closes all file descriptors. */ + ASSERT_EQ(0, close(dir_s1d1_fd)); + ASSERT_EQ(0, close(file1_s1d1_fd)); + ASSERT_EQ(0, close(dir_s2d1_fd)); +} + +TEST_F_FORK(layout1, ioctl_always_allowed) +{ + struct landlock_ruleset_attr attr = { + .handled_access_fs = LANDLOCK_ACCESS_FS_IOCTL, + }; + int ruleset_fd, fd; + int flag = 0; + int n; + + /* Enables Landlock. */ + ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0); + ASSERT_LE(0, ruleset_fd); + enforce_ruleset(_metadata, ruleset_fd); + ASSERT_EQ(0, close(ruleset_fd)); + + fd = open(file1_s1d1, O_RDONLY); + ASSERT_LE(0, fd); + + /* Checks that the restrictable FIOQSIZE is restricted. */ + EXPECT_EQ(EACCES, test_fioqsize_ioctl(fd)); + + /* Checks that unrestrictable commands are unrestricted. */ + EXPECT_EQ(0, ioctl(fd, FIOCLEX)); + EXPECT_EQ(0, ioctl(fd, FIONCLEX)); + EXPECT_EQ(0, ioctl(fd, FIONBIO, &flag)); + EXPECT_EQ(0, ioctl(fd, FIOASYNC, &flag)); + EXPECT_EQ(0, ioctl(fd, FIONREAD, &n)); + EXPECT_EQ(0, n); + + ASSERT_EQ(0, close(fd)); +} + /* clang-format off */ FIXTURE(layout1_bind) {}; /* clang-format on */
Exercises Landlock's IOCTL feature: If the LANDLOCK_ACCESS_FS_IOCTL right is restricted, the use of IOCTL fails with a freshly opened file. Irrespective of the LANDLOCK_ACCESS_FS_IOCTL right, IOCTL continues to work with a selected set of known harmless IOCTL commands. Signed-off-by: Günther Noack <gnoack@google.com> --- tools/testing/selftests/landlock/fs_test.c | 96 +++++++++++++++++++++- 1 file changed, 93 insertions(+), 3 deletions(-)