All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v1] mount_setattr02.c: Check mount_setattr attr.propagation
@ 2025-02-17  2:04 Wei Gao via ltp
  2025-02-18 15:18 ` Petr Vorel
  2025-02-19  8:29 ` [LTP] [PATCH v2] " Wei Gao via ltp
  0 siblings, 2 replies; 13+ messages in thread
From: Wei Gao via ltp @ 2025-02-17  2:04 UTC (permalink / raw)
  To: ltp

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

diff --git a/runtest/syscalls b/runtest/syscalls
index 4ab8436d3..b856027df 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -856,6 +856,7 @@ mount06 mount06
 mount07 mount07
 
 mount_setattr01 mount_setattr01
+mount_setattr02 mount_setattr02
 
 move_mount01 move_mount01
 move_mount02 move_mount02
diff --git a/testcases/kernel/syscalls/mount_setattr/.gitignore b/testcases/kernel/syscalls/mount_setattr/.gitignore
index 52a77b9ca..1654f27de 100644
--- a/testcases/kernel/syscalls/mount_setattr/.gitignore
+++ b/testcases/kernel/syscalls/mount_setattr/.gitignore
@@ -1 +1,2 @@
 /mount_setattr01
+/mount_setattr02
diff --git a/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
new file mode 100644
index 000000000..c7c5b3e74
--- /dev/null
+++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2025 SUSE LLC Wei Gao <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Basic mount_setattr() test.
+ * Test basic propagation mount attributes are set correctly.
+ */
+
+#define _GNU_SOURCE
+
+#include <sys/statvfs.h>
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+#include "tst_safe_stdio.h"
+
+#define DIRA "/DIRA_PROPAGATION_CHECK"
+
+static bool is_shared_mount(const char *path)
+{
+	FILE *file = SAFE_FOPEN("/proc/self/mountinfo", "r");
+
+	char line[PATH_MAX];
+	bool found = false;
+
+	while (fgets(line, sizeof(line), file)) {
+		char mntpoint[PATH_MAX];
+		char opts[256];
+
+		if (sscanf(line, "%*d %*d %*d:%*d %*s %255s %*s %255s",
+					mntpoint, opts) != 2)
+			continue;
+
+		if (strcmp(mntpoint, path) != 0)
+			continue;
+
+		if (strstr(opts, "shared:") != NULL) {
+			found = true;
+			break;
+		}
+	}
+
+	fclose(file);
+	return found;
+}
+
+static void cleanup(void)
+{
+	SAFE_RMDIR(DIRA);
+
+}
+
+static void setup(void)
+{
+	fsopen_supported_by_kernel();
+
+	SAFE_MKDIR(DIRA, 0777);
+}
+
+static void run(void)
+{
+
+	SAFE_UNSHARE(CLONE_NEWNS);
+	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
+	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
+
+	struct mount_attr attr = {
+		.attr_set       = MOUNT_ATTR_RDONLY | MOUNT_ATTR_NOEXEC | MOUNT_ATTR_RELATIME,
+		.attr_clr       = MOUNT_ATTR__ATIME,
+	};
+
+	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
+
+	attr.propagation = -1;
+	TST_EXP_FAIL_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)), EINVAL);
+	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
+
+	attr.propagation = MS_SHARED;
+	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(DIRA), 1);
+
+	attr.propagation = MS_PRIVATE;
+	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
+
+	attr.propagation = MS_SLAVE;
+	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
+
+	SAFE_UMOUNT(DIRA);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+};
-- 
2.35.3


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [LTP] [PATCH v1] mount_setattr02.c: Check mount_setattr attr.propagation
  2025-02-17  2:04 [LTP] [PATCH v1] mount_setattr02.c: Check mount_setattr attr.propagation Wei Gao via ltp
@ 2025-02-18 15:18 ` Petr Vorel
  2025-02-19  8:47   ` Wei Gao via ltp
  2025-02-19  8:29 ` [LTP] [PATCH v2] " Wei Gao via ltp
  1 sibling, 1 reply; 13+ messages in thread
From: Petr Vorel @ 2025-02-18 15:18 UTC (permalink / raw)
  To: Wei Gao; +Cc: ltp

Hi Wei,

nit: I guess you want to replace dot with space in subject.

> +++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2025 SUSE LLC Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Basic mount_setattr() test.
> + * Test basic propagation mount attributes are set correctly.
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include <sys/statvfs.h>
> +#include "tst_test.h"
> +#include "lapi/fsmount.h"
> +#include "tst_safe_stdio.h"
> +
> +#define DIRA "/DIRA_PROPAGATION_CHECK"

Is it necessary to to use directory under root?

...
> +static void cleanup(void)
> +{

I guess this is due result of:
SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
There should be either a proper detection whether this works or 

../../../../include/lapi/fsmount.h:113: TCONF: syscall(442) __NR_mount_setattr not supported on your arch
mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: EBUSY (16)


> +	SAFE_RMDIR(DIRA);

When running on old kernel (e.g. SLES based 4.12) it fails due TCONF:

../../../../include/lapi/fsmount.h:197: TCONF: Test not supported on kernel version < v5.2
mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: ENOENT (2)

There should be a flag to remove dir only when it was created.

> +
nit: please remove this new line (I have to keep asking this :( ).
> +}
> +
> +static void setup(void)
> +{
> +	fsopen_supported_by_kernel();
I wonder if this needed for detecting new mount API support. Because second
SAFE_MOUNT also runs code which detects code unsupported:

../../../../include/lapi/fsmount.h:113: TCONF: syscall(442) __NR_mount_setattr not supported on your arch
mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: EBUSY (16)

But I have no idea what would be needed to be done to cleanup result of the
first SAFE_MOUNT().

> +
> +	SAFE_MKDIR(DIRA, 0777);
> +}
> +
> +static void run(void)
> +{
> +
and here new line.
> +	SAFE_UNSHARE(CLONE_NEWNS);
> +	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> +	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
Do these 2 needs to be in the run()? How about move them to setup()?

static int dir_created, mounted;

static void setup(void)
{
	fsopen_supported_by_kernel();

	SAFE_MKDIR(DIRA, 0777);
	dir_created = 1;
	SAFE_UNSHARE(CLONE_NEWNS);
	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
	mounted = 1;
}

static void cleanup(void)
{
	if (mounted)
		SAFE_UMOUNT(DIRA);

	if (dir_created)
		SAFE_RMDIR(DIRA);
}

+ I later create generic helper from is_shared_mount().

Kind regards,
Petr

> +
> +	struct mount_attr attr = {
> +		.attr_set       = MOUNT_ATTR_RDONLY | MOUNT_ATTR_NOEXEC | MOUNT_ATTR_RELATIME,
> +		.attr_clr       = MOUNT_ATTR__ATIME,
> +	};
> +
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +
> +	attr.propagation = -1;
> +	TST_EXP_FAIL_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)), EINVAL);
> +	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +
> +	attr.propagation = MS_SHARED;
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(DIRA), 1);
> +
> +	attr.propagation = MS_PRIVATE;
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +
> +	attr.propagation = MS_SLAVE;
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +
> +	SAFE_UMOUNT(DIRA);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +};

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [LTP] [PATCH v2] mount_setattr02.c: Check mount_setattr attr.propagation
  2025-02-17  2:04 [LTP] [PATCH v1] mount_setattr02.c: Check mount_setattr attr.propagation Wei Gao via ltp
  2025-02-18 15:18 ` Petr Vorel
