[LTP] [PATCH v7 0/6] Refactor mqns testing suite

[LTP] [PATCH v7 0/6] Refactor mqns testing suite

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Refactoring mqns testing suite and removing libclone dependency.

Andrea Cervesato (6):
  Refactor mqns_01 using new LTP API
  Refactor mqns_02 using new LTP API
  Refactor mqns_03 using new LTP API
  Refactor mqns_04 using new LTP API
  Remove deprecated header files from mqns suite
  Remove libclone dependency from mqns suite

 runtest/containers                            |  14 +-
 testcases/kernel/containers/mqns/Makefile     |  27 +-
 testcases/kernel/containers/mqns/mqns.h       |  11 -
 testcases/kernel/containers/mqns/mqns_01.c    | 185 ++++------
 testcases/kernel/containers/mqns/mqns_02.c    | 245 +++++---------
 testcases/kernel/containers/mqns/mqns_03.c    | 320 ++++++++----------
 testcases/kernel/containers/mqns/mqns_04.c    | 291 ++++++++--------
 .../kernel/containers/mqns/mqns_helper.h      |  53 ---
 8 files changed, 441 insertions(+), 705 deletions(-)
 delete mode 100644 testcases/kernel/containers/mqns/mqns.h
 delete mode 100644 testcases/kernel/containers/mqns/mqns_helper.h

-- 
2.35.3


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

[LTP] [PATCH v7 1/6] Refactor mqns_01 using new LTP API

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/containers                         |   3 +-
 testcases/kernel/containers/mqns/mqns_01.c | 185 +++++++--------------
 2 files changed, 62 insertions(+), 126 deletions(-)

diff --git a/runtest/containers b/runtest/containers
index 23f394579..5e1b53bfa 100644
--- a/runtest/containers
+++ b/runtest/containers
@@ -16,7 +16,8 @@ pidns31 pidns31
 pidns32 pidns32
 
 mqns_01 mqns_01
-mqns_01_clone mqns_01 -clone
+mqns_01_clone mqns_01 -m clone
+mqns_01_unshare mqns_01 -m unshare
 mqns_02 mqns_02
 mqns_02_clone mqns_02 -clone
 mqns_03 mqns_03
diff --git a/testcases/kernel/containers/mqns/mqns_01.c b/testcases/kernel/containers/mqns/mqns_01.c
index 1d109e020..80e248503 100644
--- a/testcases/kernel/containers/mqns/mqns_01.c
+++ b/testcases/kernel/containers/mqns/mqns_01.c
@@ -1,148 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
-* Copyright (c) International Business Machines Corp., 2009
-* Copyright (c) Nadia Derbey, 2009
-* This program is free software; you can redistribute it and/or modify
-* it under the terms of the GNU General Public License as published by
-* the Free Software Foundation; either version 2 of the License, or
-* (at your option) any later version.
-*
-* This program is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
-* the GNU General Public License for more details.
-* You should have received a copy of the GNU General Public License
-* along with this program; if not, write to the Free Software
-* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-*
-* Author: Nadia Derbey <Nadia.Derbey@bull.net>
-*
-* Check mqns isolation: father mqns cannot be accessed from newinstance
-*
-* Mount mqueue fs
-* Create a posix mq -->mq1
-* unshare
-* In unshared process:
-*    Mount newinstance mqueuefs
-*    Check that mq1 is not readable from new ns
+ * Copyright (c) International Business Machines Corp., 2009
+ * Copyright (c) Nadia Derbey, 2009 <Nadia.Derbey@bull.net>
+ * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
 
-***************************************************************************/
+/*\
+ * [Description]
+ *
+ * Create a mqueue inside the parent and check if it can be accessed from
+ * the child namespace. Isolated and unshared process can't access to parent,
+ * but plain process can.
+ */
 
-#ifndef _GNU_SOURCE
-#define _GNU_SOURCE
-#endif
-#include <sys/wait.h>
-#include <errno.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-#include "mqns.h"
-#include "mqns_helper.h"
+#include "tst_test.h"
+#include "lapi/sched.h"
+#include "tst_safe_posix_ipc.h"
 
-char *TCID = "posixmq_namespace_01";
-int TST_TOTAL = 1;
+#define MQNAME "/MQ1"
 
-int p1[2];
-int p2[2];
+static mqd_t mqd;
+static char *str_op;
 
-int check_mqueue(void *vtest)
+static void run(void)
 {
-	char buf[30];
-	mqd_t mqd;
+	const struct tst_clone_args clone_args = { CLONE_NEWIPC, SIGCHLD };
 
-	(void) vtest;
+	tst_res(TINFO, "Checking namespaces isolation from parent to child");
 
-	close(p1[1]);
-	close(p2[0]);
+	if (str_op && !strcmp(str_op, "clone")) {
+		tst_res(TINFO, "Spawning isolated process");
 
-	if (read(p1[0], buf, strlen("go") + 1) < 0) {
-		printf("read(p1[0], ...) failed: %s\n", strerror(errno));
-		exit(1);
-	}
-	mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDONLY);
-	if (mqd == -1) {
-		if (write(p2[1], "notfnd", strlen("notfnd") + 1) < 0) {
-			perror("write(p2[1], ...) failed");
-			exit(1);
+		if (!SAFE_CLONE(&clone_args)) {
+			TST_EXP_FAIL(mq_open(MQNAME, O_RDONLY), ENOENT);
+			return;
+		}
+	} else if (str_op && !strcmp(str_op, "unshare")) {
+		tst_res(TINFO, "Spawning unshared process");
+
+		if (!SAFE_FORK()) {
+			SAFE_UNSHARE(CLONE_NEWIPC);
+			TST_EXP_FAIL(mq_open(MQNAME, O_RDONLY), ENOENT);
+			return;
 		}
 	} else {
-		if (write(p2[1], "exists", strlen("exists") + 1) < 0) {
-			perror("write(p2[1], \"exists\", 7) failed");
-			exit(1);
-		} else if (mq_close(mqd) < 0) {
-			perror("mq_close(mqd) failed");
-			exit(1);
+		tst_res(TINFO, "Spawning plain process");
+
+		if (!SAFE_FORK()) {
+			TST_EXP_POSITIVE(mq_open(MQNAME, O_RDONLY));
+			return;
 		}
 	}
-
-	exit(0);
 }
 
 static void setup(void)
 {
-	tst_require_root();
-	check_mqns();
+	mqd = SAFE_MQ_OPEN(MQNAME, O_RDWR | O_CREAT | O_EXCL, 0777, NULL);
 }
 
-int main(int argc, char *argv[])
+static void cleanup(void)
 {
-	int r;
-	mqd_t mqd;
-	char buf[30];
-	int use_clone = T_UNSHARE;
-
-	setup();
-
-	if (argc == 2 && strcmp(argv[1], "-clone") == 0) {
-		tst_resm(TINFO,
-			 "Testing posix mq namespaces through clone(2).");
-		use_clone = T_CLONE;
-	} else
-		tst_resm(TINFO,
-			 "Testing posix mq namespaces through unshare(2).");
-
-	if (pipe(p1) == -1 || pipe(p2) == -1) {
-		tst_brkm(TBROK | TERRNO, NULL, "pipe failed");
-	}
-
-	mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDWR | O_CREAT | O_EXCL,
-		0777, NULL);
-	if (mqd == -1) {
-		perror("mq_open");
-		tst_brkm(TFAIL, NULL, "mq_open failed");
+	if (mqd != -1) {
+		SAFE_MQ_CLOSE(mqd);
+		SAFE_MQ_UNLINK(MQNAME);
 	}
-
-	tst_resm(TINFO, "Checking namespaces isolation from parent to child");
-	/* fire off the test */
-	r = do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_mqueue, NULL);
-	if (r < 0) {
-		tst_resm(TFAIL, "failed clone/unshare");
-		mq_close(mqd);
-		tst_syscall(__NR_mq_unlink, NOSLASH_MQ1);
-		tst_exit();
-	}
-
-	close(p1[0]);
-	close(p2[1]);
-	if (write(p1[1], "go", strlen("go") + 1) < 0)
-		tst_resm(TBROK | TERRNO, "write(p1[1], \"go\", ...) failed");
-	else if (read(p2[0], buf, 7) < 0)
-		tst_resm(TBROK | TERRNO, "read(p2[0], buf, ...) failed");
-	else {
-		if (!strcmp(buf, "exists")) {
-			tst_resm(TFAIL, "child process found mqueue");
-		} else if (!strcmp(buf, "notfnd")) {
-			tst_resm(TPASS, "child process didn't find mqueue");
-		} else {
-			tst_resm(TFAIL, "UNKNOWN RESULT");
-		}
-	}
-
-	/* destroy the mqueue */
-	if (mq_close(mqd) == -1) {
-		tst_brkm(TBROK | TERRNO, NULL, "mq_close failed");
-	}
-	tst_syscall(__NR_mq_unlink, NOSLASH_MQ1);
-
-	tst_exit();
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.forks_child = 1,
+	.options = (struct tst_option[]) {
+		{ "m:", &str_op, "Child process isolation <clone|unshare>" },
+		{},
+	},
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_USER_NS",
+		NULL
+	},
+};
-- 
2.35.3


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

