[LTP] [PATCH v1] syscalls/mount07: Add new test case for MS_NOSYMFOLLOW

[LTP] [PATCH v1] syscalls/mount07: Add new test case for MS_NOSYMFOLLOW

[Yang Xu]

Fixes:#960
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/lapi/mount.h                       |   4 +
 runtest/syscalls                           |   1 +
 testcases/kernel/syscalls/mount/.gitignore |   1 +
 testcases/kernel/syscalls/mount/mount07.c  | 177 +++++++++++++++++++++
 4 files changed, 183 insertions(+)
 create mode 100644 testcases/kernel/syscalls/mount/mount07.c

diff --git a/include/lapi/mount.h b/include/lapi/mount.h
index 66e1d1319..c1af944fe 100644
--- a/include/lapi/mount.h
+++ b/include/lapi/mount.h
@@ -33,4 +33,8 @@
 # define UMOUNT_NOFOLLOW 8
 #endif
 
+#ifndef MS_NOSYMFOLLOW
+# define MS_NOSYMFOLLOW 256
+#endif
+
 #endif /* LAPI_MOUNT_H__ */
diff --git a/runtest/syscalls b/runtest/syscalls
index ae37a1192..81c30402b 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -804,6 +804,7 @@ mount03 mount03
 mount04 mount04
 mount05 mount05
 mount06 mount06
+mount07 mount07
 
 mount_setattr01 mount_setattr01
 
diff --git a/testcases/kernel/syscalls/mount/.gitignore b/testcases/kernel/syscalls/mount/.gitignore
index f92600d36..80885dbf0 100644
--- a/testcases/kernel/syscalls/mount/.gitignore
+++ b/testcases/kernel/syscalls/mount/.gitignore
@@ -5,3 +5,4 @@
 /mount04
 /mount05
 /mount06
+/mount07
diff --git a/testcases/kernel/syscalls/mount/mount07.c b/testcases/kernel/syscalls/mount/mount07.c
new file mode 100644
index 000000000..eb3fb55a0
--- /dev/null
+++ b/testcases/kernel/syscalls/mount/mount07.c
@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * It is a basic test for MS_NOSYMFOLLOW mount option and is copied
+ * from kernel selftests nosymfollow-test.c.
+ *
+ * It tests to make sure that symlink traversal fails with ELOOP when
+ * 'nosymfollow' is set, but symbolic links can still be created, and
+ * readlink(2) and realpath(3) still work properly. It also verifies
+ * that statfs(2) correctly returns ST_NOSYMFOLLOW.
+ */
+
+#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/mount.h>
+#include <stdbool.h>
+#include "tst_test.h"
+#include "lapi/mount.h"
+
+#ifndef ST_NOSYMFOLLOW
+# define ST_NOSYMFOLLOW 0x2000
+#endif
+
+#define MNTPOINT "mntpoint"
+
+static char test_file[PATH_MAX];
+static char link_file[PATH_MAX];
+static char temp_link_file[PATH_MAX];
+static int flag;
+
+static void setup_symlink(void)
+{
+	int fd;
+
+	fd = SAFE_CREAT(test_file, O_RDWR);
+	SAFE_SYMLINK(test_file, link_file);
+	SAFE_CLOSE(fd);
+	flag = 1;
+}
+
+static void test_link_traversal(bool nosymfollow)
+{
+	if (nosymfollow) {
+		TST_EXP_FAIL2(open(link_file, 0, O_RDWR), ELOOP,
+				"open(%s, 0, O_RDWR)", link_file);
+	} else {
+		TST_EXP_FD(open(link_file, 0, O_RDWR));
+	}
+
+	if (TST_RET > 0)
+		SAFE_CLOSE(TST_RET);
+}
+
+static void test_readlink(void)
+{
+	char buf[4096];
+
+	memset(buf, 0, 4096);
+	TST_EXP_POSITIVE(readlink(link_file, buf, sizeof(buf)),
+			"readlink(%s, buf, %ld)", link_file, sizeof(buf));
+	if (strcmp(buf, test_file) != 0) {
+		tst_res(TFAIL, "readlink strcmp failed, %s, %s",
+				buf, test_file);
+	} else {
+		tst_res(TPASS, "readlink strcmp succeeded");
+	}
+}
+
+static void test_realpath(void)
+{
+	TESTPTR(realpath(link_file, NULL));
+
+	if (!TST_RET_PTR) {
+		tst_res(TFAIL | TERRNO, "realpath failed");
+		return;
+	}
+
+	if (strcmp(TST_RET_PTR, test_file) != 0) {
+		tst_res(TFAIL, "realpath strcmp failed, %s, %s",
+				(char *)TST_RET_PTR, test_file);
+	} else {
+		tst_res(TPASS, "realpath strcmp succeeded");
+	}
+}
+
+static void test_cycle_link(void)
+{
+	TST_EXP_PASS(symlink(test_file, temp_link_file), "symlink(%s, %s)",
+			test_file, temp_link_file);
+	TST_EXP_PASS(unlink(temp_link_file));
+}
+
+static void test_statfs(bool nosymfollow)
+{
+	struct statfs buf;
+
+	SAFE_STATFS(MNTPOINT, &buf);
+	if (buf.f_flags & ST_NOSYMFOLLOW) {
+		tst_res(nosymfollow ? TPASS : TFAIL, "ST_NOSYMFOLLOW set on %s",
+				MNTPOINT);
+	} else {
+		tst_res(nosymfollow ? TFAIL : TPASS, "ST_NOSYMFOLLOW not set on %s",
+				MNTPOINT);
+	}
+}
+
+static void setup(void)
+{
+	char *tmpdir = tst_get_tmpdir();
+
+	snprintf(test_file, PATH_MAX, "%s/%s/test_file", tst_get_tmpdir(),
+			MNTPOINT);
+	snprintf(link_file, PATH_MAX, "%s/%s/link_file", tst_get_tmpdir(),
+			MNTPOINT);
+	snprintf(temp_link_file, PATH_MAX, "%s/%s/temp_link_file",
+			tst_get_tmpdir(), MNTPOINT);
+	free(tmpdir);
+}
+
+static void cleanup(void)
+{
+	if (tst_is_mounted(MNTPOINT))
+		SAFE_UMOUNT(MNTPOINT);
+}
+
+static void run_tests(bool nosymfollow)
+{
+	test_link_traversal(nosymfollow);
+	test_readlink();
+	test_realpath();
+	test_cycle_link();
+	test_statfs(nosymfollow);
+}
+
+static void run(void)
+{
+	tst_res(TINFO, "Testing behaviour when not setting MS_NOSYMFOLLOW");
+
+	TST_EXP_PASS_SILENT(mount(tst_device->dev, MNTPOINT, tst_device->fs_type,
+		0, NULL));
+	if (!flag || !strcmp(tst_device->fs_type, "tmpfs"))
+		setup_symlink();
+	run_tests(false);
+
+	tst_res(TINFO, "Testing behaviour when setting MS_NOSYMFOLLOW");
+	TST_EXP_PASS_SILENT(mount(tst_device->dev, MNTPOINT, tst_device->fs_type,
+		MS_REMOUNT | MS_NOSYMFOLLOW, NULL));
+	run_tests(true);
+
+	SAFE_UMOUNT(MNTPOINT);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+	.needs_root = 1,
+	.min_kver = "5.10",
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.skip_filesystems = (const char *const []){
+		"exfat",
+		"vfat",
+		"ntfs",
+		NULL
+	},
+};
-- 
2.31.1


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

