[LTP] [PATCH v2] syscalls/mount_setattr01: Add basic functional test

[LTP] [PATCH v2] syscalls/mount_setattr01: Add basic functional test

[Chen Hanxiao]

From: Chen Hanxiao <chenhx.fnst@fujitsu.com>

The mount_setattr() system call changes the mount properties of
a mount or an entire mount tree. Here we check whether the mount
attributes are set correctly.

Signed-off-by: Dai Shili <daisl.fnst@fujitsu.com>
Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>
---
v2:
  1) fix bugs according to Cyril's comments
  2) just set and test mount attributes, remove attr_clr section.

 include/lapi/fsmount.h                        |  45 +++++++
 runtest/syscalls                              |   2 +
 .../kernel/syscalls/mount_setattr/.gitignore  |   1 +
 .../kernel/syscalls/mount_setattr/Makefile    |   6 +
 .../syscalls/mount_setattr/mount_setattr01.c  | 118 ++++++++++++++++++
 5 files changed, 172 insertions(+)
 create mode 100644 testcases/kernel/syscalls/mount_setattr/.gitignore
 create mode 100644 testcases/kernel/syscalls/mount_setattr/Makefile
 create mode 100644 testcases/kernel/syscalls/mount_setattr/mount_setattr01.c

diff --git a/include/lapi/fsmount.h b/include/lapi/fsmount.h
index fa2530675..d44444309 100644
--- a/include/lapi/fsmount.h
+++ b/include/lapi/fsmount.h
@@ -15,6 +15,42 @@
 #include "lapi/fcntl.h"
 #include "lapi/syscalls.h"
 
+/*
+ * Mount attributes.
+ */
+#ifndef MOUNT_ATTR_RDONLY
+# define MOUNT_ATTR_RDONLY       0x00000001 /* Mount read-only */
+#endif
+#ifndef MOUNT_ATTR_NOSUID
+# define MOUNT_ATTR_NOSUID       0x00000002 /* Ignore suid and sgid bits */
+#endif
+#ifndef MOUNT_ATTR_NODEV
+# define MOUNT_ATTR_NODEV        0x00000004 /* Disallow access to device special files */
+#endif
+#ifndef MOUNT_ATTR_NOEXEC
+# define MOUNT_ATTR_NOEXEC       0x00000008 /* Disallow program execution */
+#endif
+#ifndef MOUNT_ATTR_NODIRATIME
+# define MOUNT_ATTR_NODIRATIME   0x00000080 /* Do not update directory access times */
+#endif
+#ifndef MOUNT_ATTR_NOSYMFOLLOW
+# define MOUNT_ATTR_NOSYMFOLLOW  0x00200000 /* Do not follow symlinks */
+#endif
+
+#ifndef ST_NOSYMFOLLOW
+# define ST_NOSYMFOLLOW 0x2000 /* do not follow symlinks */
+#endif
+
+/*
+ * mount_setattr()
+ */
+struct mount_attr {
+	uint64_t attr_set;
+	uint64_t attr_clr;
+	uint64_t propagation;
+	uint64_t userns_fd;
+};
+
 #ifndef HAVE_FSOPEN
 static inline int fsopen(const char *fsname, unsigned int flags)
 {
@@ -61,6 +97,15 @@ static inline int open_tree(int dirfd, const char *pathname, unsigned int flags)
 }
 #endif /* HAVE_OPEN_TREE */
 
+#ifndef HAVE_MOUNT_SETATTR
+static inline int mount_setattr(int dirfd, const char *from_pathname, unsigned int flags,
+				struct mount_attr *attr, size_t size)
+{
+	return tst_syscall(__NR_mount_setattr, dirfd, from_pathname, flags,
+			   attr, size);
+}
+#endif /* HAVE_MOUNT_SETATTR */
+
 /*
  * New headers added in kernel after 5.2 release, create them for old userspace.
 */
diff --git a/runtest/syscalls b/runtest/syscalls
index c30383ee5..1d93f419c 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -801,6 +801,8 @@ mount04 mount04
 mount05 mount05
 mount06 mount06
 