@ 2025-02-19  8:29 ` Wei Gao via ltp
  2025-02-21 12:54   ` Petr Vorel
  2025-03-19 11:41   ` [LTP] [PATCH v3] mount_setattr02.c: Check mount_setattr attr propagation Wei Gao via ltp
  1 sibling, 2 replies; 13+ messages in thread
From: Wei Gao via ltp @ 2025-02-19  8:29 UTC (permalink / raw)
  To: ltp

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

diff --git a/runtest/syscalls b/runtest/syscalls
index 4ab8436d3..b856027df 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -856,6 +856,7 @@ mount06 mount06
 mount07 mount07
 
 mount_setattr01 mount_setattr01
+mount_setattr02 mount_setattr02
 
 move_mount01 move_mount01
 move_mount02 move_mount02
diff --git a/testcases/kernel/syscalls/mount_setattr/.gitignore b/testcases/kernel/syscalls/mount_setattr/.gitignore
index 52a77b9ca..1654f27de 100644
--- a/testcases/kernel/syscalls/mount_setattr/.gitignore
+++ b/testcases/kernel/syscalls/mount_setattr/.gitignore
@@ -1 +1,2 @@
 /mount_setattr01
+/mount_setattr02
diff --git a/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
new file mode 100644
index 000000000..301c55ef9
--- /dev/null
+++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2025 SUSE LLC Wei Gao <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Basic mount_setattr() test.
+ * Test basic propagation mount attributes are set correctly.
+ */
+
+#define _GNU_SOURCE
+
+#include <sys/statvfs.h>
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+#include "tst_safe_stdio.h"
+
+#define DIRA "/DIRA_PROPAGATION_CHECK"
+
+static int dir_created, mounted;
+
+static bool is_shared_mount(const char *path)
+{
+	FILE *file = SAFE_FOPEN("/proc/self/mountinfo", "r");
+
+	char line[PATH_MAX];
+	bool found = false;
+
+	while (fgets(line, sizeof(line), file)) {
+		char mntpoint[PATH_MAX];
+		char opts[256];
+
+		if (sscanf(line, "%*d %*d %*d:%*d %*s %255s %*s %255s",
+					mntpoint, opts) != 2)
+			continue;
+
+		if (strcmp(mntpoint, path) != 0)
+			continue;
+
+		if (strstr(opts, "shared:") != NULL) {
+			found = true;
+			break;
+		}
+	}
+
+	fclose(file);
+	return found;
+}
+
+static void cleanup(void)
+{
+	if (mounted)
+		SAFE_UMOUNT(DIRA);
+
+	if (dir_created)
+		SAFE_RMDIR(DIRA);
+}
+
+static void setup(void)
+{
+	SAFE_MKDIR(DIRA, 0777);
+	SAFE_UNSHARE(CLONE_NEWNS);
+	dir_created = 1;
+	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
+	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
+	mounted = 1;
+}
+
+static void run(void)
+{
+	struct mount_attr attr = {
+		.attr_set       = MOUNT_ATTR_RDONLY | MOUNT_ATTR_NOEXEC | MOUNT_ATTR_RELATIME,
+		.attr_clr       = MOUNT_ATTR__ATIME,
+	};
+
+	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
+
+	attr.propagation = -1;
+	TST_EXP_FAIL_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)), EINVAL);
+	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
+
+	attr.propagation = MS_SHARED;
+	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(DIRA), 1);
+
+	attr.propagation = MS_PRIVATE;
+	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
+
+	attr.propagation = MS_SLAVE;
+	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+};
-- 
2.35.3


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [LTP] [PATCH v1] mount_setattr02.c: Check mount_setattr attr.propagation
  2025-02-18 15:18 ` Petr Vorel
@ 2025-02-19  8:47   ` Wei Gao via ltp
  2025-02-19  9:05     ` Petr Vorel
  0 siblings, 1 reply; 13+ messages in thread