Re: [LTP] [PATCH v1] syscalls/mount07: Add new test case for MS_NOSYMFOLLOW

[Petr Vorel]

Hi Xu,

> +static void test_statfs(bool nosymfollow)
> +{
> +	struct statfs buf;
> +
> +	SAFE_STATFS(MNTPOINT, &buf);
> +	if (buf.f_flags & ST_NOSYMFOLLOW) {
> +		tst_res(nosymfollow ? TPASS : TFAIL, "ST_NOSYMFOLLOW set on %s",
> +				MNTPOINT);
> +	} else {
> +		tst_res(nosymfollow ? TFAIL : TPASS, "ST_NOSYMFOLLOW not set on %s",
> +				MNTPOINT);
> +	}
> +}

I was thinking about this:

static void test_statfs(bool nosymfollow)
{
	struct statfs buf;
	bool st_nosymfollow;
	int res;

	SAFE_STATFS(MNTPOINT, &buf);
	st_nosymfollow = buf.f_flags & ST_NOSYMFOLLOW;
	res = !!st_nosymfollow == !!nosymfollow ? TPASS : TFAIL;

	tst_res(res, "ST_NOSYMFOLLOW %sset on %s", st_nosymfollow ? "" : "not ",
		MNTPOINT);
}
but your version is much readable, thus forget the above.

LGTM, thanks!
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

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

Re: [LTP] [PATCH v1] syscalls/mount07: Add new test case for MS_NOSYMFOLLOW

[xuyang2018.jy]

Hi Petr

> Hi Xu,
> 
>> +static void test_statfs(bool nosymfollow)
>> +{
>> +	struct statfs buf;
>> +
>> +	SAFE_STATFS(MNTPOINT, &buf);
>> +	if (buf.f_flags & ST_NOSYMFOLLOW) {
>> +		tst_res(nosymfollow ? TPASS : TFAIL, "ST_NOSYMFOLLOW set on %s",
>> +				MNTPOINT);
>> +	} else {
>> +		tst_res(nosymfollow ? TFAIL : TPASS, "ST_NOSYMFOLLOW not set on %s",
>> +				MNTPOINT);
>> +	}
>> +}
> 
> I was thinking about this:
> 
> static void test_statfs(bool nosymfollow)
> {
> 	struct statfs buf;
> 	bool st_nosymfollow;
> 	int res;
> 
> 	SAFE_STATFS(MNTPOINT, &buf);
> 	st_nosymfollow = buf.f_flags & ST_NOSYMFOLLOW;
> 	res = !!st_nosymfollow == !!nosymfollow ? TPASS : TFAIL;
> 
> 	tst_res(res, "ST_NOSYMFOLLOW %sset on %s", st_nosymfollow ? "" : "not ",
> 		MNTPOINT);
> }
> but your version is much readable, thus forget the above.
> 
> LGTM, thanks!
> Reviewed-by: Petr Vorel <pvorel@suse.cz>

Thanks for your review, merged!

Best Regards
Yang Xu
> 
> Kind regards,
> Petr

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