[LTP] [PATCH v1] chmod02.c: Block mode changes of symlinks

[LTP] [PATCH v1] chmod02.c: Block mode changes of symlinks

[Wei Gao via ltp]

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

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",
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] chmod02.c: Block mode changes of symlinks

[Petr Vorel]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] chmod08.c: Block mode changes of symlinks

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 runtest/syscalls                           |  1 +
 testcases/kernel/syscalls/chmod/.gitignore |  1 +
 testcases/kernel/syscalls/chmod/chmod08.c  | 67 ++++++++++++++++++++++
 3 files changed, 69 insertions(+)
 create mode 100644 testcases/kernel/syscalls/chmod/chmod08.c

diff --git a/runtest/syscalls b/runtest/syscalls
index fe9ad0895..5754d8195 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -70,6 +70,7 @@ chmod03 chmod03
 chmod05 chmod05
 chmod06 chmod06
 chmod07 chmod07
+chmod08 chmod08
 
 chown01 chown01
 chown01_16 chown01_16
diff --git a/testcases/kernel/syscalls/chmod/.gitignore b/testcases/kernel/syscalls/chmod/.gitignore
index 27ddfce16..f295f4dcb 100644
--- a/testcases/kernel/syscalls/chmod/.gitignore
+++ b/testcases/kernel/syscalls/chmod/.gitignore
@@ -3,3 +3,4 @@
 /chmod05
 /chmod06
 /chmod07
+/chmod08
diff --git a/testcases/kernel/syscalls/chmod/chmod08.c b/testcases/kernel/syscalls/chmod/chmod08.c
new file mode 100644
index 000000000..368a1d635
--- /dev/null
+++ b/testcases/kernel/syscalls/chmod/chmod08.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 Wei Gao <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Test for kernel commit
+ * 5d1f903f75a8 ("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;
+
+	TST_EXP_EXPR(stat_file.st_mode != (unsigned int)mode,
+			"stat(%s) mode=%04o", TESTFILE, stat_file.st_mode);
+
+	TST_EXP_EXPR(stat_sym.st_mode != (unsigned int)mode,
+			"stat(%s) mode=%04o", TESTFILE, stat_sym.st_mode);
+
+	SAFE_CLOSE(fd);
+}
+
+static void setup(void)
+{
+	SAFE_TOUCH(TESTFILE, MODE, NULL);
+	SAFE_SYMLINK(TESTFILE, TESTFILE_SYMLINK);
+}
+
+static void cleanup(void)
+{
+	remove(TESTFILE);
+	remove(TESTFILE_SYMLINK);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+	.min_kver = "6.6",
+	.mntpoint = "mntpoint",
+	.all_filesystems = 1
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] chmod08.c: Block mode changes of symlinks

[Andrea Cervesato via ltp]

Hi Wei,

there's already a patch doing that. I wrote it long time ago, but it's 
pending for review. Please take a look if you are interested:
https://patchwork.ozlabs.org/project/ltp/patch/20240220131319.11761-1-andrea.cervesato@suse.de/

Andrea

On 5/24/24 10:56, Wei Gao via ltp wrote:
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>   runtest/syscalls                           |  1 +
>   testcases/kernel/syscalls/chmod/.gitignore |  1 +
>   testcases/kernel/syscalls/chmod/chmod08.c  | 67 ++++++++++++++++++++++
>   3 files changed, 69 insertions(+)
>   create mode 100644 testcases/kernel/syscalls/chmod/chmod08.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index fe9ad0895..5754d8195 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -70,6 +70,7 @@ chmod03 chmod03
>   chmod05 chmod05
>   chmod06 chmod06
>   chmod07 chmod07
> +chmod08 chmod08
>   
>   chown01 chown01
>   chown01_16 chown01_16
> diff --git a/testcases/kernel/syscalls/chmod/.gitignore b/testcases/kernel/syscalls/chmod/.gitignore
> index 27ddfce16..f295f4dcb 100644
> --- a/testcases/kernel/syscalls/chmod/.gitignore
> +++ b/testcases/kernel/syscalls/chmod/.gitignore
> @@ -3,3 +3,4 @@
>   /chmod05
>   /chmod06
>   /chmod07
> +/chmod08
> diff --git a/testcases/kernel/syscalls/chmod/chmod08.c b/testcases/kernel/syscalls/chmod/chmod08.c
> new file mode 100644
> index 000000000..368a1d635
> --- /dev/null
> +++ b/testcases/kernel/syscalls/chmod/chmod08.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2024 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Test for kernel commit
> + * 5d1f903f75a8 ("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;
> +
> +	TST_EXP_EXPR(stat_file.st_mode != (unsigned int)mode,
> +			"stat(%s) mode=%04o", TESTFILE, stat_file.st_mode);
> +
> +	TST_EXP_EXPR(stat_sym.st_mode != (unsigned int)mode,
> +			"stat(%s) mode=%04o", TESTFILE, stat_sym.st_mode);
> +
> +	SAFE_CLOSE(fd);
> +}
> +
> +static void setup(void)
> +{
> +	SAFE_TOUCH(TESTFILE, MODE, NULL);
> +	SAFE_SYMLINK(TESTFILE, TESTFILE_SYMLINK);
> +}
> +
> +static void cleanup(void)
> +{
> +	remove(TESTFILE);
> +	remove(TESTFILE_SYMLINK);
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = run,
> +	.min_kver = "6.6",
> +	.mntpoint = "mntpoint",
> +	.all_filesystems = 1
> +};