+mount_setattr01 mount_setattr01
+
 move_mount01 move_mount01
 move_mount02 move_mount02
 
diff --git a/testcases/kernel/syscalls/mount_setattr/.gitignore b/testcases/kernel/syscalls/mount_setattr/.gitignore
new file mode 100644
index 000000000..52a77b9ca
--- /dev/null
+++ b/testcases/kernel/syscalls/mount_setattr/.gitignore
@@ -0,0 +1 @@
+/mount_setattr01
diff --git a/testcases/kernel/syscalls/mount_setattr/Makefile b/testcases/kernel/syscalls/mount_setattr/Makefile
new file mode 100644
index 000000000..5ea7d67db
--- /dev/null
+++ b/testcases/kernel/syscalls/mount_setattr/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c b/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
new file mode 100644
index 000000000..d63db46fa
--- /dev/null
+++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
+ * Author: Dai Shili <daisl.fnst@fujitsu.com>
+ * Author: Chen Hanxiao <chenhx.fnst@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Basic mount_setattr() test.
+ * Test whether the basic mount attributes are set correctly.
+ *
+ * Verify some MOUNT_SETATTR(2) attributes:
+ * 1) MOUNT_ATTR_RDONLY - makes the mount read-only
+ * 2) MOUNT_ATTR_NOSUID - causes the mount not to honor the
+ *    set-user-ID and set-group-ID mode bits and file capabilities
+ *    when executing programs.
+ * 3) MOUNT_ATTR_NODEV - prevents access to devices on this mount
+ * 4) MOUNT_ATTR_NOEXEC - prevents executing programs on this mount
+ * 5) MOUNT_ATTR_NOSYMFOLLOW - prevents following symbolic links
+ *    on this mount
+ * 6) MOUNT_ATTR_NODIRATIME - prevents updating access time for
+ *    directories on this mount
+ * Minimum Linux version required is v5.12.
+ */
+
+#define _GNU_SOURCE
+
+#include <sys/statvfs.h>
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+#include "lapi/stat.h"
+
+#define MNTPOINT        "mntpoint"
+#define OT_MNTPOINT     "ot_mntpoint"
+#define TCASE_ENTRY(attrs, exp_attrs)   \
+	{                                \
+		.name = #attrs,                 \
+		.mount_attrs = attrs,           \
+		.expect_attrs = exp_attrs       \
+	}
+
+static int dir_created;
+
+static struct tcase {
+	char *name;
+	unsigned int mount_attrs;
+	unsigned int expect_attrs;
+} tcases[] = {
+	TCASE_ENTRY(MOUNT_ATTR_RDONLY, ST_RDONLY),
+	TCASE_ENTRY(MOUNT_ATTR_NOSUID, ST_NOSUID),
+	TCASE_ENTRY(MOUNT_ATTR_NODEV, ST_NODEV),
+	TCASE_ENTRY(MOUNT_ATTR_NOEXEC, ST_NOEXEC),
+	TCASE_ENTRY(MOUNT_ATTR_NOSYMFOLLOW, ST_NOSYMFOLLOW),
+	TCASE_ENTRY(MOUNT_ATTR_NODIRATIME, ST_NODIRATIME),
+};
+
+static void cleanup(void)
+{
+	if (dir_created)
+		SAFE_RMDIR(OT_MNTPOINT);
+}
+
+static void setup(void)
+{
+	fsopen_supported_by_kernel();
+	SAFE_MKDIR(OT_MNTPOINT, 0777);
+	dir_created = 1;
+}
+
+static void run(unsigned int n)
+{
+	int otfd;
+	struct tcase *tc = &tcases[n];
+	struct mount_attr attr = {
+		.attr_set = tc->mount_attrs,
+	};
+	struct statvfs buf;
+
+	TST_EXP_FD_SILENT(open_tree(AT_FDCWD, MNTPOINT, AT_EMPTY_PATH |
+		AT_SYMLINK_NOFOLLOW | OPEN_TREE_CLOEXEC | OPEN_TREE_CLONE));
+	otfd = (int)TST_RET;
+
+	TST_EXP_PASS_SILENT(mount_setattr(otfd, "", AT_EMPTY_PATH, &attr, sizeof(attr)),
+		"%s set", tc->name);
+
+	TEST(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH));
+
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TTERRNO, "move_mount() failed");
+		return;
+	}
+
+	SAFE_CLOSE(otfd);
+
+	TST_EXP_PASS_SILENT(statvfs(OT_MNTPOINT, &buf), "statvfs sucess");
+
+	if ((buf.f_flag & tc->expect_attrs) == 0)
+		tst_res(TFAIL, "%s is not actually set as expected", tc->name);
+	else
+		tst_res(TPASS, "%s is actually set as expected", tc->name);
+
+	if (tst_is_mounted_at_tmpdir(OT_MNTPOINT))
+		SAFE_UMOUNT(OT_MNTPOINT);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.mount_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.skip_filesystems = (const char *const []){"fuse", NULL},
+};
-- 
2.35.1




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

