Message ID | 20240725-landlock-v4-5-66f5a1c0c693@suse.com |
---|---|
State | Changes Requested |
Headers | show |
Series | landlock testing suite | expand |
Hi Andrea, Reviewed-by: Petr Vorel <pvorel@suse.cz> Few notes below. ... > +static struct tst_test test = { > + .test_all = run, > + .setup = setup, > + .cleanup = cleanup, > + .min_kver = "6.10", nit: would not be able to check the availablity via landlock ABI == 5? > + .needs_tmpdir = 1, > + .needs_root = 1, > + .forks_child = 1, > + .needs_kconfigs = (const char *[]) { > + "CONFIG_SECURITY_LANDLOCK=y", > + NULL Maybe we don't need this in any landlock test, if we are checking verify_landlock_is_enabled(), right? > + }, > + .bufs = (struct tst_buffers []) { > + {&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)}, > + {&path_beneath_attr, .size = sizeof(struct landlock_path_beneath_attr)}, > + {}, > + }, > + .caps = (struct tst_cap []) { > + TST_CAP(TST_CAP_REQ, CAP_SYS_ADMIN), > + {} > + }, > + .format_device = 1, > + .mount_device = 1, Please remove these two before merge: $ cd metadata && make ... testcases/kernel/syscalls/landlock/landlock06.c: useless tag: format_device testcases/kernel/syscalls/landlock/landlock06.c: useless tag: needs_tmpdir > + .mntpoint = MNTPOINT, > + .all_filesystems = 1, > + .skip_filesystems = (const char *[]) { > + "vfat", > + "exfat", I wonder why landlock is not supported... > + NULL > + }, > +}; Kind regards, Petr
On Fri, Jul 26, 2024 at 03:24:40PM +0200, Petr Vorel wrote: > Hi Andrea, > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > Few notes below. > > ... > > +static struct tst_test test = { > > + .test_all = run, > > + .setup = setup, > > + .cleanup = cleanup, > > + .min_kver = "6.10", > nit: would not be able to check the availablity via landlock ABI == 5? Because Landlock is available since 5.13, I guess min_kver should be set to the same version.
> On Fri, Jul 26, 2024 at 03:24:40PM +0200, Petr Vorel wrote: > > Hi Andrea, > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > Few notes below. > > ... > > > +static struct tst_test test = { > > > + .test_all = run, > > > + .setup = setup, > > > + .cleanup = cleanup, > > > + .min_kver = "6.10", > > nit: would not be able to check the availablity via landlock ABI == 5? > Because Landlock is available since 5.13, I guess min_kver should be set > to the same version. If anybody backport this, kernel version will not match. IMHO it's better to avoid specifying version if we can detect with something else (ABI version in this case). Also avoid kernel config reading if not needed is IMHO better. Kind regards, Petr
> > On Fri, Jul 26, 2024 at 03:24:40PM +0200, Petr Vorel wrote: > > > Hi Andrea, > > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > > Few notes below. > > > ... > > > > +static struct tst_test test = { > > > > + .test_all = run, > > > > + .setup = setup, > > > > + .cleanup = cleanup, > > > > + .min_kver = "6.10", > > > nit: would not be able to check the availablity via landlock ABI == 5? > > Because Landlock is available since 5.13, I guess min_kver should be set > > to the same version. > If anybody backport this, kernel version will not match. IMHO it's better to > avoid specifying version if we can detect with something else (ABI version in > this case). Also avoid kernel config reading if not needed is IMHO better. Although functionality is not backported often to the enterprise kernels (e.g. SLES, RHEL), certain things are backported, e.g. [1]. And that is the reason why LTP even has a way to detect enterprise kernel [2]. Obviously the easiest thing is to avoid kernel version if there is way to detect functionality by different way. Kind regards, Petr [1] https://github.com/linux-test-project/ltp/commit/c354ba291da3e255c135ef78da0c7b8c5556da07 [2] https://github.com/linux-test-project/ltp/blob/master/lib/tst_kvercmp.c#L131 > Kind regards, > Petr
Hi all, ... > > + .mntpoint = MNTPOINT, > > + .all_filesystems = 1, > > + .skip_filesystems = (const char *[]) { > > + "vfat", > > + "exfat", > I wonder why landlock is not supported... It was probably obvious for others: not only ioctl(file_fd, FIONREAD, &sz), but even mkfifo(sandbox/fifo, 0640) fails on exfat and vfat. Kind regards, Petr > > + NULL > > + }, > > +}; > Kind regards, > Petr
diff --git a/runtest/syscalls b/runtest/syscalls index 6522f5bc7..7ebdb41d8 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -701,6 +701,7 @@ landlock02 landlock02 landlock03 landlock03 landlock04 landlock04 landlock05 landlock05 +landlock06 landlock06 lchown01 lchown01 lchown01_16 lchown01_16 diff --git a/testcases/kernel/syscalls/landlock/.gitignore b/testcases/kernel/syscalls/landlock/.gitignore index a7ea6be2e..315ac1dca 100644 --- a/testcases/kernel/syscalls/landlock/.gitignore +++ b/testcases/kernel/syscalls/landlock/.gitignore @@ -4,3 +4,4 @@ landlock02 landlock03 landlock04 landlock05 +landlock06 diff --git a/testcases/kernel/syscalls/landlock/landlock06.c b/testcases/kernel/syscalls/landlock/landlock06.c new file mode 100644 index 000000000..647ebbe48 --- /dev/null +++ b/testcases/kernel/syscalls/landlock/landlock06.c @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> + */ + +/*\ + * [Description] + * + * This test verifies LANDLOCK_ACCESS_FS_IOCTL_DEV access in the + * landlock sandbox by creating a pipe and testing that ioctl() can be executed + * on it. The test is also verifying that some of the I/O operations can be + * always executed no matter the sandbox rules. + */ + +#include "landlock_common.h" +#include <sys/ioctl.h> + +#define MNTPOINT "sandbox" +#define FILENAME MNTPOINT"/fifo" + +static struct landlock_ruleset_attr *ruleset_attr; +static struct landlock_path_beneath_attr *path_beneath_attr; +static int file_fd; +static int dev_fd; + +static void run(void) +{ + if (SAFE_FORK()) + return; + + int flag; + size_t sz = 0; + + TST_EXP_PASS(ioctl(file_fd, FIONREAD, &sz)); + + /* check unrestrictable commands */ + TST_EXP_PASS(ioctl(dev_fd, FIOCLEX)); + TST_EXP_PASS(ioctl(dev_fd, FIONCLEX)); + TST_EXP_PASS(ioctl(dev_fd, FIONBIO, &flag)); + TST_EXP_PASS(ioctl(dev_fd, FIOASYNC, &flag)); + + _exit(0); +} + +static void setup(void) +{ + int ruleset_fd; + + verify_landlock_is_enabled(); + + SAFE_MKFIFO(FILENAME, 0640); + + file_fd = SAFE_OPEN(FILENAME, O_RDONLY | O_NONBLOCK, 0640); + dev_fd = SAFE_OPEN("/dev/zero", O_RDONLY | O_NONBLOCK, 0640); + + tst_res(TINFO, "Applying LANDLOCK_ACCESS_FS_IOCTL_DEV"); + + ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_IOCTL_DEV; + + ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET( + ruleset_attr, sizeof(struct landlock_ruleset_attr), 0); + + apply_landlock_layer( + ruleset_attr, + path_beneath_attr, + MNTPOINT, + LANDLOCK_ACCESS_FS_IOCTL_DEV + ); + + SAFE_CLOSE(ruleset_fd); +} + +static void cleanup(void) +{ + if (dev_fd != -1) + SAFE_CLOSE(dev_fd); + + if (file_fd != -1) + SAFE_CLOSE(file_fd); +} + +static struct tst_test test = { + .test_all = run, + .setup = setup, + .cleanup = cleanup, + .min_kver = "6.10", + .needs_tmpdir = 1, + .needs_root = 1, + .forks_child = 1, + .needs_kconfigs = (const char *[]) { + "CONFIG_SECURITY_LANDLOCK=y", + NULL + }, + .bufs = (struct tst_buffers []) { + {&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)}, + {&path_beneath_attr, .size = sizeof(struct landlock_path_beneath_attr)}, + {}, + }, + .caps = (struct tst_cap []) { + TST_CAP(TST_CAP_REQ, CAP_SYS_ADMIN), + {} + }, + .format_device = 1, + .mount_device = 1, + .mntpoint = MNTPOINT, + .all_filesystems = 1, + .skip_filesystems = (const char *[]) { + "vfat", + "exfat", + NULL + }, +};