-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3] chmod09.c: Block mode changes of symlinks

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 runtest/syscalls                           |  1 +
 testcases/kernel/syscalls/chmod/.gitignore |  1 +
 testcases/kernel/syscalls/chmod/chmod09.c  | 67 ++++++++++++++++++++++
 3 files changed, 69 insertions(+)
 create mode 100644 testcases/kernel/syscalls/chmod/chmod09.c

diff --git a/runtest/syscalls b/runtest/syscalls
index fe9ad0895..c8a7b3ecb 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -70,6 +70,7 @@ chmod03 chmod03
 chmod05 chmod05
 chmod06 chmod06
 chmod07 chmod07
+chmod09 chmod09
 
 chown01 chown01
 chown01_16 chown01_16
diff --git a/testcases/kernel/syscalls/chmod/.gitignore b/testcases/kernel/syscalls/chmod/.gitignore
index 27ddfce16..10249b089 100644
--- a/testcases/kernel/syscalls/chmod/.gitignore
+++ b/testcases/kernel/syscalls/chmod/.gitignore
@@ -3,3 +3,4 @@
 /chmod05
 /chmod06
 /chmod07
+/chmod09
diff --git a/testcases/kernel/syscalls/chmod/chmod09.c b/testcases/kernel/syscalls/chmod/chmod09.c
new file mode 100644
index 000000000..368a1d635
--- /dev/null
+++ b/testcases/kernel/syscalls/chmod/chmod09.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 Wei Gao <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Test for kernel commit
+ * 5d1f903f75a8 ("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;
+
+	TST_EXP_EXPR(stat_file.st_mode != (unsigned int)mode,
+			"stat(%s) mode=%04o", TESTFILE, stat_file.st_mode);
+
+	TST_EXP_EXPR(stat_sym.st_mode != (unsigned int)mode,
+			"stat(%s) mode=%04o", TESTFILE, stat_sym.st_mode);
+
+	SAFE_CLOSE(fd);
+}
+
+static void setup(void)
+{
+	SAFE_TOUCH(TESTFILE, MODE, NULL);
+	SAFE_SYMLINK(TESTFILE, TESTFILE_SYMLINK);
+}
+
+static void cleanup(void)
+{
+	remove(TESTFILE);
+	remove(TESTFILE_SYMLINK);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+	.min_kver = "6.6",
+	.mntpoint = "mntpoint",
+	.all_filesystems = 1
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] chmod08.c: Block mode changes of symlinks

[Cyril Hrubis]

Hi!
> there's already a patch doing that. I wrote it long time ago, but it's 
> pending for review. Please take a look if you are interested:
> https://patchwork.ozlabs.org/project/ltp/patch/20240220131319.11761-1-andrea.cervesato@suse.de/

This is a different test though, it checks that we can't change mode for
the file via the /proc/self/fd/fd* symlinks.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] chmod09.c: Block mode changes of symlinks

[Cyril Hrubis]

Hi!
> +/*\
> + * [Description]
> + *
> + * Test for kernel commit
> + * 5d1f903f75a8 ("attr: block mode changes of symlinks")

I've added this to the tst_test tags as well and pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp