| Message ID | 20240530-ioctl_ficlone-v1-3-fa96f07d0fca@suse.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add ioctl_ficlone testing suite | expand |
Hi! > inotify_init1_01 inotify_init1_01 > inotify_init1_02 inotify_init1_02 > diff --git a/testcases/kernel/syscalls/ioctl/.gitignore b/testcases/kernel/syscalls/ioctl/.gitignore > index 3d25fdfb2..d0b470714 100644 > --- a/testcases/kernel/syscalls/ioctl/.gitignore > +++ b/testcases/kernel/syscalls/ioctl/.gitignore > @@ -24,3 +24,4 @@ > /ioctl_sg01 > /ioctl_ficlone01 > /ioctl_ficlone02 > +/ioctl_ficlone03 > diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c > new file mode 100644 > index 000000000..c6f9806a3 > --- /dev/null > +++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c > @@ -0,0 +1,96 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com > + */ > + > +/*\ > + * [Description] > + * > + * This test verifies that ioctl() FICLONE feature correctly raises exceptions > + * when it's supposed to. > + */ > + > +#include "tst_test.h" > +#include "lapi/fs.h" > + > +#define MNTPOINT "mnt" > + > +static int invalid_fd = -1; > +static int rw_file = -1; > +static int ro_file = -1; > +static int wo_file = -1; > +static int dir_fd = -1; > +static int immut_fd = -1; > +static int mnt_file = -1; > + > +static struct tcase { > + int *src_fd; > + int *dst_fd; > + int errno_exp; > + char *msg; > +} tcases[] = { > + {&invalid_fd, &rw_file, EBADF, "invalid source"}, > + {&rw_file, &invalid_fd, EBADF, "invalid destination"}, Can we move these invalid_fd tests into a separate test and use tst_fd to loop over all kinds of invalid input and output file descriptors?
Hi, On 5/30/24 12:52, Cyril Hrubis wrote: > Hi! >> inotify_init1_01 inotify_init1_01 >> inotify_init1_02 inotify_init1_02 >> diff --git a/testcases/kernel/syscalls/ioctl/.gitignore b/testcases/kernel/syscalls/ioctl/.gitignore >> index 3d25fdfb2..d0b470714 100644 >> --- a/testcases/kernel/syscalls/ioctl/.gitignore >> +++ b/testcases/kernel/syscalls/ioctl/.gitignore >> @@ -24,3 +24,4 @@ >> /ioctl_sg01 >> /ioctl_ficlone01 >> /ioctl_ficlone02 >> +/ioctl_ficlone03 >> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c >> new file mode 100644 >> index 000000000..c6f9806a3 >> --- /dev/null >> +++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c >> @@ -0,0 +1,96 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com >> + */ >> + >> +/*\ >> + * [Description] >> + * >> + * This test verifies that ioctl() FICLONE feature correctly raises exceptions >> + * when it's supposed to. >> + */ >> + >> +#include "tst_test.h" >> +#include "lapi/fs.h" >> + >> +#define MNTPOINT "mnt" >> + >> +static int invalid_fd = -1; >> +static int rw_file = -1; >> +static int ro_file = -1; >> +static int wo_file = -1; >> +static int dir_fd = -1; >> +static int immut_fd = -1; >> +static int mnt_file = -1; >> + >> +static struct tcase { >> + int *src_fd; >> + int *dst_fd; >> + int errno_exp; >> + char *msg; >> +} tcases[] = { >> + {&invalid_fd, &rw_file, EBADF, "invalid source"}, >> + {&rw_file, &invalid_fd, EBADF, "invalid destination"}, > Can we move these invalid_fd tests into a separate test and use tst_fd > to loop over all kinds of invalid input and output file descriptors? > I don't know if it's worth to loop over all possible cases. Each one of them has a specific test case. Also, invalid_fd test cases would look so simple that it makes more sense to keep it here. Andrea
Hi! > > Can we move these invalid_fd tests into a separate test and use tst_fd > > to loop over all kinds of invalid input and output file descriptors? > > > I don't know if it's worth to loop over all possible cases. Each one of > them has a specific test case. This was actually requested by kernel developers, to feed the syscalls with all kind of unexpected file descriptors. > Also, invalid_fd test cases would look so simple that it makes more > sense to keep it here. Fair enough, we can keep these two here, but we need a tst_fd test as well.
Hi! On 5/31/24 10:03, Cyril Hrubis wrote: > Hi! >>> Can we move these invalid_fd tests into a separate test and use tst_fd >>> to loop over all kinds of invalid input and output file descriptors? >>> >> I don't know if it's worth to loop over all possible cases. Each one of >> them has a specific test case. > This was actually requested by kernel developers, to feed the syscalls > with all kind of unexpected file descriptors. This is what fuzz testing is doing already, isn't it? >> Also, invalid_fd test cases would look so simple that it makes more >> sense to keep it here. > Fair enough, we can keep these two here, but we need a tst_fd test as > well. > Andrea
On 5/31/24 10:03, Cyril Hrubis wrote: > Hi! >>> Can we move these invalid_fd tests into a separate test and use tst_fd >>> to loop over all kinds of invalid input and output file descriptors? >>> >> I don't know if it's worth to loop over all possible cases. Each one of >> them has a specific test case. > This was actually requested by kernel developers, to feed the syscalls > with all kind of unexpected file descriptors. > >> Also, invalid_fd test cases would look so simple that it makes more >> sense to keep it here. > Fair enough, we can keep these two here, but we need a tst_fd test as > well. > I'm trying to create a test using tst_fs but it seems quite unpredictable. For example, if the given paths are located on a different mount, EXDEV is raised. But we don't have any control on it and it can happen with literally anything (signalfd, memfd, devices, etc) on the SUT we are testing. So that ends up skipping 90% of the provided file descriptors. I don't know if I really want to follow this direction. Andrea
diff --git a/runtest/syscalls b/runtest/syscalls index b049530d3..452a7a071 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -593,6 +593,7 @@ ioctl_sg01 ioctl_sg01 ioctl_ficlone01 ioctl_ficlone01 ioctl_ficlone02 ioctl_ficlone02 +ioctl_ficlone03 ioctl_ficlone03 inotify_init1_01 inotify_init1_01 inotify_init1_02 inotify_init1_02 diff --git a/testcases/kernel/syscalls/ioctl/.gitignore b/testcases/kernel/syscalls/ioctl/.gitignore index 3d25fdfb2..d0b470714 100644 --- a/testcases/kernel/syscalls/ioctl/.gitignore +++ b/testcases/kernel/syscalls/ioctl/.gitignore @@ -24,3 +24,4 @@ /ioctl_sg01 /ioctl_ficlone01 /ioctl_ficlone02 +/ioctl_ficlone03 diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c new file mode 100644 index 000000000..c6f9806a3 --- /dev/null +++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com + */ + +/*\ + * [Description] + * + * This test verifies that ioctl() FICLONE feature correctly raises exceptions + * when it's supposed to. + */ + +#include "tst_test.h" +#include "lapi/fs.h" + +#define MNTPOINT "mnt" + +static int invalid_fd = -1; +static int rw_file = -1; +static int ro_file = -1; +static int wo_file = -1; +static int dir_fd = -1; +static int immut_fd = -1; +static int mnt_file = -1; + +static struct tcase { + int *src_fd; + int *dst_fd; + int errno_exp; + char *msg; +} tcases[] = { + {&invalid_fd, &rw_file, EBADF, "invalid source"}, + {&rw_file, &invalid_fd, EBADF, "invalid destination"}, + {&rw_file, &ro_file, EBADF, "read-only destination"}, + {&wo_file, &rw_file, EBADF, "write-only source"}, + {&rw_file, &dir_fd, EISDIR, "source is a directory"}, + {&dir_fd, &rw_file, EISDIR, "destination is a directory"}, + {&rw_file, &immut_fd, EPERM, "destination is immutable"}, + {&rw_file, &mnt_file, EXDEV, "destination is on a different mount"}, + {&mnt_file, &rw_file, EXDEV, "source is on a different mount"}, +}; + +static void run(unsigned int n) +{ + struct tcase *tc = &tcases[n]; + + TST_EXP_FAIL(ioctl(*tc->dst_fd, FICLONE, *tc->src_fd), + tc->errno_exp, + "%s", tc->msg); +} + +static void setup(void) +{ + int attr; + + rw_file = SAFE_OPEN("ok_only", O_CREAT | O_RDWR, 0640); + ro_file = SAFE_OPEN("rd_only", O_CREAT | O_RDONLY, 0640); + wo_file = SAFE_OPEN("rw_only", O_CREAT | O_WRONLY, 0640); + + SAFE_MKDIR("mydir", 0640); + dir_fd = SAFE_OPEN("mydir", O_DIRECTORY, 0640); + + attr = FS_IMMUTABLE_FL; + immut_fd = SAFE_OPEN("immutable", O_CREAT | O_RDWR, 0640); + SAFE_IOCTL(immut_fd, FS_IOC_SETFLAGS, &attr); + + mnt_file = SAFE_OPEN(MNTPOINT"/file", O_CREAT | O_RDWR, 0640); +} + +static void cleanup(void) +{ + int attr; + + SAFE_IOCTL(immut_fd, FS_IOC_GETFLAGS, &attr); + attr &= ~FS_IMMUTABLE_FL; + SAFE_IOCTL(immut_fd, FS_IOC_SETFLAGS, &attr); + SAFE_CLOSE(immut_fd); + + SAFE_CLOSE(rw_file); + SAFE_CLOSE(ro_file); + SAFE_CLOSE(wo_file); + SAFE_CLOSE(dir_fd); + SAFE_CLOSE(mnt_file); +} + +static struct tst_test test = { + .test = run, + .tcnt = ARRAY_SIZE(tcases), + .setup = setup, + .cleanup = cleanup, + .min_kver = "4.5", + .needs_root = 1, + .mount_device = 1, + .mntpoint = MNTPOINT, + .dev_fs_type = "btrfs", +};