[LTP] [PATCH v7 2/6] Refactor mqns_02 using new LTP API

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/containers                         |   3 +-
 testcases/kernel/containers/mqns/mqns_02.c | 245 ++++++++-------------
 2 files changed, 90 insertions(+), 158 deletions(-)

diff --git a/runtest/containers b/runtest/containers
index 5e1b53bfa..dbb4c5fa6 100644
--- a/runtest/containers
+++ b/runtest/containers
@@ -19,7 +19,8 @@ mqns_01 mqns_01
 mqns_01_clone mqns_01 -m clone
 mqns_01_unshare mqns_01 -m unshare
 mqns_02 mqns_02
-mqns_02_clone mqns_02 -clone
+mqns_02_clone mqns_02 -m clone
+mqns_02_unshare mqns_02 -m unshare
 mqns_03 mqns_03
 mqns_03_clone mqns_03 -clone
 mqns_04 mqns_04
diff --git a/testcases/kernel/containers/mqns/mqns_02.c b/testcases/kernel/containers/mqns/mqns_02.c
index d4e785b59..9291787be 100644
--- a/testcases/kernel/containers/mqns/mqns_02.c
+++ b/testcases/kernel/containers/mqns/mqns_02.c
@@ -1,180 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
-* Copyright (c) International Business Machines Corp., 2009
-* Copyright (c) Nadia Derbey, 2009
-* This program is free software; you can redistribute it and/or modify
-* it under the terms of the GNU General Public License as published by
-* the Free Software Foundation; either version 2 of the License, or
-* (at your option) any later version.
-*
-* This program is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
-* the GNU General Public License for more details.
-* You should have received a copy of the GNU General Public License
-* along with this program; if not, write to the Free Software
-* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-*
-* Author: Nadia Derbey <Nadia.Derbey@bull.net>
-*
-* Check mqns isolation: child mqns cannot be accessed from father
-*
-* Mount mqueue fs
-* unshare
-* In unshared process:
-*    Mount newinstance mqueuefs
-*    Create a posix mq -->mq1
-* Check that mq1 is not readable from father
-*
-* Changelog:
-*	Dec 16: accomodate new mqns semantics (Serge Hallyn)
-
-***************************************************************************/
-
-#ifndef _GNU_SOURCE
-#define _GNU_SOURCE
-#endif
-#include <sys/wait.h>
-#include <errno.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-#include "mqns.h"
-#include "mqns_helper.h"
-
-char *TCID = "posixmq_namespace_02";
-int TST_TOTAL = 1;
-
-int p1[2];
-int p2[2];
-
-int check_mqueue(void *vtest)
-{
-	char buf[30];
-	mqd_t mqd;
+ * Copyright (c) International Business Machines Corp., 2009
+ * Copyright (c) Nadia Derbey, 2009 <Nadia.Derbey@bull.net>
+ * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
 
-	(void) vtest;
+/*\
+ * [Description]
+ *
+ * Create a mqueue with the same name in both parent and isolated/forked child,
+ * then check namespace isolation.
+ */
 
-	close(p1[1]);
-	close(p2[0]);
+#include "tst_test.h"
+#include "lapi/sched.h"
+#include "tst_safe_posix_ipc.h"
 
-	if (read(p1[0], buf, 3) < 0) {
-		perror("read(p1[0], ..) failed");
-		exit(1);
-	} else {
+#define MQNAME "/MQ1"
 
-		mqd =
-		    tst_syscall(__NR_mq_open, NOSLASH_MQ1,
-			    O_RDWR | O_CREAT | O_EXCL, 0777, NULL);
-		if (mqd == -1) {
-			if (write(p2[1], "mqfail", strlen("mqfail") + 1) < 0) {
-				perror("write(p2[1], \"mqfail\", ..) failed");
-				exit(1);
-			}
-		} else {
-
-			if (write(p2[1], "mqopen", strlen("mqopen") + 1) < 0) {
-				perror("write(p2[1], \"mqopen\", ..) failed");
-				exit(1);
-			} else {
-
-				if (read(p1[0], buf, 5) < 0) {
-					perror("read(p1[0], ..) failed");
-					exit(1);
-				} else {
-
-					/* destroy the mqueue */
-					if (mq_close(mqd) < 0) {
-						perror("mq_close(mqd) failed");
-						exit(1);
-					} else if (tst_syscall(__NR_mq_unlink,
-							   NOSLASH_MQ1) < 0) {
-						perror("mq_unlink(" NOSLASH_MQ1
-						       ") failed");
-						exit(1);
-					} else if (write(p2[1], "done",
-							 strlen("done") + 1)
-						   < 0) {
-						perror("write(p2[1], "
-						       "\"done\", ..) failed");
-						exit(1);
-					}
-
-				}
-
-			}
+static mqd_t mqd;
+static char *str_op;
 
-		}
+static int create_message_queue(void)
+{
+	return mq_open(MQNAME, O_RDWR | O_CREAT | O_EXCL, 0777, NULL);
+}
 
-	}
-	exit(0);
+static void shared_child(void)
+{
+	mqd_t mqd1 = -1;
 
+	TST_EXP_FAIL(mqd1 = create_message_queue(), EEXIST);
+
+	if (mqd1 != -1) {
+		SAFE_MQ_CLOSE(mqd1);
+		SAFE_MQ_UNLINK(MQNAME);
+	}
 }
 
-static void setup(void)
+static void isolated_child(void)
 {
-	tst_require_root();
-	check_mqns();
+	mqd_t mqd1 = -1;
+
+	TST_EXP_POSITIVE(mqd1 = create_message_queue());
+
+	if (mqd1 != -1) {
+		SAFE_MQ_CLOSE(mqd1);
+		SAFE_MQ_UNLINK(MQNAME);
+	}
 }
 
-int main(int argc, char *argv[])
+static void run(void)
 {
-	int r;
-	mqd_t mqd;
-	char buf[30];
-	int use_clone = T_UNSHARE;
-
-	setup();
-
-	if (argc == 2 && strcmp(argv[1], "-clone") == 0) {
-		tst_resm(TINFO,
-			 "Testing posix mq namespaces through clone(2).");
-		use_clone = T_CLONE;
-	} else
-		tst_resm(TINFO,
-			 "Testing posix mq namespaces through unshare(2).");
-
-	if (pipe(p1) == -1 || pipe(p2) == -1) {
-		tst_brkm(TBROK | TERRNO, NULL, "pipe");
-	}
+	const struct tst_clone_args clone_args = { CLONE_NEWIPC, SIGCHLD };
 
-	/* fire off the test */
-	r = do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_mqueue, NULL);
-	if (r < 0) {
-		tst_brkm(TFAIL, NULL, "failed clone/unshare");
-	}
+	tst_res(TINFO, "Checking namespaces isolation from parent to child");
 
-	tst_resm(TINFO, "Checking namespaces isolation (child to parent)");
+	if (str_op && !strcmp(str_op, "clone")) {
+		tst_res(TINFO, "Spawning isolated process");
 
-	close(p1[0]);
-	close(p2[1]);
-	if (write(p1[1], "go", strlen("go") + 1) < 0) {
-		tst_brkm(TBROK, NULL, "write(p1[1], \"go\", ..) failed");
-	}
+		if (!SAFE_CLONE(&clone_args)) {
+			isolated_child();
+			return;
+		}
+	} else if (str_op && !strcmp(str_op, "unshare")) {
+		tst_res(TINFO, "Spawning unshared process");
 
-	if (read(p2[0], buf, 7) < 0) {
-		tst_resm(TBROK | TERRNO, "read(p2[0], ..) failed");
-	} else if (!strcmp(buf, "mqfail")) {
-		tst_resm(TFAIL, "child process could not create mqueue");
-		umount(DEV_MQUEUE);
-	} else if (strcmp(buf, "mqopen")) {
-		tst_resm(TFAIL, "child process could not create mqueue");
-		umount(DEV_MQUEUE);
-	} else {
-		mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDONLY);
-		if (mqd == -1) {
-			tst_resm(TPASS,
-				 "Parent process can't see the mqueue");
-		} else {
-			tst_resm(TFAIL | TERRNO,
-				 "Parent process found mqueue");
-			mq_close(mqd);
+		if (!SAFE_FORK()) {
+			SAFE_UNSHARE(CLONE_NEWIPC);
+			isolated_child();
+			return;
 		}
-		if (write(p1[1], "cont", 5) < 0) {
-			tst_resm(TBROK | TERRNO, "write(p1[1], ..) failed");
+	} else {
+		tst_res(TINFO, "Spawning plain process");
+
+		if (!SAFE_FORK()) {
+			shared_child();
+			return;
 		}
-		read(p2[0], buf, 7);
 	}
+}
 
-	tst_exit();
+static void setup(void)
+{
+	mqd = SAFE_MQ_OPEN(MQNAME, O_RDWR | O_CREAT | O_EXCL, 0777, NULL);
 }
+
+static void cleanup(void)
+{
+	if (mqd != -1) {
+		SAFE_MQ_CLOSE(mqd);
+		SAFE_MQ_UNLINK(MQNAME);
+	}
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.forks_child = 1,
+	.options = (struct tst_option[]) {
+		{ "m:", &str_op, "Child process isolation <clone|unshare>" },
+		{},
+	},
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_USER_NS",
+		NULL
+	},
+};
-- 
2.35.3


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

[LTP] [PATCH v7 3/6] Refactor mqns_03 using new LTP API

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/containers                         |   4 +-
 testcases/kernel/containers/mqns/mqns_03.c | 320 +++++++++------------
 2 files changed, 145 insertions(+), 179 deletions(-)

diff --git a/runtest/containers b/runtest/containers
index dbb4c5fa6..07bcc8a19 100644
--- a/runtest/containers
+++ b/runtest/containers
@@ -21,8 +21,8 @@ mqns_01_unshare mqns_01 -m unshare
 mqns_02 mqns_02
 mqns_02_clone mqns_02 -m clone
 mqns_02_unshare mqns_02 -m unshare
-mqns_03 mqns_03
-mqns_03_clone mqns_03 -clone
+mqns_02_clone mqns_03 -m clone
+mqns_02_unshare mqns_03 -m unshare
 mqns_04 mqns_04
 mqns_04_clone mqns_04 -clone
 
diff --git a/testcases/kernel/containers/mqns/mqns_03.c b/testcases/kernel/containers/mqns/mqns_03.c
index a7452b970..81dea7d38 100644
--- a/testcases/kernel/containers/mqns/mqns_03.c
+++ b/testcases/kernel/containers/mqns/mqns_03.c
@@ -1,207 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
-* Copyright (c) International Business Machines Corp., 2009
-* This program is free software; you can redistribute it and/or modify
-* it under the terms of the GNU General Public License as published by
-* the Free Software Foundation; either version 2 of the License, or
-* (at your option) any later version.
-*
-* This program is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
-* the GNU General Public License for more details.
-* You should have received a copy of the GNU General Public License
-* along with this program; if not, write to the Free Software
-* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-*
-* Author: Serge Hallyn <serue@us.ibm.com>
-*
-* Check ipcns+sb longevity
-*
-* Mount mqueue fs
-* unshare
-* In unshared process:
-*    Create "/mq1" with mq_open()
-*    Mount mqueuefs
-*    Check that /mq1 exists
-*    Create /dev/mqueue/mq2 through vfs (create(2))
-*    Umount /dev/mqueue
-*    Remount /dev/mqueue
-*    Check that both /mq1 and /mq2 exist
-
-***************************************************************************/
-
-#ifndef _GNU_SOURCE
-#define _GNU_SOURCE
-#endif
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <sys/wait.h>
-#include <assert.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include "mqns.h"
-#include "mqns_helper.h"
-
-char *TCID = "posixmq_namespace_03";
-int TST_TOTAL = 1;
-
-int p1[2];
-int p2[2];
-
-#define FNAM1 DEV_MQUEUE2 SLASH_MQ1
-#define FNAM2 DEV_MQUEUE2 SLASH_MQ2
-
-int check_mqueue(void *vtest)
+ * Copyright (c) International Business Machines Corp., 2009
+ * Copyright (c) Serge Hallyn <serue@us.ibm.com>
+ * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Test mqueuefs from an isolated/forked process namespace.
+ *
+ * [Algorithm]
+ *
+ * - create /MQ1
+ * - mount mqueue inside the temporary folder
+ * - check for /MQ1 existance
+ * - create /MQ2 inside the temporary folder
+ * - umount
+ * - mount mqueue inside the temporary folder
+ * - check /MQ1 existance
+ * - check /MQ2 existance
+ * - umount
+ */
+
+#include "tst_test.h"
+#include "lapi/sched.h"
+#include "tst_safe_posix_ipc.h"
+#include "tst_safe_stdio.h"
+
+#define CHECK_MQ_OPEN_RET(x) ((x) >= 0 || ((x) == -1 && errno != EMFILE))
+
+#define MQNAME1 "/MQ1"
+#define MQNAME2 "/MQ2"
+
+static char *str_op;
+static char *devdir;
+static char *mqueue1;
+static char *mqueue2;
+static int *mq_freed1;
+static int *mq_freed2;
+
+static void check_mqueue(void)
 {
-	char buf[30];
-	mqd_t mqd;
 	int rc;
+	mqd_t mqd;
 	struct stat statbuf;
 
-	(void) vtest;
+	tst_res(TINFO, "Creating %s mqueue from within child process", MQNAME1);
 
-	close(p1[1]);
-	close(p2[0]);
+	mqd = TST_RETRY_FUNC(
+		mq_open(MQNAME1, O_RDWR | O_CREAT | O_EXCL, 0777, NULL),
+		CHECK_MQ_OPEN_RET);
+	if (mqd == -1)
+		tst_brk(TBROK | TERRNO, "mq_open failed");
 
-	if (read(p1[0], buf, 3) != 3) {	/* go */
-		perror("read failed");
-		exit(1);
-	}
+	SAFE_MQ_CLOSE(mqd);
+	tst_atomic_inc(mq_freed1);
 
-	mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDWR | O_CREAT | O_EXCL,
-		0755, NULL);
-	if (mqd == -1) {
-		write(p2[1], "mqfail", 7);
-		exit(1);
-	}
+	tst_res(TINFO, "Mount %s from within child process", devdir);
 
-	mq_close(mqd);
+	SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL);
 