Re: [LTP] [PATCH v2] syscalls/mount_setattr01: Add basic functional test

[Petr Vorel]

Hi Chen, Dai,

> From: Chen Hanxiao <chenhx.fnst@fujitsu.com>

> diff --git a/include/lapi/fsmount.h b/include/lapi/fsmount.h
...
> +/*
> + * mount_setattr()
> + */
> +struct mount_attr {
> +	uint64_t attr_set;
> +	uint64_t attr_clr;
> +	uint64_t propagation;
> +	uint64_t userns_fd;
> +};
Interesting enough: in kernel tools/testing/selftests/mount_setattr/mount_setattr_test.c
defines it as __u64 (IMHO should be really uint64_t as that test is userspace as
Cyril pointed out) but real kernel code in fs/namespace.c happily uses "unsigned
int" :).

...
> diff --git a/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c b/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
> new file mode 100644
> index 000000000..d63db46fa
> --- /dev/null
> +++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
> + * Author: Dai Shili <daisl.fnst@fujitsu.com>
> + * Author: Chen Hanxiao <chenhx.fnst@fujitsu.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Basic mount_setattr() test.
> + * Test whether the basic mount attributes are set correctly.
> + *
> + * Verify some MOUNT_SETATTR(2) attributes:
> + * 1) MOUNT_ATTR_RDONLY - makes the mount read-only
> + * 2) MOUNT_ATTR_NOSUID - causes the mount not to honor the
> + *    set-user-ID and set-group-ID mode bits and file capabilities
> + *    when executing programs.
> + * 3) MOUNT_ATTR_NODEV - prevents access to devices on this mount
> + * 4) MOUNT_ATTR_NOEXEC - prevents executing programs on this mount
> + * 5) MOUNT_ATTR_NOSYMFOLLOW - prevents following symbolic links
> + *    on this mount
> + * 6) MOUNT_ATTR_NODIRATIME - prevents updating access time for
> + *    directories on this mount
> + * Minimum Linux version required is v5.12.
Since we don't check for v5.12, it might be better to state "The functionality
was added in v5.12." (although only enterprise kernels would backport new
functionality, mainline stable kernels will not).
> + */

This needs some changes in order to be formatted properly as list and have
paragraphs. Sigh, nobody really runs:

cd metadata/ && make && chromium ../docparse/metadata.html

to have look what output his docs has :(
(can be fixed before merge)

/*\
 * [Description]
 *
 * Basic mount_setattr() test.
 * Test whether the basic mount attributes are set correctly.
 *
 * Verify some MOUNT_SETATTR(2) attributes:
 *
 * 1. MOUNT_ATTR_RDONLY &ndash; makes the mount read-only
 * 2. MOUNT_ATTR_NOSUID &ndash; causes the mount not to honor the
 *    set-user-ID and set-group-ID mode bits and file capabilities
 *    when executing programs.
 * 3. MOUNT_ATTR_NODEV &ndash; prevents access to devices on this mount
 * 4. MOUNT_ATTR_NOEXEC &ndash; prevents executing programs on this mount
 * 5. MOUNT_ATTR_NOSYMFOLLOW &ndash; prevents following symbolic links
 *    on this mount
 * 6. MOUNT_ATTR_NODIRATIME &ndash; prevents updating access time for
 *    directories on this mount
 *
 * Minimum Linux version required is v5.12.
 */

...
> +static void run(unsigned int n)
> +{
> +	int otfd;
> +	struct tcase *tc = &tcases[n];
> +	struct mount_attr attr = {
> +		.attr_set = tc->mount_attrs,
> +	};
> +	struct statvfs buf;
> +
> +	TST_EXP_FD_SILENT(open_tree(AT_FDCWD, MNTPOINT, AT_EMPTY_PATH |
> +		AT_SYMLINK_NOFOLLOW | OPEN_TREE_CLOEXEC | OPEN_TREE_CLONE));
Although Cyril mentioned only TST_EXP_FD_SILENT(), IMHO it should be followed with:
	if (!TST_PASS)
		return;

Or does it make sense to continue testing when open_tree() fails?

> +	otfd = (int)TST_RET;
> +
> +	TST_EXP_PASS_SILENT(mount_setattr(otfd, "", AT_EMPTY_PATH, &attr, sizeof(attr)),
> +		"%s set", tc->name);

Shouldn't be here also :
	if (!TST_PASS)
		return;

check? I guess we need SAFE_ variants for not having to check it all the time.

Or does it make sense to continue testing when mount_setattr() fails?

> +	TEST(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH));
> +
> +	if (TST_RET == -1) {
> +		tst_res(TFAIL | TTERRNO, "move_mount() failed");
> +		return;
> +	}

Maybe instead of TEST() and if use this?

	TST_EXP_PASS_SILENT(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH));

	if (!TST_PASS)
		return;

> +
> +	SAFE_CLOSE(otfd);
> +
> +	TST_EXP_PASS_SILENT(statvfs(OT_MNTPOINT, &buf), "statvfs sucess");
And here as well.
> +
> +	if ((buf.f_flag & tc->expect_attrs) == 0)
> +		tst_res(TFAIL, "%s is not actually set as expected", tc->name);
> +	else
> +		tst_res(TPASS, "%s is actually set as expected", tc->name);
> +
> +	if (tst_is_mounted_at_tmpdir(OT_MNTPOINT))
> +		SAFE_UMOUNT(OT_MNTPOINT);
> +}

The rest LGTM.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2] syscalls/mount_setattr01: Add basic functional test

[Cyril Hrubis]

Hi!
> > +struct mount_attr {
> > +	uint64_t attr_set;
> > +	uint64_t attr_clr;
> > +	uint64_t propagation;
> > +	uint64_t userns_fd;
> > +};
> Interesting enough: in kernel tools/testing/selftests/mount_setattr/mount_setattr_test.c
> defines it as __u64 (IMHO should be really uint64_t as that test is userspace as
> Cyril pointed out) but real kernel code in fs/namespace.c happily uses "unsigned
> int" :).

You are just confused by the fact that there are two different
structures there is mount_attr and mount_kattr, the mount_setattr()
syscall gets the mount_attr from userspace and then fills in mount_kattr
based on that. See the function build_mount_kattr() in fs/namespace.c

I guess that this is simply done so that there is enough space for flags
to be added in the future without a need to change the structure.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2] syscalls/mount_setattr01: Add basic functional test

[Petr Vorel]

> Hi!
> > > +struct mount_attr {
> > > +	uint64_t attr_set;
> > > +	uint64_t attr_clr;
> > > +	uint64_t propagation;
> > > +	uint64_t userns_fd;
> > > +};
> > Interesting enough: in kernel tools/testing/selftests/mount_setattr/mount_setattr_test.c
> > defines it as __u64 (IMHO should be really uint64_t as that test is userspace as
> > Cyril pointed out) but real kernel code in fs/namespace.c happily uses "unsigned
> > int" :).