From: Wei Gao via ltp @ 2025-02-19  8:47 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

On Tue, Feb 18, 2025 at 04:18:58PM +0100, Petr Vorel wrote:
> Hi Wei,
> 
> nit: I guess you want to replace dot with space in subject.
You mean i s/attr.propagation/attr propagation ?
> 
> > +++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
> > @@ -0,0 +1,102 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) 2025 SUSE LLC Wei Gao <wegao@suse.com>
> > + */
> > +
> > +/*\
> > + * [Description]
> > + *
> > + * Basic mount_setattr() test.
> > + * Test basic propagation mount attributes are set correctly.
> > + */
> > +
> > +#define _GNU_SOURCE
> > +
> > +#include <sys/statvfs.h>
> > +#include "tst_test.h"
> > +#include "lapi/fsmount.h"
> > +#include "tst_safe_stdio.h"
> > +
> > +#define DIRA "/DIRA_PROPAGATION_CHECK"
> 
> Is it necessary to to use directory under root?
Yes. Otherwise failed will happen during mount_setattr.
But i have not check for detail.
> 
> ...
> > +static void cleanup(void)
> > +{
> 
> I guess this is due result of:
> SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> There should be either a proper detection whether this works or 
> 
> ../../../../include/lapi/fsmount.h:113: TCONF: syscall(442) __NR_mount_setattr not supported on your arch
> mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: EBUSY (16)
> 
> 
> > +	SAFE_RMDIR(DIRA);
> 
> When running on old kernel (e.g. SLES based 4.12) it fails due TCONF:
> 
> ../../../../include/lapi/fsmount.h:197: TCONF: Test not supported on kernel version < v5.2
> mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: ENOENT (2)
> 
> There should be a flag to remove dir only when it was created.
> 
> > +
> nit: please remove this new line (I have to keep asking this :( ).
Sorry, i need pay attention, thanks for point this out with great patient :).
> > +}
> > +
> > +static void setup(void)
> > +{
> > +	fsopen_supported_by_kernel();
> I wonder if this needed for detecting new mount API support. Because second
> SAFE_MOUNT also runs code which detects code unsupported:
> 
> ../../../../include/lapi/fsmount.h:113: TCONF: syscall(442) __NR_mount_setattr not supported on your arch
> mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: EBUSY (16)
> 
> But I have no idea what would be needed to be done to cleanup result of the
> first SAFE_MOUNT().
> 
> > +
> > +	SAFE_MKDIR(DIRA, 0777);
> > +}
> > +
> > +static void run(void)
> > +{
> > +
> and here new line.
> > +	SAFE_UNSHARE(CLONE_NEWNS);
> > +	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> > +	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
> Do these 2 needs to be in the run()? How about move them to setup()?
> 
> static int dir_created, mounted;
> 
> static void setup(void)
> {
> 	fsopen_supported_by_kernel();
> 
> 	SAFE_MKDIR(DIRA, 0777);
> 	dir_created = 1;
> 	SAFE_UNSHARE(CLONE_NEWNS);
> 	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> 	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
> 	mounted = 1;
> }
> 
> static void cleanup(void)
> {
> 	if (mounted)
> 		SAFE_UMOUNT(DIRA);
> 
> 	if (dir_created)
> 		SAFE_RMDIR(DIRA);
> }
> 
> + I later create generic helper from is_shared_mount().
I sent v2 patch now, once your create generic helper i can do another patch together with
your generic helper.
> 
> Kind regards,
> Petr
> 

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [LTP] [PATCH v1] mount_setattr02.c: Check mount_setattr attr.propagation
  2025-02-19  8:47   ` Wei Gao via ltp
@ 2025-02-19  9:05     ` Petr Vorel
  2025-02-19  9:51       ` Wei Gao via ltp
  0 siblings, 1 reply; 13+ messages in thread
From: Petr Vorel @ 2025-02-19  9:05 UTC (permalink / raw)
  To: Wei Gao; +Cc: ltp

> On Tue, Feb 18, 2025 at 04:18:58PM +0100, Petr Vorel wrote:
> > Hi Wei,

> > nit: I guess you want to replace dot with space in subject.
> You mean i s/attr.propagation/attr propagation ?

Yes.

...
> > > +#define DIRA "/DIRA_PROPAGATION_CHECK"

> > Is it necessary to to use directory under root?
> Yes. Otherwise failed will happen during mount_setattr.
> But i have not check for detail.

Ideally we would create files in TMPDIR. Specially if bug in the code leave
/DIRA_PROPAGATION_CHECK kept.

> > ...
> > > +static void cleanup(void)
> > > +{

> > I guess this is due result of:
> > SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> > There should be either a proper detection whether this works or 

> > ../../../../include/lapi/fsmount.h:113: TCONF: syscall(442) __NR_mount_setattr not supported on your arch
> > mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: EBUSY (16)


> > > +	SAFE_RMDIR(DIRA);

> > When running on old kernel (e.g. SLES based 4.12) it fails due TCONF:

> > ../../../../include/lapi/fsmount.h:197: TCONF: Test not supported on kernel version < v5.2
> > mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: ENOENT (2)

> > There should be a flag to remove dir only when it was created.

> > > +
> > nit: please remove this new line (I have to keep asking this :( ).
> Sorry, i need pay attention, thanks for point this out with great patient :).

Thank you, not a big deal :).

...
> > + I later create generic helper from is_shared_mount().
> I sent v2 patch now, once your create generic helper i can do another patch together with
> your generic helper.

Sure, it should not block your work on the test.

Kind regards,
Petr

> > Kind regards,
> > Petr

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [LTP] [PATCH v1] mount_setattr02.c: Check mount_setattr attr.propagation
  2025-02-19  9:05     ` Petr Vorel
@ 2025-02-19  9:51       ` Wei Gao via ltp
  2025-02-19 11:24         ` Petr Vorel
  0 siblings, 1 reply; 13+ messages in thread
From: Wei Gao via ltp @ 2025-02-19  9:51 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

On Wed, Feb 19, 2025 at 10:05:36AM +0100, Petr Vorel wrote:
> > On Tue, Feb 18, 2025 at 04:18:58PM +0100, Petr Vorel wrote:
> > > Hi Wei,
> 
> > > nit: I guess you want to replace dot with space in subject.
> > You mean i s/attr.propagation/attr propagation ?
> 
> Yes.
Will update it in v3
> 
> ...
> > > > +#define DIRA "/DIRA_PROPAGATION_CHECK"
> 
> > > Is it necessary to to use directory under root?
> > Yes. Otherwise failed will happen during mount_setattr.
> > But i have not check for detail.
> 
> Ideally we would create files in TMPDIR. Specially if bug in the code leave
> /DIRA_PROPAGATION_CHECK kept.
Got it, will try using TMPDIR and investigate why it report error.
> 
> > > ...
> Sure, it should not block your work on the test.
> 
> Kind regards,
> Petr
> 
> > > Kind regards,
> > > Petr

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [LTP] [PATCH v1] mount_setattr02.c: Check mount_setattr attr.propagation
  2025-02-19  9:51       ` Wei Gao via ltp
@ 2025-02-19 11:24         ` Petr Vorel
  0 siblings, 0 replies; 13+ messages in thread
From: Petr Vorel @ 2025-02-19 11:24 UTC (permalink / raw)
  To: Wei Gao; +Cc: ltp

> On Wed, Feb 19, 2025 at 10:05:36AM +0100, Petr Vorel wrote:
> > > On Tue, Feb 18, 2025 at 04:18:58PM +0100, Petr Vorel wrote:
> > > > Hi Wei,

> > > > nit: I guess you want to replace dot with space in subject.
> > > You mean i s/attr.propagation/attr propagation ?

> > Yes.
> Will update it in v3

This is not important (the code matters), but IMHO something like:

"mount_setattr02: Add test to check mount attributes propagation"

would be more readable for me than "mount_setattr02.c: Check mount_setattr
attr.propagation" you provided. From your subject it's not even obvious it's a
new test and not a change in the existing test.

> > ...
> > > > > +#define DIRA "/DIRA_PROPAGATION_CHECK"

> > > > Is it necessary to to use directory under root?
> > > Yes. Otherwise failed will happen during mount_setattr.
> > > But i have not check for detail.

> > Ideally we would create files in TMPDIR. Specially if bug in the code leave
> > /DIRA_PROPAGATION_CHECK kept.
> Got it, will try using TMPDIR and investigate why it report error.

Thank you!

Kind regards,
Petr

> > > > ...
> > Sure, it should not block your work on the test.

> > Kind regards,
> > Petr

> > > > Kind regards,
> > > > Petr

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [LTP] [PATCH v2] mount_setattr02.c: Check mount_setattr attr.propagation
  2025-02-19  8:29 ` [LTP] [PATCH v2] " Wei Gao via ltp
@ 2025-02-21 12:54   ` Petr Vorel
  2025-02-24  1:44     ` Wei Gao via ltp
  2025-03-19 11:41   ` [LTP] [PATCH v3] mount_setattr02.c: Check mount_setattr attr propagation Wei Gao via ltp
  1 sibling, 1 reply; 13+ messages in thread
From: Petr Vorel @ 2025-02-21 12:54 UTC (permalink / raw)
  To: Wei Gao; +Cc: ltp

Hi Wei,

nit: You did not change commit message subject (we talked about it).

> --- /dev/null
> +++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
> @@ -0,0 +1,103 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2025 SUSE LLC Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
NOTE: this '[Description]' should no longer be added.

See:
https://github.com/linux-test-project/ltp/commit/a3e27df34d6cb49477c9bd6f9bbaa1ce4de155f9
(it updated also examples in doc)

and follow ups
https://github.com/linux-test-project/ltp/commit/63eceaa2a83bca41997edcd649eb62272622a100
https://github.com/linux-test-project/ltp/commit/824f66ca72fc7505a31c30792334905b646c9d37
> + *
> + * Basic mount_setattr() test.
nit: here is better a blank line.
> + * Test basic propagation mount attributes are set correctly.
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include <sys/statvfs.h>
> +#include "tst_test.h"
> +#include "lapi/fsmount.h"
> +#include "tst_safe_stdio.h"
> +
> +#define DIRA "/DIRA_PROPAGATION_CHECK"

Also, I found a way to test into TMPDIR. There is no need to touch a real root.
It works even on separate /tmp.

If you agree, I merge with following diff.
https://github.com/pevik/ltp/blob/wei/mount_setattr02.v2.fixes/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c

With changes below.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

+++ testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
@@ -4,9 +4,8 @@
  */
 
 /*\
- * [Description]
- *
  * Basic mount_setattr() test.
+ *
  * Test basic propagation mount attributes are set correctly.
  */
 
@@ -17,9 +16,9 @@
 #include "lapi/fsmount.h"
 #include "tst_safe_stdio.h"
 
-#define DIRA "/DIRA_PROPAGATION_CHECK"
+static char *tmpdir;
 
-static int dir_created, mounted;
+static int mounted;
 
 static bool is_shared_mount(const char *path)
 {
@@ -52,19 +51,15 @@ static bool is_shared_mount(const char *path)
 static void cleanup(void)
 {
 	if (mounted)
-		SAFE_UMOUNT(DIRA);
-
-	if (dir_created)
-		SAFE_RMDIR(DIRA);
+		SAFE_UMOUNT(tmpdir);
 }
 
 static void setup(void)
 {
-	SAFE_MKDIR(DIRA, 0777);
+	tmpdir = tst_tmpdir_path();
 	SAFE_UNSHARE(CLONE_NEWNS);
-	dir_created = 1;
 	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
-	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
+	SAFE_MOUNT("testing", tmpdir, "tmpfs", MS_NOATIME | MS_NODEV, "");
 	mounted = 1;
 }
 
@@ -75,24 +70,24 @@ static void run(void)
 		.attr_clr       = MOUNT_ATTR__ATIME,
 	};
 
-	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
-	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
+	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
 
 	attr.propagation = -1;
-	TST_EXP_FAIL_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)), EINVAL);
-	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
+	TST_EXP_FAIL_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)), EINVAL);
+	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
 
 	attr.propagation = MS_SHARED;