-	rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL);
-	if (rc == -1) {
-		write(p2[1], "mount1", 7);
-		exit(1);
-	}
+	SAFE_STAT(mqueue1, &statbuf);
+	tst_res(TPASS, "%s exists at first mount", mqueue1);
 
-	rc = stat(FNAM1, &statbuf);
-	if (rc == -1) {
-		write(p2[1], "stat1", 6);
-		exit(1);
-	}
+	tst_res(TINFO, "Creating %s from within child process", mqueue2);
 
-	rc = creat(FNAM2, 0755);
-	if (rc == -1) {
-		write(p2[1], "creat", 6);
-		exit(1);
-	}
+	rc = SAFE_CREAT(mqueue2, 0755);
+	SAFE_CLOSE(rc);
+	tst_atomic_inc(mq_freed2);
 
-	close(rc);
+	tst_res(TINFO, "Mount %s from within child process a second time", devdir);
 
-	rc = umount(DEV_MQUEUE2);
-	if (rc == -1) {
-		write(p2[1], "umount", 7);
-		exit(1);
-	}
+	SAFE_UMOUNT(devdir);
+	SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL);
 
-	rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL);
-	if (rc == -1) {
-		write(p2[1], "mount2", 7);
-		exit(1);
-	}
+	SAFE_STAT(mqueue1, &statbuf);
+	tst_res(TPASS, "%s exists at second mount", mqueue1);
 
-	rc = stat(FNAM1, &statbuf);
-	if (rc == -1) {
-		write(p2[1], "stat2", 7);
-		exit(1);
-	}
+	SAFE_STAT(mqueue2, &statbuf);
+	tst_res(TPASS, "%s exists at second mount", mqueue2);
 
-	rc = stat(FNAM2, &statbuf);
-	if (rc == -1) {
-		write(p2[1], "stat3", 7);
-		exit(1);
-	}
+	SAFE_UMOUNT(devdir);
+
+	SAFE_MQ_UNLINK(MQNAME1);
+	tst_atomic_store(0, mq_freed1);
 
-	write(p2[1], "done", 5);
+	SAFE_MQ_UNLINK(MQNAME2);
+	tst_atomic_store(0, mq_freed2);
+}
 