> You are just confused by the fact that there are two different
> structures there is mount_attr and mount_kattr, the mount_setattr()
> syscall gets the mount_attr from userspace and then fills in mount_kattr
> based on that. See the function build_mount_kattr() in fs/namespace.c

> I guess that this is simply done so that there is enough space for flags
> to be added in the future without a need to change the structure.

Ah, thx for info.

Kind regards,
Petr

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

[LTP] 回复: [PATCH v2] syscalls/mount_setattr01: Add basic functional test

[chenhx.fnst]

Hi,

> -----邮件原件-----
> 发件人: Petr Vorel <pvorel@suse.cz>
> 发送时间: 2022年4月21日 4:43
> <chrubis@suse.cz>
> 主题: Re: [LTP] [PATCH v2] syscalls/mount_setattr01: Add basic functional test
> 
> Hi Chen, Dai,
> 
> > From: Chen Hanxiao <chenhx.fnst@fujitsu.com>
> 
> > diff --git a/include/lapi/fsmount.h b/include/lapi/fsmount.h
> ...
> > +/*
> > + * mount_setattr()
> > + */
> > +struct mount_attr {
> > +	uint64_t attr_set;
> > +	uint64_t attr_clr;
> > +	uint64_t propagation;
> > +	uint64_t userns_fd;
> > +};
> Interesting enough: in kernel
> tools/testing/selftests/mount_setattr/mount_setattr_test.c
> defines it as __u64 (IMHO should be really uint64_t as that test is userspace as
> Cyril pointed out) but real kernel code in fs/namespace.c happily uses "unsigned
> int" :).
> 
> ...
> > diff --git a/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
> > b/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
> > new file mode 100644
> > index 000000000..d63db46fa
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
> > @@ -0,0 +1,118 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
> > + * Author: Dai Shili <daisl.fnst@fujitsu.com>
> > + * Author: Chen Hanxiao <chenhx.fnst@fujitsu.com>  */
> > +
> > +/*\
> > + * [Description]
> > + *
> > + * Basic mount_setattr() test.
> > + * Test whether the basic mount attributes are set correctly.
> > + *
> > + * Verify some MOUNT_SETATTR(2) attributes:
> > + * 1) MOUNT_ATTR_RDONLY - makes the mount read-only
> > + * 2) MOUNT_ATTR_NOSUID - causes the mount not to honor the
> > + *    set-user-ID and set-group-ID mode bits and file capabilities
> > + *    when executing programs.
> > + * 3) MOUNT_ATTR_NODEV - prevents access to devices on this mount
> > + * 4) MOUNT_ATTR_NOEXEC - prevents executing programs on this mount
> > + * 5) MOUNT_ATTR_NOSYMFOLLOW - prevents following symbolic links
> > + *    on this mount
> > + * 6) MOUNT_ATTR_NODIRATIME - prevents updating access time for
> > + *    directories on this mount
> > + * Minimum Linux version required is v5.12.
> Since we don't check for v5.12, it might be better to state "The functionality was
> added in v5.12." (although only enterprise kernels would backport new
> functionality, mainline stable kernels will not).

Fine.
> > + */
> 
> This needs some changes in order to be formatted properly as list and have
> paragraphs. Sigh, nobody really runs:
> 
> cd metadata/ && make && chromium ../docparse/metadata.html
> 
> to have look what output his docs has :( (can be fixed before merge)
> 
Will be fixed and I'll be more careful next time.

> /*\
...
> OPEN_TREE_CLONE));
> Although Cyril mentioned only TST_EXP_FD_SILENT(), IMHO it should be
> followed with:
> 	if (!TST_PASS)
> 		return;
> 
> Or does it make sense to continue testing when open_tree() fails?
> 
> > +	otfd = (int)TST_RET;
> > +
> > +	TST_EXP_PASS_SILENT(mount_setattr(otfd, "", AT_EMPTY_PATH, &attr,
> sizeof(attr)),
> > +		"%s set", tc->name);
> 
> Shouldn't be here also :
> 	if (!TST_PASS)
> 		return;
> 
> check? I guess we need SAFE_ variants for not having to check it all the time.

It's hard for a SAFE_MOUNT_SETATTR:
mount_setattr(2) need:
#include <linux/mount.h> /* Definition of MOUNT_ATTR_* constants */

But linux/mount.h have some conflicts with sys/mount.h, which is widely used in LTP.
If we really want SAFE_MOUNT_SETATTR, we may need a big refactor.

> 
> Or does it make sense to continue testing when mount_setattr() fails?
> 
> > +	TEST(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT,
> > +MOVE_MOUNT_F_EMPTY_PATH));
> > +
> > +	if (TST_RET == -1) {
> > +		tst_res(TFAIL | TTERRNO, "move_mount() failed");
> > +		return;
> > +	}
> 
> Maybe instead of TEST() and if use this?
> 
> 	TST_EXP_PASS_SILENT(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT,
> MOVE_MOUNT_F_EMPTY_PATH));
> 
> 	if (!TST_PASS)
> 		return;
> 