-	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
-	TST_EXP_EQ_LI(is_shared_mount(DIRA), 1);
+	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 1);
 
 	attr.propagation = MS_PRIVATE;
-	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
-	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
+	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
 
 	attr.propagation = MS_SLAVE;
-	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
-	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
+	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
 }
 
 static struct tst_test test = {
@@ -100,4 +95,5 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.needs_root = 1,
+	.needs_tmpdir = 1,
 };

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [LTP] [PATCH v2] mount_setattr02.c: Check mount_setattr attr.propagation
  2025-02-21 12:54   ` Petr Vorel
@ 2025-02-24  1:44     ` Wei Gao via ltp
  0 siblings, 0 replies; 13+ messages in thread
From: Wei Gao via ltp @ 2025-02-24  1:44 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

On Fri, Feb 21, 2025 at 01:54:58PM +0100, Petr Vorel wrote:
> Hi Wei,
> 
> nit: You did not change commit message subject (we talked about it).
Oh, if you check time of message, i sent v2 patch before we talked about this :)
> 
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
> > @@ -0,0 +1,103 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) 2025 SUSE LLC Wei Gao <wegao@suse.com>
> > + */
> > +
> > +/*\
> > + * [Description]
> NOTE: this '[Description]' should no longer be added.
> 
> See:
> https://github.com/linux-test-project/ltp/commit/a3e27df34d6cb49477c9bd6f9bbaa1ce4de155f9
> (it updated also examples in doc)
> 
> and follow ups
> https://github.com/linux-test-project/ltp/commit/63eceaa2a83bca41997edcd649eb62272622a100
> https://github.com/linux-test-project/ltp/commit/824f66ca72fc7505a31c30792334905b646c9d37
Got it, thanks your information.
> > + *
> > + * Basic mount_setattr() test.
> nit: here is better a blank line.
> > + * Test basic propagation mount attributes are set correctly.
> > + */
> > +
> > +#define _GNU_SOURCE
> > +
> > +#include <sys/statvfs.h>
> > +#include "tst_test.h"
> > +#include "lapi/fsmount.h"
> > +#include "tst_safe_stdio.h"
> > +
> > +#define DIRA "/DIRA_PROPAGATION_CHECK"
> 
> Also, I found a way to test into TMPDIR. There is no need to touch a real root.
> It works even on separate /tmp.
> 
> If you agree, I merge with following diff.
> https://github.com/pevik/ltp/blob/wei/mount_setattr02.v2.fixes/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
I agree. Great!
> 
> With changes below.
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
> Kind regards,
> Petr
> 
> +++ testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
> @@ -4,9 +4,8 @@
>   */
>  
>  /*\
> - * [Description]
> - *
>   * Basic mount_setattr() test.
> + *
>   * Test basic propagation mount attributes are set correctly.
>   */
>  
> @@ -17,9 +16,9 @@
>  #include "lapi/fsmount.h"
>  #include "tst_safe_stdio.h"
>  
> -#define DIRA "/DIRA_PROPAGATION_CHECK"
> +static char *tmpdir;
>  
> -static int dir_created, mounted;
> +static int mounted;
>  
>  static bool is_shared_mount(const char *path)
>  {
> @@ -52,19 +51,15 @@ static bool is_shared_mount(const char *path)
>  static void cleanup(void)
>  {
>  	if (mounted)
> -		SAFE_UMOUNT(DIRA);
> -
> -	if (dir_created)
> -		SAFE_RMDIR(DIRA);
> +		SAFE_UMOUNT(tmpdir);
>  }
>  
>  static void setup(void)
>  {
> -	SAFE_MKDIR(DIRA, 0777);
> +	tmpdir = tst_tmpdir_path();
>  	SAFE_UNSHARE(CLONE_NEWNS);
> -	dir_created = 1;
>  	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> -	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
> +	SAFE_MOUNT("testing", tmpdir, "tmpfs", MS_NOATIME | MS_NODEV, "");
>  	mounted = 1;
>  }
>  
> @@ -75,24 +70,24 @@ static void run(void)
>  		.attr_clr       = MOUNT_ATTR__ATIME,
>  	};
>  
> -	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> -	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
>  
>  	attr.propagation = -1;
> -	TST_EXP_FAIL_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)), EINVAL);
> -	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +	TST_EXP_FAIL_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)), EINVAL);
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
>  
>  	attr.propagation = MS_SHARED;
> -	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> -	TST_EXP_EQ_LI(is_shared_mount(DIRA), 1);
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 1);
>  
>  	attr.propagation = MS_PRIVATE;
> -	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> -	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
>  
>  	attr.propagation = MS_SLAVE;
> -	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> -	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
>  }
>  
>  static struct tst_test test = {
> @@ -100,4 +95,5 @@ static struct tst_test test = {
>  	.setup = setup,
>  	.cleanup = cleanup,
>  	.needs_root = 1,
> +	.needs_tmpdir = 1,
>  };

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [LTP] [PATCH v3] mount_setattr02.c: Check mount_setattr attr propagation
  2025-02-19  8:29 ` [LTP] [PATCH v2] " Wei Gao via ltp
  2025-02-21 12:54   ` Petr Vorel
@ 2025-03-19 11:41   ` Wei Gao via ltp
  2025-07-11  9:21     ` Cyril Hrubis
  2025-07-24 13:40     ` [LTP] [PATCH v4] " Wei Gao via ltp
  1 sibling, 2 replies; 13+ messages in thread
From: Wei Gao via ltp @ 2025-03-19 11:41 UTC (permalink / raw)
  To: ltp

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

diff --git a/runtest/syscalls b/runtest/syscalls
index 839c23d0a..60cbb66b7 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -858,6 +858,7 @@ mount06 mount06
 mount07 mount07
 
 mount_setattr01 mount_setattr01
+mount_setattr02 mount_setattr02
 
 move_mount01 move_mount01
 move_mount02 move_mount02
diff --git a/testcases/kernel/syscalls/mount_setattr/.gitignore b/testcases/kernel/syscalls/mount_setattr/.gitignore
index 52a77b9ca..1654f27de 100644
--- a/testcases/kernel/syscalls/mount_setattr/.gitignore
+++ b/testcases/kernel/syscalls/mount_setattr/.gitignore
@@ -1 +1,2 @@
 /mount_setattr01
+/mount_setattr02
diff --git a/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
new file mode 100644
index 000000000..fcc088e3b
--- /dev/null
+++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2025 SUSE LLC Wei Gao <wegao@suse.com>
+ */
+
+/*\
+ * Basic mount_setattr() test.
+ *
+ * Test basic propagation mount attributes are set correctly.
+ */
+
+#define _GNU_SOURCE
+
+#include <sys/statvfs.h>
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+#include "tst_safe_stdio.h"
+
+static char *tmpdir;
+
+static int mounted;
+
+static bool is_shared_mount(const char *path)
+{
+	FILE *file = SAFE_FOPEN("/proc/self/mountinfo", "r");
+
+	char line[PATH_MAX];
+	bool found = false;
+
+	while (fgets(line, sizeof(line), file)) {
+		char mntpoint[PATH_MAX];
+		char opts[256];
+
+		if (sscanf(line, "%*d %*d %*d:%*d %*s %255s %*s %255s",
+					mntpoint, opts) != 2)
+			continue;
+
+		if (strcmp(mntpoint, path) != 0)
+			continue;
+
+		if (strstr(opts, "shared:") != NULL) {
+			found = true;
+			break;
+		}
+	}
+
+	fclose(file);
+	return found;
+}
+
+static void cleanup(void)
+{
+	if (mounted)
+		SAFE_UMOUNT(tmpdir);
+}
+
+static void setup(void)
+{
+	tmpdir = tst_tmpdir_path();
+	SAFE_UNSHARE(CLONE_NEWNS);
+	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
+	SAFE_MOUNT("testing", tmpdir, "tmpfs", MS_NOATIME | MS_NODEV, "");
+	mounted = 1;
+}
+
+static void run(void)
+{
+	struct mount_attr attr = {
+		.attr_set       = MOUNT_ATTR_RDONLY | MOUNT_ATTR_NOEXEC | MOUNT_ATTR_RELATIME,
+		.attr_clr       = MOUNT_ATTR__ATIME,
+	};
+
+	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
+
+	attr.propagation = -1;
+	TST_EXP_FAIL_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)), EINVAL);
+	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
+
+	attr.propagation = MS_SHARED;
+	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 1);
+
+	attr.propagation = MS_PRIVATE;
+	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
+
+	attr.propagation = MS_SLAVE;
+	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+};
-- 
2.35.3


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [LTP] [PATCH v3] mount_setattr02.c: Check mount_setattr attr propagation
  2025-03-19 11:41   ` [LTP] [PATCH v3] mount_setattr02.c: Check mount_setattr attr propagation Wei Gao via ltp
@ 2025-07-11  9:21     ` Cyril Hrubis
  2025-07-24 13:40     ` [LTP] [PATCH v4] " Wei Gao via ltp
  1 sibling, 0 replies; 13+ messages in thread
From: Cyril Hrubis @ 2025-07-11  9:21 UTC (permalink / raw)
  To: Wei Gao; +Cc: ltp

Hi!
>  move_mount01 move_mount01
>  move_mount02 move_mount02
> diff --git a/testcases/kernel/syscalls/mount_setattr/.gitignore b/testcases/kernel/syscalls/mount_setattr/.gitignore
> index 52a77b9ca..1654f27de 100644
> --- a/testcases/kernel/syscalls/mount_setattr/.gitignore
> +++ b/testcases/kernel/syscalls/mount_setattr/.gitignore
> @@ -1 +1,2 @@
>  /mount_setattr01
> +/mount_setattr02
> diff --git a/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
> new file mode 100644
> index 000000000..fcc088e3b
> --- /dev/null
> +++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2025 SUSE LLC Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * Basic mount_setattr() test.
> + *
> + * Test basic propagation mount attributes are set correctly.

This is too sparse. We should at least write here which parameters we
are testing, obviously the test is checking if the propagation field of
the structure is handled properly.

And then we should list what we are trying to test

- When propagation is set to 0 it's not changed
- EINVAL with propagation set to -1
- MS_SHARED turns propagation on
- MS_PRIVATE turns propagation off
...

> + */
> +
> +#define _GNU_SOURCE
> +
> +#include <sys/statvfs.h>
> +#include "tst_test.h"
> +#include "lapi/fsmount.h"
> +#include "tst_safe_stdio.h"
> +
> +static char *tmpdir;
> +
> +static int mounted;
> +
> +static bool is_shared_mount(const char *path)
> +{
> +	FILE *file = SAFE_FOPEN("/proc/self/mountinfo", "r");
> +
> +	char line[PATH_MAX];
> +	bool found = false;
> +
> +	while (fgets(line, sizeof(line), file)) {
> +		char mntpoint[PATH_MAX];
> +		char opts[256];
> +
> +		if (sscanf(line, "%*d %*d %*d:%*d %*s %255s %*s %255s",
> +					mntpoint, opts) != 2)
> +			continue;
> +
> +		if (strcmp(mntpoint, path) != 0)
> +			continue;
> +
> +		if (strstr(opts, "shared:") != NULL) {
> +			found = true;
> +			break;
> +		}
> +	}
> +
> +	fclose(file);
> +	return found;
> +}
> +
> +static void cleanup(void)
> +{
> +	if (mounted)
> +		SAFE_UMOUNT(tmpdir);
> +}
> +
> +static void setup(void)
> +{
> +	tmpdir = tst_tmpdir_path();
> +	SAFE_UNSHARE(CLONE_NEWNS);
> +	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> +	SAFE_MOUNT("testing", tmpdir, "tmpfs", MS_NOATIME | MS_NODEV, "");
                     ^
		     We tend to include ltp in names that are visible on
		     the system so that's clear where they came from.

		     It would be a bit better if the name of the mount
		     was "ltp-mount_setattr" or something that makes it clear
		     which test was mounting it.
> +	mounted = 1;
> +}
> +
> +static void run(void)
> +{
> +	struct mount_attr attr = {
> +		.attr_set       = MOUNT_ATTR_RDONLY | MOUNT_ATTR_NOEXEC | MOUNT_ATTR_RELATIME,
> +		.attr_clr       = MOUNT_ATTR__ATIME,


I suppose that we can as well set these to zero as we are trying to test
only the propagation part. Or is there a reason why we are specifying
any attributes here?

> +	};
> +
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
> +
> +	attr.propagation = -1;
> +	TST_EXP_FAIL_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)), EINVAL);
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
> +
> +	attr.propagation = MS_SHARED;
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 1);