-	exit(0);
+static void run(void)
+{
+	const struct tst_clone_args clone_args = { CLONE_NEWIPC, SIGCHLD };
+
+	if (str_op && !strcmp(str_op, "clone")) {
+		tst_res(TINFO, "Spawning isolated process");
+
+		if (!SAFE_CLONE(&clone_args)) {
+			check_mqueue();
+			return;
+		}
+	} else if (str_op && !strcmp(str_op, "unshare")) {
+		tst_res(TINFO, "Spawning unshared process");
+
+		if (!SAFE_FORK()) {
+			SAFE_UNSHARE(CLONE_NEWIPC);
+			check_mqueue();
+			return;
+		}
+	}
 }
 
 static void setup(void)
 {
-	tst_require_root();
-	check_mqns();
+	char *tmpdir;
+
+	if (!str_op)
+		tst_brk(TCONF, "Please specify clone|unshare child isolation");
+
+	tmpdir = tst_get_tmpdir();
+
+	SAFE_ASPRINTF(&devdir, "%s/mqueue", tmpdir);
+	SAFE_MKDIR(devdir, 0755);
+
+	SAFE_ASPRINTF(&mqueue1, "%s" MQNAME1, devdir);
+	SAFE_ASPRINTF(&mqueue2, "%s" MQNAME2, devdir);
+
+	mq_freed1 = SAFE_MMAP(NULL,
+		sizeof(int),
+		PROT_READ | PROT_WRITE,
+		MAP_SHARED | MAP_ANONYMOUS,
+		-1, 0);
+
+	mq_freed2 = SAFE_MMAP(NULL,
+		sizeof(int),
+		PROT_READ | PROT_WRITE,
+		MAP_SHARED | MAP_ANONYMOUS,
+		-1, 0);
 }
 
-int main(int argc, char *argv[])
+static void cleanup(void)
 {
-	int r;
-	char buf[30];
-	int use_clone = T_UNSHARE;
-
-	setup();
-
-	if (argc == 2 && strcmp(argv[1], "-clone") == 0) {
-		tst_resm(TINFO, "Testing posix mq namespaces through clone(2)");
-		use_clone = T_CLONE;
-	} else
-		tst_resm(TINFO,
-			 "Testing posix mq namespaces through unshare(2)");
-
-	if (pipe(p1) == -1 || pipe(p2) == -1)
-		tst_brkm(TBROK | TERRNO, NULL, "pipe failed");
-
-	/* fire off the test */
-	r = do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_mqueue, NULL);
-	if (r < 0) {
-		tst_brkm(TBROK | TERRNO, NULL, "failed clone/unshare");
-	}
+	if (!devdir)
+		return;
 
-	tst_resm(TINFO, "Checking correct umount+remount of mqueuefs");
-
-	mkdir(DEV_MQUEUE2, 0755);
-
-	close(p1[0]);
-	close(p2[1]);
-	write(p1[1], "go", 3);
-
-	read(p2[0], buf, 7);
-	r = TFAIL;
-	if (!strcmp(buf, "mqfail")) {
-		tst_resm(TFAIL, "child process could not create mqueue");
-		goto fail;
-	} else if (!strcmp(buf, "mount1")) {
-		tst_resm(TFAIL, "child process could not mount mqueue");
-		goto fail;
-	} else if (!strcmp(buf, "stat1x")) {
-		tst_resm(TFAIL, "mq created by child is not in mqueuefs");
-		goto fail;
-	} else if (!strcmp(buf, "creat")) {
-		tst_resm(TFAIL, "child couldn't creat mq through mqueuefs");
-		goto fail;
-	} else if (!strcmp(buf, "umount")) {
-		tst_resm(TFAIL, "child couldn't umount mqueuefs");
-		goto fail;
-	} else if (!strcmp(buf, "mount2")) {
-		tst_resm(TFAIL, "child couldn't remount mqueuefs");
-		goto fail;
-	} else if (!strcmp(buf, "stat2")) {
-		tst_resm(TFAIL,
-			 "mq_open()d file gone after remount of mqueuefs");
-		goto fail;
-	} else if (!strcmp(buf, "stat3")) {
-		tst_resm(TFAIL,
-			 "creat(2)'d file gone after remount of mqueuefs");
-		goto fail;
-	}
+	if (tst_is_mounted(devdir))
+		SAFE_UMOUNT(devdir);
 
-	tst_resm(TPASS, "umount+remount of mqueuefs remounted the right fs");
+	if (*mq_freed1)
+		SAFE_MQ_UNLINK(MQNAME1);
 
-	r = 0;
-fail:
-	umount(DEV_MQUEUE2);
-	rmdir(DEV_MQUEUE2);
-	tst_exit();
+	if (*mq_freed2)
+		SAFE_MQ_UNLINK(MQNAME2);
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.forks_child = 1,
+	.needs_tmpdir = 1,
+	.options = (struct tst_option[]) {
+		{ "m:", &str_op, "Child process isolation <clone|unshare>" },
+		{},
+	},
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_USER_NS",
+		NULL
+	},
+};
-- 
2.35.3


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

[LTP] [PATCH v7 4/6] Refactor mqns_04 using new LTP API

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/containers                         |   4 +-
 testcases/kernel/containers/mqns/mqns_04.c | 291 ++++++++++-----------
 2 files changed, 139 insertions(+), 156 deletions(-)

diff --git a/runtest/containers b/runtest/containers
index 07bcc8a19..c88e3f777 100644
--- a/runtest/containers
+++ b/runtest/containers
@@ -23,8 +23,8 @@ mqns_02_clone mqns_02 -m clone
 mqns_02_unshare mqns_02 -m unshare
 mqns_02_clone mqns_03 -m clone
 mqns_02_unshare mqns_03 -m unshare
-mqns_04 mqns_04
-mqns_04_clone mqns_04 -clone
+mqns_02_clone mqns_04 -m clone
+mqns_02_unshare mqns_04 -m unshare
 
 netns_netlink netns_netlink
 netns_breakns_ip_ipv4_netlink netns_breakns.sh
diff --git a/testcases/kernel/containers/mqns/mqns_04.c b/testcases/kernel/containers/mqns/mqns_04.c
index d07a85c04..75c54259c 100644
--- a/testcases/kernel/containers/mqns/mqns_04.c
+++ b/testcases/kernel/containers/mqns/mqns_04.c
@@ -1,187 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
-* Copyright (c) International Business Machines Corp., 2009
-* This program is free software; you can redistribute it and/or modify
-* it under the terms of the GNU General Public License as published by
-* the Free Software Foundation; either version 2 of the License, or
-* (at your option) any later version.
-*
-* This program is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
-* the GNU General Public License for more details.
-* You should have received a copy of the GNU General Public License
-* along with this program; if not, write to the Free Software
-* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-*
-* Author: Serge Hallyn <serue@us.ibm.com>
-*
-* Check mqueuefs lifetime
-* . parent creates /dev/mqueue2
-* . child mounts mqueue there
-* . child does mq_open("/ab")
-* . parent checks for /dev/mqueue2
-* . child exits
-* . parent checks for /dev/mqueue2
-* . parent tries 'touch /dev/mqueue2/dd' -> should fail
-* . parent umounts /dev/mqueue2
-
-***************************************************************************/
-
-#ifndef _GNU_SOURCE
-#define _GNU_SOURCE
-#endif
-#include <sys/types.h>
-#include <sys/stat.h>
+ * Copyright (c) International Business Machines Corp., 2009
+ * Copyright (c) Serge Hallyn <serue@us.ibm.com>
+ * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Test mqueuefs manipulation from child/parent namespaces.
+ *
+ * [Algorithm]
+ *
+ * - parent creates mqueue folder in <tmpdir>
+ * - child mounts mqueue there
+ * - child creates /MQ1 mqueue
+ * - parent checks for <tmpdir>/mqueue/MQ1 existence
+ * - child exits
+ * - parent checks for <tmpdir>/mqueue/MQ1 existence
+ * - parent tries 'touch <tmpdir>/mqueue/MQ2' -> should fail
+ * - parent umount mqueuefs
+ */
+
 #include <sys/wait.h>
-#include <assert.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include "mqns.h"
-#include "mqns_helper.h"
+#include "tst_test.h"
+#include "lapi/sched.h"
+#include "tst_safe_posix_ipc.h"
+#include "tst_safe_stdio.h"
 
-char *TCID = "posixmq_namespace_04";
-int TST_TOTAL = 1;
+#define CHECK_MQ_OPEN_RET(x) ((x) >= 0 || ((x) == -1 && errno != EMFILE))
 
-int p1[2];
-int p2[2];
+#define MQNAME1 "/MQ1"
+#define MQNAME2 "/MQ2"
 
