| Message ID | 20240510002331.31431-1-wegao@suse.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1] chmod02.c: Block mode changes of symlinks | expand |
Hi Wei, ... > testcases/kernel/syscalls/chmod/chmod02.c | 68 ++++++++++++++++++++++ nit: original chmod02.c was removed in eab36ea01, now you are using this filename again. I don't remember if there is any consensus about reusing test name (maybe it's ok), but I would prefer to create new file (i.e. chmod08.c), because reusing names can lead to confusion (people may not notice when looking into git history that these 2 tests are completely unrelated. If nothing, different GPL version can be taken by mistake (e.g. original chmod02.c used GPL 2 only license). ... > +/*\ > + * [Description] > + * > + * Test for kernel commit 5d1f903f75a80daa4dfb3d84e114ec8ecbf29956 > + * "attr: block mode changes of symlinks". nit: 5d1f903f75a8 ("attr: block mode changes of symlinks") > + * nit: please remove this blank line above. > + */ > + > +#include "lapi/fcntl.h" > +#include "tst_test.h" > + > +#define MODE 0644 > +#define TESTFILE "testfile" > +#define TESTFILE_SYMLINK "testfile_symlink" > + > +static void run(void) > +{ > + struct stat stat_file, stat_sym; > + int mode = 0; > + char fd_path[100]; > + > + int fd = SAFE_OPEN(TESTFILE_SYMLINK, O_PATH | O_NOFOLLOW); > + > + sprintf(fd_path, "/proc/self/fd/%d", fd); > + > + TST_EXP_FAIL(chmod(fd_path, mode), ENOTSUP, "chmod(%s, %04o)", > + TESTFILE_SYMLINK, mode); > + > + SAFE_STAT(TESTFILE, &stat_file); > + SAFE_LSTAT(TESTFILE_SYMLINK, &stat_sym); > + > + stat_file.st_mode &= ~S_IFREG; > + stat_sym.st_mode &= ~S_IFLNK; > + > + if (stat_file.st_mode == (unsigned int)mode) { > + tst_res(TFAIL, "stat(%s) mode=%04o", > + TESTFILE, stat_file.st_mode); > + } else { > + tst_res(TPASS, "stat(%s) mode=%04o", > + TESTFILE, stat_file.st_mode); > + } Maybe using TST_EXP_EXPR() to not repeat yourself? > + > + if (stat_sym.st_mode == (unsigned int)mode) { > + tst_res(TFAIL, "stat(%s) mode=%04o", > + TESTFILE_SYMLINK, stat_sym.st_mode); > + } else { > + tst_res(TPASS, "stat(%s) mode=%04o", > + TESTFILE_SYMLINK, stat_sym.st_mode); > + } And here as well? Missing SAFE_CLOSE(fd); causes EMFILE (Too many open files) failure on high -i (e.g. -i 1100). > +} > + > +static void setup(void) > +{ > + SAFE_TOUCH(TESTFILE, MODE, NULL); > + SAFE_SYMLINK(TESTFILE, TESTFILE_SYMLINK); > +} > + > +static struct tst_test test = { > + .setup = setup, > + .test_all = run, > + .needs_tmpdir = 1, > + .min_kver = "6.6", Looking into kernel commit [1] it's in VFS therefore it should be safe to test it on single filesystem which is in TMPDIR (i.e. not using .all_filesystems). But "If this causes any regressions" and "It's a simple patch that can be easily reverted." suggests that we should think twice when not running it with .all_filesystems (if used, exfat and vfat fails with EPERM, but it works on NTFS). Kind regards, Petr [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5d1f903f75a80daa4dfb3d84e114ec8ecbf29956
diff --git a/runtest/syscalls b/runtest/syscalls index fe9ad0895..b8f1d53d5 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -66,6 +66,7 @@ chdir04 chdir04 chmod01 chmod01 chmod01A symlink01 -T chmod01 +chmod02 chmod02 chmod03 chmod03 chmod05 chmod05 chmod06 chmod06 diff --git a/testcases/kernel/syscalls/chmod/.gitignore b/testcases/kernel/syscalls/chmod/.gitignore index 27ddfce16..9cc923e69 100644 --- a/testcases/kernel/syscalls/chmod/.gitignore +++ b/testcases/kernel/syscalls/chmod/.gitignore @@ -1,4 +1,5 @@ /chmod01 +/chmod02 /chmod03 /chmod05 /chmod06 diff --git a/testcases/kernel/syscalls/chmod/chmod02.c b/testcases/kernel/syscalls/chmod/chmod02.c new file mode 100644 index 000000000..412c78e9a --- /dev/null +++ b/testcases/kernel/syscalls/chmod/chmod02.c @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2024 Wei Gao <wegao@suse.com> + */ + +/*\ + * [Description] + * + * Test for kernel commit 5d1f903f75a80daa4dfb3d84e114ec8ecbf29956 + * "attr: block mode changes of symlinks". + * + */ + +#include "lapi/fcntl.h" +#include "tst_test.h" + +#define MODE 0644 +#define TESTFILE "testfile" +#define TESTFILE_SYMLINK "testfile_symlink" + +static void run(void) +{ + struct stat stat_file, stat_sym; + int mode = 0; + char fd_path[100]; + + int fd = SAFE_OPEN(TESTFILE_SYMLINK, O_PATH | O_NOFOLLOW); + + sprintf(fd_path, "/proc/self/fd/%d", fd); + + TST_EXP_FAIL(chmod(fd_path, mode), ENOTSUP, "chmod(%s, %04o)", + TESTFILE_SYMLINK, mode); + + SAFE_STAT(TESTFILE, &stat_file); + SAFE_LSTAT(TESTFILE_SYMLINK, &stat_sym); + + stat_file.st_mode &= ~S_IFREG; + stat_sym.st_mode &= ~S_IFLNK; + + if (stat_file.st_mode == (unsigned int)mode) { + tst_res(TFAIL, "stat(%s) mode=%04o", + TESTFILE, stat_file.st_mode); + } else { + tst_res(TPASS, "stat(%s) mode=%04o", + TESTFILE, stat_file.st_mode); + } + + if (stat_sym.st_mode == (unsigned int)mode) { + tst_res(TFAIL, "stat(%s) mode=%04o", + TESTFILE_SYMLINK, stat_sym.st_mode); + } else { + tst_res(TPASS, "stat(%s) mode=%04o", + TESTFILE_SYMLINK, stat_sym.st_mode); + } +} + +static void setup(void) +{ + SAFE_TOUCH(TESTFILE, MODE, NULL); + SAFE_SYMLINK(TESTFILE, TESTFILE_SYMLINK); +} + +static struct tst_test test = { + .setup = setup, + .test_all = run, + .needs_tmpdir = 1, + .min_kver = "6.6", +};
Signed-off-by: Wei Gao <wegao@suse.com> --- runtest/syscalls | 1 + testcases/kernel/syscalls/chmod/.gitignore | 1 + testcases/kernel/syscalls/chmod/chmod02.c | 68 ++++++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 testcases/kernel/syscalls/chmod/chmod02.c