Sure, I'll post v3 soon.

Regards,
- Chen

> > +
> > +	SAFE_CLOSE(otfd);
> > +
> > +	TST_EXP_PASS_SILENT(statvfs(OT_MNTPOINT, &buf), "statvfs sucess");
> And here as well.
> > +
> > +	if ((buf.f_flag & tc->expect_attrs) == 0)
> > +		tst_res(TFAIL, "%s is not actually set as expected", tc->name);
> > +	else
> > +		tst_res(TPASS, "%s is actually set as expected", tc->name);
> > +
> > +	if (tst_is_mounted_at_tmpdir(OT_MNTPOINT))
> > +		SAFE_UMOUNT(OT_MNTPOINT);
> > +}
> 
> The rest LGTM.
> 
> Kind regards,
> Petr

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

Re: [LTP] 回复: [PATCH v2] syscalls/mount_setattr01: Add basic functional test

[Petr Vorel]

Hi Chen,

> > I guess we need SAFE_ variants for not having to check it all the time.

> It's hard for a SAFE_MOUNT_SETATTR:
> mount_setattr(2) need:
> #include <linux/mount.h> /* Definition of MOUNT_ATTR_* constants */

> But linux/mount.h have some conflicts with sys/mount.h, which is widely used in LTP.
> If we really want SAFE_MOUNT_SETATTR, we may need a big refactor.

Well, you don't have to bother with SAFE_MOUNT_SETATTR() unless you have time
for it. That will not block this patchset.

FYI I did some cleanup of include/lapi/mount.h and include/lapi/fsmount.h:
c0cb5d1962 ("lapi/mount.h: Include kernel/libc header")
8ae596f8b5 ("lapi/fsmount.h: Include lapi/mount.h instead libc header")

FYI we include kernel header and if missing, we include libc header:
#ifdef HAVE_LINUX_MOUNT_H
# include <linux/mount.h>
#else
# include <sys/mount.h>
#endif

If kernel header is old enough (theoretically we support compilation on 3.10 [1]),
we just include it and don't bother about checks.

If you want to work on SAFE_*(): not only SAFE_MOUNT_SETATTR(), but more
functions could be defined:
SAFE_MOVE_MOUNT() (could be used in fs{config,mount,open} tests), maybe
SAFE_OPEN_TREE() (it'd be used only in this test as open_tree tests should
probably keep using TEST(open_tree(...)).

I'd put it/them into new file lib/tst_safe_fsmount.c and include/tst_safe_fsmount.h.

NOTE: Although we have SAFE_MOUNT() in lib/safe_macros.c, IMHO we should keep it
there, instead of creating lib/tst_safe_mount.c, because lib/safe_macros.c
supports also legacy API (which we don't want to continue developing) and we
don't want to put many include/tst_safe_mount.h.

TODO for myself: SAFE_STATVFS() and SAFE_FSTATVFS()


> Sure, I'll post v3 soon.
+1

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/wiki/Supported-kernel,-libc,-toolchain-versions

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