-#define FNAM1 DEV_MQUEUE2 SLASH_MQ1
-#define FNAM2 DEV_MQUEUE2 SLASH_MQ2
+static char *str_op;
+static char *devdir;
+static char *mqueue1;
+static char *mqueue2;
+static int *mq_freed1;
+static int *mq_freed2;
 
-int check_mqueue(void *vtest)
+static void check_mqueue(void)
 {
-	char buf[30];
 	mqd_t mqd;
-	int rc;
 
-	(void) vtest;
+	tst_res(TINFO, "Creating %s mqueue from within child process", MQNAME1);
 
-	close(p1[1]);
-	close(p2[0]);
+	mqd = TST_RETRY_FUNC(
+		mq_open(MQNAME1, O_RDWR | O_CREAT | O_EXCL, 0755, NULL),
+		CHECK_MQ_OPEN_RET);
+	if (mqd == -1)
+		tst_brk(TBROK | TERRNO, "mq_open failed");
 
-	read(p1[0], buf, 3);	/* go */
+	SAFE_MQ_CLOSE(mqd);
+	tst_atomic_inc(mq_freed1);
 
-	mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDWR | O_CREAT | O_EXCL,
-		0755, NULL);
-	if (mqd == -1) {
-		write(p2[1], "mqfail", 7);
-		tst_exit();
-	}
+	tst_res(TINFO, "Mount %s from within child process", devdir);
+
+	SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL);
+
+	TST_CHECKPOINT_WAKE_AND_WAIT(0);
+}
 
-	mq_close(mqd);
+static void run(void)
+{
+	const struct tst_clone_args clone_args = { CLONE_NEWIPC, SIGCHLD };
+	struct stat statbuf;
 
-	rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL);
-	if (rc == -1) {
-		perror("mount");
-		write(p2[1], "mount", 6);
-		tst_exit();
+	if (str_op && !strcmp(str_op, "clone")) {
+		tst_res(TINFO, "Spawning isolated process");
+
+		if (!SAFE_CLONE(&clone_args)) {
+			check_mqueue();
+			return;
+		}
+	} else if (str_op && !strcmp(str_op, "unshare")) {
+		tst_res(TINFO, "Spawning unshared process");
+
+		if (!SAFE_FORK()) {
+			SAFE_UNSHARE(CLONE_NEWIPC);
+			check_mqueue();
+			return;
+		}
 	}
 
-	write(p2[1], "go", 3);
-	read(p1[0], buf, 3);
+	TST_CHECKPOINT_WAIT(0);
 
-	tst_exit();
-}
+	SAFE_STAT(mqueue1, &statbuf);
+	tst_res(TPASS, "%s child's mqueue can be accessed from parent", mqueue1);
 
-static void setup(void)
-{
-	tst_require_root();
-	check_mqns();
-}
+	TST_CHECKPOINT_WAKE(0);
 
-int main(int argc, char *argv[])
-{
-	int rc;
-	int status;
-	char buf[30];
-	struct stat statbuf;
-	int use_clone = T_UNSHARE;
+	tst_res(TINFO, "Waiting child to exit");
 
-	setup();
+	tst_reap_children();
+	tst_atomic_dec(mq_freed1);
 
-	if (argc == 2 && strcmp(argv[1], "-clone") == 0) {
-		tst_resm(TINFO,
-			 "Testing posix mq namespaces through clone(2).");
-		use_clone = T_CLONE;
-	} else
-		tst_resm(TINFO,
-			 "Testing posix mq namespaces through unshare(2).");
+	SAFE_STAT(mqueue1, &statbuf);
+	tst_res(TPASS, "%s child's mqueue can be accessed from parent after child's dead", mqueue1);
 
-	if (pipe(p1) == -1) {
-		perror("pipe");
-		exit(EXIT_FAILURE);
-	}
-	if (pipe(p2) == -1) {
-		perror("pipe");
-		exit(EXIT_FAILURE);
-	}
+	tst_res(TINFO, "Try to create %s from parent", mqueue2);
 
-	mkdir(DEV_MQUEUE2, 0755);
+	TST_EXP_FAIL(creat(mqueue2, 0755), EACCES);
+	if (!TST_PASS)
+		tst_atomic_inc(mq_freed2);
 
-	tst_resm(TINFO, "Checking mqueue filesystem lifetime");
+	SAFE_UMOUNT(devdir);
+}
 
-	/* fire off the test */
-	rc = do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_mqueue, NULL);
-	if (rc < 0) {
-		tst_resm(TFAIL, "failed clone/unshare");
-		goto fail;
-	}
+static void setup(void)
+{
+	char *tmpdir;
 
-	close(p1[0]);
-	close(p2[1]);
-	write(p1[1], "go", 3);
-
-	read(p2[0], buf, 7);
-	if (!strcmp(buf, "mqfail")) {
-		tst_resm(TFAIL, "child process could not create mqueue");
-		goto fail;
-	} else if (!strcmp(buf, "mount")) {
-		tst_resm(TFAIL, "child process could not mount mqueue");
-		goto fail;
-	}
+	if (!str_op)
+		tst_brk(TCONF, "Please specify clone|unshare child isolation");
 
-	rc = stat(FNAM1, &statbuf);
-	if (rc == -1) {
-		perror("stat");
-		write(p1[1], "go", 3);
-		tst_resm(TFAIL, "parent could not see child's created mq");
-		goto fail;
-	}
-	write(p1[1], "go", 3);
+	tmpdir = tst_get_tmpdir();
 
-	rc = wait(&status);
-	if (rc == -1) {
-		perror("wait");
-		tst_resm(TFAIL, "error while parent waited on child to exit");
-		goto fail;
-	}
-	if (!WIFEXITED(status)) {
-		tst_resm(TFAIL, "Child did not exit normally (status %d)",
-			 status);
-		goto fail;
-	}
-	rc = stat(FNAM1, &statbuf);
-	if (rc == -1) {
-		tst_resm(TFAIL,
-			 "parent's view of child's mq died with child");
-		goto fail;
-	}
+	SAFE_ASPRINTF(&devdir, "%s/mqueue", tmpdir);
+	SAFE_MKDIR(devdir, 0755);
 
-	rc = creat(FNAM2, 0755);
-	if (rc != -1) {
-		tst_resm(TFAIL,
-			 "parent was able to create a file in dead child's mqfs");
-		goto fail;
-	}
+	SAFE_ASPRINTF(&mqueue1, "%s" MQNAME1, devdir);
+	SAFE_ASPRINTF(&mqueue2, "%s" MQNAME2, devdir);
+
+	mq_freed1 = SAFE_MMAP(NULL,
+		sizeof(int),
+		PROT_READ | PROT_WRITE,
+		MAP_SHARED | MAP_ANONYMOUS,
+		-1, 0);
 
-	tst_resm(TPASS, "Child mqueue fs still visible for parent");
+	mq_freed2 = SAFE_MMAP(NULL,
+		sizeof(int),
+		PROT_READ | PROT_WRITE,
+		MAP_SHARED | MAP_ANONYMOUS,
+		-1, 0);
+}
 
-fail:
-	umount(DEV_MQUEUE2);
-	rmdir(DEV_MQUEUE2);
+static void cleanup(void)
+{
+	if (!devdir)
+		return;
 
-	tst_exit();
+	if (tst_is_mounted(devdir))
+		SAFE_UMOUNT(devdir);
+
+	if (*mq_freed1)
+		SAFE_MQ_UNLINK(MQNAME1);
+
+	if (*mq_freed2)
+		SAFE_MQ_UNLINK(MQNAME2);
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.forks_child = 1,
+	.needs_tmpdir = 1,
+	.needs_checkpoints = 1,
+	.options = (struct tst_option[]) {
+		{ "m:", &str_op, "Child process isolation <clone|unshare>" },
+		{},
+	},
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_USER_NS",
+		NULL
+	},
+};
-- 
2.35.3


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

[LTP] [PATCH v7 5/6] Remove deprecated header files from mqns suite

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 testcases/kernel/containers/mqns/mqns.h       | 11 ----
 .../kernel/containers/mqns/mqns_helper.h      | 53 -------------------
 2 files changed, 64 deletions(-)
 delete mode 100644 testcases/kernel/containers/mqns/mqns.h
 delete mode 100644 testcases/kernel/containers/mqns/mqns_helper.h