Here we should once more check with propagation = 0 and expect that the
mount is shared after that call.

> +	attr.propagation = MS_PRIVATE;
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
> +
> +	attr.propagation = MS_SLAVE;
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);

Can we figure out slave mount from the mountinfo? I suppose there will
be slave in in the opts.

> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +	.needs_tmpdir = 1,
> +};
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [LTP] [PATCH v4] mount_setattr02.c: Check mount_setattr attr propagation
  2025-03-19 11:41   ` [LTP] [PATCH v3] mount_setattr02.c: Check mount_setattr attr propagation Wei Gao via ltp
  2025-07-11  9:21     ` Cyril Hrubis
@ 2025-07-24 13:40     ` Wei Gao via ltp
  2025-07-28 15:37       ` Cyril Hrubis
  1 sibling, 1 reply; 13+ messages in thread
From: Wei Gao via ltp @ 2025-07-24 13:40 UTC (permalink / raw)
  To: ltp

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

diff --git a/runtest/syscalls b/runtest/syscalls
index 9c80bccb0..2528fec52 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -866,6 +866,7 @@ mount06 mount06
 mount07 mount07
 
 mount_setattr01 mount_setattr01
+mount_setattr02 mount_setattr02
 
 move_mount01 move_mount01
 move_mount02 move_mount02