diff --git a/testcases/kernel/containers/mqns/mqns.h b/testcases/kernel/containers/mqns/mqns.h
deleted file mode 100644
index 5a9056838..000000000
--- a/testcases/kernel/containers/mqns/mqns.h
+++ /dev/null
@@ -1,11 +0,0 @@
-#ifndef __MQNS_H
-#define __MQNS_H
-
-#define DEV_MQUEUE "/dev/mqueue"
-#define DEV_MQUEUE2 "/dev/mqueue2"
-#define SLASH_MQ1 "/MQ1"
-#define NOSLASH_MQ1 "MQ1"
-#define SLASH_MQ2 "/MQ2"
-#define NOSLASH_MQ2 "MQ2"
-
-#endif /* __MQNS_H */
diff --git a/testcases/kernel/containers/mqns/mqns_helper.h b/testcases/kernel/containers/mqns/mqns_helper.h
deleted file mode 100644
index 03f50aa36..000000000
--- a/testcases/kernel/containers/mqns/mqns_helper.h
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
- * Copyright (c) International Business Machines Corp., 2009
- * Copyright (c) Nadia Derbey, 2009
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
- * the GNU General Public License for more details.
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- *
- * Author: Serge Hallyn <serue@us.ibm.com>
- ***************************************************************************/
-#include <sys/mount.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <mqueue.h>
-#include "../libclone/libclone.h"
-#include "lapi/syscalls.h"
-#include "safe_macros.h"
-#include "test.h"
-
-static int dummy_child(void *v)
-{
-	(void) v;
-	return 0;
-}
-
-static void check_mqns(void)
-{
-	int pid, status;
-	mqd_t mqd;
-
-	mq_unlink("/checkmqnsenabled");
-	mqd =
-	    mq_open("/checkmqnsenabled", O_RDWR | O_CREAT | O_EXCL, 0777, NULL);
-	if (mqd == -1)
-		tst_brkm(TCONF, NULL, "mq_open check failed");
-
-	mq_close(mqd);
-	mq_unlink("/checkmqnsenabled");
-
-	pid = do_clone_unshare_test(T_CLONE, CLONE_NEWIPC, dummy_child, NULL);
-	if (pid == -1)
-		tst_brkm(TCONF | TERRNO, NULL, "CLONE_NEWIPC not supported");
-
-	SAFE_WAIT(NULL, &status);
-}
-- 
2.35.3


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

[LTP] [PATCH v7 6/6] Remove libclone dependency from mqns suite

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 testcases/kernel/containers/mqns/Makefile | 27 +++++------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/testcases/kernel/containers/mqns/Makefile b/testcases/kernel/containers/mqns/Makefile
index 64c3763ee..eb0f97c2b 100644
--- a/testcases/kernel/containers/mqns/Makefile
+++ b/testcases/kernel/containers/mqns/Makefile
@@ -1,29 +1,12 @@
-################################################################################
-##                                                                            ##
-## Copyright (c) International Business Machines  Corp., 2009                 ##
-## Copyright (c) Nadia Derbey, 2009                                           ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-################################################################################
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) International Business Machines  Corp., 2009
+# Copyright (c) Nadia Derbey, 2009 
+# Copyright (C) 2021 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
 
 top_srcdir		?= ../../../..
 
 include $(top_srcdir)/include/mk/testcases.mk
-include $(abs_srcdir)/../Makefile.inc
 
-LDLIBS			:= -lpthread -lrt -lclone $(LDLIBS)
+LDLIBS			:= -lpthread -lrt $(LDLIBS)
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
-- 
2.35.3


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

Re: [LTP] [PATCH v7 3/6] Refactor mqns_03 using new LTP API

[Cyril Hrubis]

Hi!
> +#include "tst_test.h"
> +#include "lapi/sched.h"
> +#include "tst_safe_posix_ipc.h"
> +#include "tst_safe_stdio.h"
> +
> +#define CHECK_MQ_OPEN_RET(x) ((x) >= 0 || ((x) == -1 && errno != EMFILE))
> +
> +#define MQNAME1 "/MQ1"
> +#define MQNAME2 "/MQ2"
> +
> +static char *str_op;
> +static char *devdir;
> +static char *mqueue1;
> +static char *mqueue2;
> +static int *mq_freed1;
> +static int *mq_freed2;
> +
> +static void check_mqueue(void)
>  {
> -	char buf[30];
> -	mqd_t mqd;
>  	int rc;
> +	mqd_t mqd;
>  	struct stat statbuf;
>  
> -	(void) vtest;
> +	tst_res(TINFO, "Creating %s mqueue from within child process", MQNAME1);
>  
> -	close(p1[1]);
> -	close(p2[0]);
> +	mqd = TST_RETRY_FUNC(
> +		mq_open(MQNAME1, O_RDWR | O_CREAT | O_EXCL, 0777, NULL),
> +		CHECK_MQ_OPEN_RET);
> +	if (mqd == -1)
> +		tst_brk(TBROK | TERRNO, "mq_open failed");
>  
> -	if (read(p1[0], buf, 3) != 3) {	/* go */
> -		perror("read failed");
> -		exit(1);
> -	}
> +	SAFE_MQ_CLOSE(mqd);
> +	tst_atomic_inc(mq_freed1);

I do not think that we need atomicity here, the cleanup code does not
run concurently at all as the cleanup in the parent is triggered after
the child did exit. I suppose that instead we need to set the mq_freed
to be volatile because it's shared memory which may change at any
change, so we need to tell that to the compiler.

> -	mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDWR | O_CREAT | O_EXCL,
> -		0755, NULL);
> -	if (mqd == -1) {
> -		write(p2[1], "mqfail", 7);
> -		exit(1);
> -	}
> +	tst_res(TINFO, "Mount %s from within child process", devdir);
>  
> -	mq_close(mqd);
> +	SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL);
>  
> -	rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL);
> -	if (rc == -1) {
> -		write(p2[1], "mount1", 7);
> -		exit(1);
> -	}
> +	SAFE_STAT(mqueue1, &statbuf);
> +	tst_res(TPASS, "%s exists at first mount", mqueue1);
>  
> -	rc = stat(FNAM1, &statbuf);
> -	if (rc == -1) {
> -		write(p2[1], "stat1", 6);
> -		exit(1);
> -	}
> +	tst_res(TINFO, "Creating %s from within child process", mqueue2);
>  
> -	rc = creat(FNAM2, 0755);
> -	if (rc == -1) {
> -		write(p2[1], "creat", 6);
> -		exit(1);
> -	}
> +	rc = SAFE_CREAT(mqueue2, 0755);
> +	SAFE_CLOSE(rc);
> +	tst_atomic_inc(mq_freed2);
>  
> -	close(rc);
> +	tst_res(TINFO, "Mount %s from within child process a second time", devdir);
>  
> -	rc = umount(DEV_MQUEUE2);
> -	if (rc == -1) {
> -		write(p2[1], "umount", 7);
> -		exit(1);
> -	}
> +	SAFE_UMOUNT(devdir);
> +	SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL);
>  
> -	rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL);
> -	if (rc == -1) {
> -		write(p2[1], "mount2", 7);
> -		exit(1);
> -	}
> +	SAFE_STAT(mqueue1, &statbuf);
> +	tst_res(TPASS, "%s exists at second mount", mqueue1);
>  
> -	rc = stat(FNAM1, &statbuf);
> -	if (rc == -1) {
> -		write(p2[1], "stat2", 7);
> -		exit(1);
> -	}
> +	SAFE_STAT(mqueue2, &statbuf);
> +	tst_res(TPASS, "%s exists at second mount", mqueue2);
>  
> -	rc = stat(FNAM2, &statbuf);
> -	if (rc == -1) {
> -		write(p2[1], "stat3", 7);
> -		exit(1);
> -	}
> +	SAFE_UMOUNT(devdir);
> +
> +	SAFE_MQ_UNLINK(MQNAME1);
> +	tst_atomic_store(0, mq_freed1);
>  
> -	write(p2[1], "done", 5);
> +	SAFE_MQ_UNLINK(MQNAME2);
> +	tst_atomic_store(0, mq_freed2);
> +}
>  
> -	exit(0);
> +static void run(void)
> +{
> +	const struct tst_clone_args clone_args = { CLONE_NEWIPC, SIGCHLD };
> +
> +	if (str_op && !strcmp(str_op, "clone")) {
> +		tst_res(TINFO, "Spawning isolated process");
> +
> +		if (!SAFE_CLONE(&clone_args)) {
> +			check_mqueue();
> +			return;
> +		}
> +	} else if (str_op && !strcmp(str_op, "unshare")) {
> +		tst_res(TINFO, "Spawning unshared process");
> +
> +		if (!SAFE_FORK()) {
> +			SAFE_UNSHARE(CLONE_NEWIPC);
> +			check_mqueue();
> +			return;
> +		}
> +	}
>  }
>  
>  static void setup(void)
>  {
> -	tst_require_root();
> -	check_mqns();
> +	char *tmpdir;
> +
> +	if (!str_op)
> +		tst_brk(TCONF, "Please specify clone|unshare child isolation");
> +
> +	tmpdir = tst_get_tmpdir();
> +
> +	SAFE_ASPRINTF(&devdir, "%s/mqueue", tmpdir);
> +	SAFE_MKDIR(devdir, 0755);
> +
> +	SAFE_ASPRINTF(&mqueue1, "%s" MQNAME1, devdir);
> +	SAFE_ASPRINTF(&mqueue2, "%s" MQNAME2, devdir);
> +
> +	mq_freed1 = SAFE_MMAP(NULL,
> +		sizeof(int),
> +		PROT_READ | PROT_WRITE,
> +		MAP_SHARED | MAP_ANONYMOUS,
> +		-1, 0);
> +
> +	mq_freed2 = SAFE_MMAP(NULL,
> +		sizeof(int),
> +		PROT_READ | PROT_WRITE,
> +		MAP_SHARED | MAP_ANONYMOUS,
> +		-1, 0);

So here you are allocating two pages of memory for something that is
basically two bitflags. Can you at least change this to a single mmap()
something as:

static int *mq_freed;

	mq_freed = SAFE_MMAP(NULL, 2 * sizeof(int), ...)


	mq_freed[0] = 1;
	...

Moreover since we can actually stat()/access() the mqueue we can as well
check for the existence of the devdir in the cleanup and only remove it
if it exists in the filesystem.

Also I would be more afraid of the mqueue filesystem being mounted in
the temp directory if we trigger a failure between one of the
mount()/umount() calls, so we should as well check if it's mounted in
the cleanup and attempt to umount it.


-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v7 1/6] Refactor mqns_01 using new LTP API

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v7 2/6] Refactor mqns_02 using new LTP API

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v7 4/6] Refactor mqns_04 using new LTP API