diff --git a/testcases/kernel/syscalls/mount_setattr/.gitignore b/testcases/kernel/syscalls/mount_setattr/.gitignore
index 52a77b9ca..1654f27de 100644
--- a/testcases/kernel/syscalls/mount_setattr/.gitignore
+++ b/testcases/kernel/syscalls/mount_setattr/.gitignore
@@ -1 +1,2 @@
 /mount_setattr01
+/mount_setattr02
diff --git a/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
new file mode 100644
index 000000000..640778f7c
--- /dev/null
+++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2025 SUSE LLC Wei Gao <wegao@suse.com>
+ */
+
+/*\
+ * This test is checking if the propagation field of the
+ * structure is handled properly.
+ *
+ * - EINVAL with propagation set to -1
+ * - When propagation is set to 0 it's not changed
+ * - MS_SHARED turns propagation on
+ * - MS_SLAVE turns propagation off
+ * - MS_PRIVATE turns propagation off
+ */
+
+#define _GNU_SOURCE
+
+#include <sys/statvfs.h>
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+#include "tst_safe_stdio.h"
+
+static char *tmpdir;
+static char slavedir[PATH_MAX];
+static int mounted;
+
+enum mount_type {
+	MOUNT_TYPE_SHARED,
+	MOUNT_TYPE_MASTER
+};
+
+static bool check_mount_type(const char *path, enum mount_type type_to_check)
+{
+	FILE *file = SAFE_FOPEN("/proc/self/mountinfo", "r");
+
+	char line[PATH_MAX];
+	bool found = false;
+
+	while (fgets(line, sizeof(line), file)) {
+		char mntpoint[PATH_MAX];
+		char opts[256];
+
+		if (sscanf(line, "%*d %*d %*d:%*d %*s %255s %*s %255s",
+					mntpoint, opts) != 2)
+			continue;
+
+		if (strcmp(mntpoint, path) != 0)
+			continue;
+
+		switch (type_to_check) {
+		case MOUNT_TYPE_SHARED:
+			if (strstr(opts, "shared:") != NULL)
+				found = true;
+			break;
+		case MOUNT_TYPE_MASTER:
+			if (strstr(opts, "master:") != NULL)
+				found = true;
+			break;
+		default:
+			tst_res(TFAIL, "Unexpected mount_type value: %d", type_to_check);
+		}
+	}
+
+	fclose(file);
+	return found;
+}
+
+static void cleanup(void)
+{
+	if (mounted) {
+		SAFE_UMOUNT(slavedir);
+		SAFE_UMOUNT(tmpdir);
+	}
+}
+
+static void setup(void)
+{
+	tmpdir = tst_tmpdir_path();
+	sprintf(slavedir, "%s/slavedir", tmpdir);
+	SAFE_UNSHARE(CLONE_NEWNS);
+	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
+	SAFE_MOUNT("ltp-mount_setattr", tmpdir, "tmpfs", MS_NOATIME | MS_NODEV, "");
+	SAFE_MKDIR(slavedir, 0777);
+	mounted = 1;
+}
+
+static void run(void)
+{
+	struct mount_attr attr = {
+		.attr_set       = 0,
+		.attr_clr       = 0,
+	};
+
+	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(check_mount_type(tmpdir, MOUNT_TYPE_SHARED), 0);
+
+	attr.propagation = -1;
+	TST_EXP_FAIL_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)), EINVAL);
+	TST_EXP_EQ_LI(check_mount_type(tmpdir, MOUNT_TYPE_SHARED), 0);
+
+	attr.propagation = MS_SHARED;
+	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(check_mount_type(tmpdir, MOUNT_TYPE_SHARED), 1);
+
+	attr.propagation = 0;
+	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(check_mount_type(tmpdir, MOUNT_TYPE_SHARED), 1);
+
+	attr.propagation = MS_SLAVE;
+	SAFE_MOUNT(tmpdir, slavedir, "none", MS_BIND, NULL);
+	TST_EXP_PASS_SILENT(mount_setattr(-1, slavedir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(check_mount_type(slavedir, MOUNT_TYPE_MASTER), 1);
+	TST_EXP_EQ_LI(check_mount_type(slavedir, MOUNT_TYPE_SHARED), 0);
+
+	attr.propagation = MS_PRIVATE;
+	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
+	TST_EXP_EQ_LI(check_mount_type(tmpdir, MOUNT_TYPE_SHARED), 0);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+};
-- 
2.49.0


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [LTP] [PATCH v4] mount_setattr02.c: Check mount_setattr attr propagation
  2025-07-24 13:40     ` [LTP] [PATCH v4] " Wei Gao via ltp
@ 2025-07-28 15:37       ` Cyril Hrubis
  0 siblings, 0 replies; 13+ messages in thread
From: Cyril Hrubis @ 2025-07-28 15:37 UTC (permalink / raw)
  To: Wei Gao; +Cc: ltp

Hi!
Pushed with some changes, thanks.


- We have to move the SAFE_UMOUNT(tmpdir) to the run() otherwise the test
  fails to cleanup() with -i 2

- Make sure that the slavedir was mounted shared before making it slave

- Minor adjustenment to the test description

diff --git a/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
index 640778f7c..bb246ed10 100644
--- a/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
+++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
@@ -5,7 +5,7 @@

 /*\
  * This test is checking if the propagation field of the
- * structure is handled properly.
+ * mount_attr structure is handled properly.
  *
  * - EINVAL with propagation set to -1
  * - When propagation is set to 0 it's not changed
@@ -68,10 +68,8 @@ static bool check_mount_type(const char *path, enum mount_type type_to_check)

 static void cleanup(void)
 {
-       if (mounted) {
-               SAFE_UMOUNT(slavedir);
+       if (mounted)
                SAFE_UMOUNT(tmpdir);
-       }
 }

 static void setup(void)
@@ -109,9 +107,11 @@ static void run(void)

        attr.propagation = MS_SLAVE;
        SAFE_MOUNT(tmpdir, slavedir, "none", MS_BIND, NULL);
+       TST_EXP_EQ_LI(check_mount_type(slavedir, MOUNT_TYPE_SHARED), 1);
        TST_EXP_PASS_SILENT(mount_setattr(-1, slavedir, 0, &attr, sizeof(attr)));
        TST_EXP_EQ_LI(check_mount_type(slavedir, MOUNT_TYPE_MASTER), 1);
        TST_EXP_EQ_LI(check_mount_type(slavedir, MOUNT_TYPE_SHARED), 0);
+       SAFE_UMOUNT(slavedir);

        attr.propagation = MS_PRIVATE;
        TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));


-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-07-28 15:37 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-17  2:04 [LTP] [PATCH v1] mount_setattr02.c: Check mount_setattr attr.propagation Wei Gao via ltp
2025-02-18 15:18 ` Petr Vorel
2025-02-19  8:47   ` Wei Gao via ltp
2025-02-19  9:05     ` Petr Vorel
2025-02-19  9:51       ` Wei Gao via ltp
2025-02-19 11:24         ` Petr Vorel
2025-02-19  8:29 ` [LTP] [PATCH v2] " Wei Gao via ltp
2025-02-21 12:54   ` Petr Vorel
2025-02-24  1:44     ` Wei Gao via ltp
2025-03-19 11:41   ` [LTP] [PATCH v3] mount_setattr02.c: Check mount_setattr attr propagation Wei Gao via ltp
2025-07-11  9:21     ` Cyril Hrubis
2025-07-24 13:40     ` [LTP] [PATCH v4] " Wei Gao via ltp
2025-07-28 15:37       ` Cyril Hrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.