[Cyril Hrubis]

Hi!
The same comments as for mqns_03 apply here as well.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v7 3/6] Refactor mqns_03 using new LTP API

[Andrea Cervesato via ltp]

Hi Cyril,

On 5/22/23 16:41, Cyril Hrubis wrote:
> Hi!
>> +#include "tst_test.h"
>> +#include "lapi/sched.h"
>> +#include "tst_safe_posix_ipc.h"
>> +#include "tst_safe_stdio.h"
>> +
>> +#define CHECK_MQ_OPEN_RET(x) ((x) >= 0 || ((x) == -1 && errno != EMFILE))
>> +
>> +#define MQNAME1 "/MQ1"
>> +#define MQNAME2 "/MQ2"
>> +
>> +static char *str_op;
>> +static char *devdir;
>> +static char *mqueue1;
>> +static char *mqueue2;
>> +static int *mq_freed1;
>> +static int *mq_freed2;
>> +
>> +static void check_mqueue(void)
>>   {
>> -	char buf[30];
>> -	mqd_t mqd;
>>   	int rc;
>> +	mqd_t mqd;
>>   	struct stat statbuf;
>>   
>> -	(void) vtest;
>> +	tst_res(TINFO, "Creating %s mqueue from within child process", MQNAME1);
>>   
>> -	close(p1[1]);
>> -	close(p2[0]);
>> +	mqd = TST_RETRY_FUNC(
>> +		mq_open(MQNAME1, O_RDWR | O_CREAT | O_EXCL, 0777, NULL),
>> +		CHECK_MQ_OPEN_RET);
>> +	if (mqd == -1)
>> +		tst_brk(TBROK | TERRNO, "mq_open failed");
>>   
>> -	if (read(p1[0], buf, 3) != 3) {	/* go */
>> -		perror("read failed");
>> -		exit(1);
>> -	}
>> +	SAFE_MQ_CLOSE(mqd);
>> +	tst_atomic_inc(mq_freed1);
> I do not think that we need atomicity here, the cleanup code does not
> run concurently at all as the cleanup in the parent is triggered after
> the child did exit. I suppose that instead we need to set the mq_freed
> to be volatile because it's shared memory which may change at any
> change, so we need to tell that to the compiler.
That's fine, but I followed suggestions in the reviews. I think that 
having 3 people reviewing the same patch doesn't help the development 
process. Now I'm not sure who I should follow :-)
>> -	mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDWR | O_CREAT | O_EXCL,
>> -		0755, NULL);
>> -	if (mqd == -1) {
>> -		write(p2[1], "mqfail", 7);
>> -		exit(1);
>> -	}
>> +	tst_res(TINFO, "Mount %s from within child process", devdir);
>>   
>> -	mq_close(mqd);
>> +	SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL);
>>   
>> -	rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL);
>> -	if (rc == -1) {
>> -		write(p2[1], "mount1", 7);
>> -		exit(1);
>> -	}
>> +	SAFE_STAT(mqueue1, &statbuf);
>> +	tst_res(TPASS, "%s exists at first mount", mqueue1);
>>   
>> -	rc = stat(FNAM1, &statbuf);
>> -	if (rc == -1) {
>> -		write(p2[1], "stat1", 6);
>> -		exit(1);
>> -	}
>> +	tst_res(TINFO, "Creating %s from within child process", mqueue2);
>>   
>> -	rc = creat(FNAM2, 0755);
>> -	if (rc == -1) {
>> -		write(p2[1], "creat", 6);
>> -		exit(1);
>> -	}
>> +	rc = SAFE_CREAT(mqueue2, 0755);
>> +	SAFE_CLOSE(rc);
>> +	tst_atomic_inc(mq_freed2);
>>   
>> -	close(rc);
>> +	tst_res(TINFO, "Mount %s from within child process a second time", devdir);
>>   
>> -	rc = umount(DEV_MQUEUE2);
>> -	if (rc == -1) {
>> -		write(p2[1], "umount", 7);
>> -		exit(1);
>> -	}
>> +	SAFE_UMOUNT(devdir);
>> +	SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL);
>>   
>> -	rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL);
>> -	if (rc == -1) {
>> -		write(p2[1], "mount2", 7);
>> -		exit(1);
>> -	}
>> +	SAFE_STAT(mqueue1, &statbuf);
>> +	tst_res(TPASS, "%s exists at second mount", mqueue1);
>>   
>> -	rc = stat(FNAM1, &statbuf);
>> -	if (rc == -1) {
>> -		write(p2[1], "stat2", 7);
>> -		exit(1);
>> -	}
>> +	SAFE_STAT(mqueue2, &statbuf);
>> +	tst_res(TPASS, "%s exists at second mount", mqueue2);
>>   
>> -	rc = stat(FNAM2, &statbuf);
>> -	if (rc == -1) {
>> -		write(p2[1], "stat3", 7);
>> -		exit(1);
>> -	}
>> +	SAFE_UMOUNT(devdir);
>> +
>> +	SAFE_MQ_UNLINK(MQNAME1);
>> +	tst_atomic_store(0, mq_freed1);
>>   
>> -	write(p2[1], "done", 5);
>> +	SAFE_MQ_UNLINK(MQNAME2);
>> +	tst_atomic_store(0, mq_freed2);
>> +}
>>   
>> -	exit(0);
>> +static void run(void)
>> +{
>> +	const struct tst_clone_args clone_args = { CLONE_NEWIPC, SIGCHLD };
>> +
>> +	if (str_op && !strcmp(str_op, "clone")) {
>> +		tst_res(TINFO, "Spawning isolated process");
>> +
>> +		if (!SAFE_CLONE(&clone_args)) {
>> +			check_mqueue();
>> +			return;
>> +		}
>> +	} else if (str_op && !strcmp(str_op, "unshare")) {
>> +		tst_res(TINFO, "Spawning unshared process");
>> +
>> +		if (!SAFE_FORK()) {
>> +			SAFE_UNSHARE(CLONE_NEWIPC);
>> +			check_mqueue();
>> +			return;
>> +		}
>> +	}
>>   }
>>   
>>   static void setup(void)
>>   {
>> -	tst_require_root();
>> -	check_mqns();
>> +	char *tmpdir;
>> +
>> +	if (!str_op)
>> +		tst_brk(TCONF, "Please specify clone|unshare child isolation");
>> +
>> +	tmpdir = tst_get_tmpdir();
>> +
>> +	SAFE_ASPRINTF(&devdir, "%s/mqueue", tmpdir);
>> +	SAFE_MKDIR(devdir, 0755);
>> +
>> +	SAFE_ASPRINTF(&mqueue1, "%s" MQNAME1, devdir);
>> +	SAFE_ASPRINTF(&mqueue2, "%s" MQNAME2, devdir);
>> +
>> +	mq_freed1 = SAFE_MMAP(NULL,
>> +		sizeof(int),
>> +		PROT_READ | PROT_WRITE,
>> +		MAP_SHARED | MAP_ANONYMOUS,
>> +		-1, 0);
>> +
>> +	mq_freed2 = SAFE_MMAP(NULL,
>> +		sizeof(int),
>> +		PROT_READ | PROT_WRITE,
>> +		MAP_SHARED | MAP_ANONYMOUS,
>> +		-1, 0);
> So here you are allocating two pages of memory for something that is
> basically two bitflags. Can you at least change this to a single mmap()
> something as:
>
> static int *mq_freed;
>
> 	mq_freed = SAFE_MMAP(NULL, 2 * sizeof(int), ...)
>
>
> 	mq_freed[0] = 1;
> 	...
>
> Moreover since we can actually stat()/access() the mqueue we can as well
> check for the existence of the devdir in the cleanup and only remove it
> if it exists in the filesystem.
>
> Also I would be more afraid of the mqueue filesystem being mounted in
> the temp directory if we trigger a failure between one of the
> mount()/umount() calls, so we should as well check if it's mounted in
> the cleanup and attempt to umount it.
>
>
Andrea


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

Re: [LTP] [PATCH v7 3/6] Refactor mqns_03 using new LTP API

[Cyril Hrubis]

Hi!
> > I do not think that we need atomicity here, the cleanup code does not
> > run concurently at all as the cleanup in the parent is triggered after
> > the child did exit. I suppose that instead we need to set the mq_freed
> > to be volatile because it's shared memory which may change at any
> > change, so we need to tell that to the compiler.
> That's fine, but I followed suggestions in the reviews. I think that 
> having 3 people reviewing the same patch doesn't help the development 
> process. Now I'm not sure who I should follow :-)

It's actually the other way around, the more people look at the code the
better, at least that way we have potential to catch more problems
earlier. And if the reviewers disagree, let them fight for the right
answer.

I think that in this case this all can be actually simplified and we can
get rid of the mq_freed flag as I tried to outline below.

> >> -	mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDWR | O_CREAT | O_EXCL,
> >> -		0755, NULL);
> >> -	if (mqd == -1) {
> >> -		write(p2[1], "mqfail", 7);
> >> -		exit(1);
> >> -	}
> >> +	tst_res(TINFO, "Mount %s from within child process", devdir);
> >>   
> >> -	mq_close(mqd);
> >> +	SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL);
> >>   
> >> -	rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL);
> >> -	if (rc == -1) {
> >> -		write(p2[1], "mount1", 7);
> >> -		exit(1);
> >> -	}
> >> +	SAFE_STAT(mqueue1, &statbuf);
> >> +	tst_res(TPASS, "%s exists at first mount", mqueue1);
> >>   
> >> -	rc = stat(FNAM1, &statbuf);
> >> -	if (rc == -1) {
> >> -		write(p2[1], "stat1", 6);
> >> -		exit(1);
> >> -	}
> >> +	tst_res(TINFO, "Creating %s from within child process", mqueue2);
> >>   
> >> -	rc = creat(FNAM2, 0755);
> >> -	if (rc == -1) {
> >> -		write(p2[1], "creat", 6);
> >> -		exit(1);
> >> -	}
> >> +	rc = SAFE_CREAT(mqueue2, 0755);
> >> +	SAFE_CLOSE(rc);
> >> +	tst_atomic_inc(mq_freed2);
> >>   
> >> -	close(rc);
> >> +	tst_res(TINFO, "Mount %s from within child process a second time", devdir);
> >>   
> >> -	rc = umount(DEV_MQUEUE2);
> >> -	if (rc == -1) {
> >> -		write(p2[1], "umount", 7);
> >> -		exit(1);
> >> -	}
> >> +	SAFE_UMOUNT(devdir);
> >> +	SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL);
> >>   
> >> -	rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL);
> >> -	if (rc == -1) {
> >> -		write(p2[1], "mount2", 7);
> >> -		exit(1);
> >> -	}
> >> +	SAFE_STAT(mqueue1, &statbuf);
> >> +	tst_res(TPASS, "%s exists at second mount", mqueue1);
> >>   
> >> -	rc = stat(FNAM1, &statbuf);
> >> -	if (rc == -1) {
> >> -		write(p2[1], "stat2", 7);
> >> -		exit(1);
> >> -	}
> >> +	SAFE_STAT(mqueue2, &statbuf);
> >> +	tst_res(TPASS, "%s exists at second mount", mqueue2);
> >>   
> >> -	rc = stat(FNAM2, &statbuf);
> >> -	if (rc == -1) {
> >> -		write(p2[1], "stat3", 7);
> >> -		exit(1);
> >> -	}
> >> +	SAFE_UMOUNT(devdir);
> >> +
> >> +	SAFE_MQ_UNLINK(MQNAME1);
> >> +	tst_atomic_store(0, mq_freed1);
> >>   
> >> -	write(p2[1], "done", 5);
> >> +	SAFE_MQ_UNLINK(MQNAME2);
> >> +	tst_atomic_store(0, mq_freed2);
> >> +}
> >>   
> >> -	exit(0);
> >> +static void run(void)
> >> +{
> >> +	const struct tst_clone_args clone_args = { CLONE_NEWIPC, SIGCHLD };
> >> +
> >> +	if (str_op && !strcmp(str_op, "clone")) {
> >> +		tst_res(TINFO, "Spawning isolated process");
> >> +
> >> +		if (!SAFE_CLONE(&clone_args)) {
> >> +			check_mqueue();
> >> +			return;
> >> +		}
> >> +	} else if (str_op && !strcmp(str_op, "unshare")) {
> >> +		tst_res(TINFO, "Spawning unshared process");
> >> +
> >> +		if (!SAFE_FORK()) {
> >> +			SAFE_UNSHARE(CLONE_NEWIPC);
> >> +			check_mqueue();
> >> +			return;
> >> +		}
> >> +	}
> >>   }
> >>   
> >>   static void setup(void)
> >>   {
> >> -	tst_require_root();
> >> -	check_mqns();
> >> +	char *tmpdir;
> >> +
> >> +	if (!str_op)
> >> +		tst_brk(TCONF, "Please specify clone|unshare child isolation");
> >> +
> >> +	tmpdir = tst_get_tmpdir();
> >> +
> >> +	SAFE_ASPRINTF(&devdir, "%s/mqueue", tmpdir);
> >> +	SAFE_MKDIR(devdir, 0755);
> >> +
> >> +	SAFE_ASPRINTF(&mqueue1, "%s" MQNAME1, devdir);
> >> +	SAFE_ASPRINTF(&mqueue2, "%s" MQNAME2, devdir);
> >> +
> >> +	mq_freed1 = SAFE_MMAP(NULL,
> >> +		sizeof(int),
> >> +		PROT_READ | PROT_WRITE,
> >> +		MAP_SHARED | MAP_ANONYMOUS,
> >> +		-1, 0);
> >> +
> >> +	mq_freed2 = SAFE_MMAP(NULL,
> >> +		sizeof(int),
> >> +		PROT_READ | PROT_WRITE,
> >> +		MAP_SHARED | MAP_ANONYMOUS,
> >> +		-1, 0);
> > So here you are allocating two pages of memory for something that is
> > basically two bitflags. Can you at least change this to a single mmap()
> > something as:
> >
> > static int *mq_freed;
> >
> > 	mq_freed = SAFE_MMAP(NULL, 2 * sizeof(int), ...)
> >
> >
> > 	mq_freed[0] = 1;
> > 	...
> >
> > Moreover since we can actually stat()/access() the mqueue we can as well
> > check for the existence of the devdir in the cleanup and only remove it
> > if it exists in the filesystem.
> >
> > Also I would be more afraid of the mqueue filesystem being mounted in
> > the temp directory if we trigger a failure between one of the
> > mount()/umount() calls, so we should as well check if it's mounted in
> > the cleanup and attempt to umount it.
> >
> >
> Andrea
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v7 3/6] Refactor mqns_03 using new LTP API

[Petr Vorel]

> Hi!
> > > I do not think that we need atomicity here, the cleanup code does not
> > > run concurently at all as the cleanup in the parent is triggered after
> > > the child did exit. I suppose that instead we need to set the mq_freed
> > > to be volatile because it's shared memory which may change at any
> > > change, so we need to tell that to the compiler.
> > That's fine, but I followed suggestions in the reviews. I think that 
> > having 3 people reviewing the same patch doesn't help the development 
> > process. Now I'm not sure who I should follow :-)

> It's actually the other way around, the more people look at the code the
> better, at least that way we have potential to catch more problems
> earlier. And if the reviewers disagree, let them fight for the right
> answer.

+1

Kind regards,
Petr

> I think that in this case this all can be actually simplified and we can
> get rid of the mq_freed flag as I tried to outline below.

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