diff mbox series

[v3,2/7] Refactor mqns_01 using new LTP API

Message ID 20220722120501.28670-3-andrea.cervesato@suse.com
State New
Headers show
Series Refactor mqns testing suite | expand

Commit Message

Andrea Cervesato July 22, 2022, 12:04 p.m. UTC
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/containers                         |   3 +-
 testcases/kernel/containers/mqns/common.h  | 101 +++++++++++
 testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++--------------
 3 files changed, 166 insertions(+), 131 deletions(-)
 create mode 100644 testcases/kernel/containers/mqns/common.h

Comments

Richard Palethorpe Aug. 11, 2022, 9:53 a.m. UTC | #1
Hello,

Andrea Cervesato via ltp <ltp@lists.linux.it> writes:

> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  runtest/containers                         |   3 +-
>  testcases/kernel/containers/mqns/common.h  | 101 +++++++++++
>  testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++--------------
>  3 files changed, 166 insertions(+), 131 deletions(-)
>  create mode 100644 testcases/kernel/containers/mqns/common.h
>
> diff --git a/runtest/containers b/runtest/containers
> index 2637b62fe..863a964ad 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/common.h b/testcases/kernel/containers/mqns/common.h
> new file mode 100644
> index 000000000..92a77b566
> --- /dev/null
> +++ b/testcases/kernel/containers/mqns/common.h
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +#ifndef MQNS_H
> +#define MQNS_H
> +
> +#include <stdlib.h>
> +#include "lapi/namespaces_constants.h"
> +#include "tst_test.h"
> +#include "tst_safe_posix_ipc.h"
> +
> +enum {
> +	T_CLONE,
> +	T_UNSHARE,
> +	T_NONE,
> +};
> +
> +static int dummy_child1(void *v)
> +{
> +	(void)v;
> +	return 0;
> +}
> +
> +static inline void check_newipc(void)
> +{
> +	int pid, status;
> +
> +	pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1,
>  NULL);

ltp_clone_quick is still part of the old API and only uses clone2. I
think it should be replaced with tst_clone. This may require extending
tst_clone. In fact we probably need a test variant to switch between the
clone2 and clone3 syscalls when using tst_clone.

I'll leave it to you whether you want to try that and rebase this patch
set on it.
Andrea Cervesato Aug. 11, 2022, 11:13 a.m. UTC | #2
Hi!

On 8/11/22 11:53, Richard Palethorpe wrote:
> Hello,
>
> Andrea Cervesato via ltp <ltp@lists.linux.it> writes:
>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>>   runtest/containers                         |   3 +-
>>   testcases/kernel/containers/mqns/common.h  | 101 +++++++++++
>>   testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++--------------
>>   3 files changed, 166 insertions(+), 131 deletions(-)
>>   create mode 100644 testcases/kernel/containers/mqns/common.h
>>
>> diff --git a/runtest/containers b/runtest/containers
>> index 2637b62fe..863a964ad 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/common.h b/testcases/kernel/containers/mqns/common.h
>> new file mode 100644
>> index 000000000..92a77b566
>> --- /dev/null
>> +++ b/testcases/kernel/containers/mqns/common.h
>> @@ -0,0 +1,101 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>> + */
>> +
>> +#ifndef MQNS_H
>> +#define MQNS_H
>> +
>> +#include <stdlib.h>
>> +#include "lapi/namespaces_constants.h"
>> +#include "tst_test.h"
>> +#include "tst_safe_posix_ipc.h"
>> +
>> +enum {
>> +	T_CLONE,
>> +	T_UNSHARE,
>> +	T_NONE,
>> +};
>> +
>> +static int dummy_child1(void *v)
>> +{
>> +	(void)v;
>> +	return 0;
>> +}
>> +
>> +static inline void check_newipc(void)
>> +{
>> +	int pid, status;
>> +
>> +	pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1,
>>   NULL);
> ltp_clone_quick is still part of the old API and only uses clone2. I
> think it should be replaced with tst_clone. This may require extending
> tst_clone. In fact we probably need a test variant to switch between the
> clone2 and clone3 syscalls when using tst_clone.
>
> I'll leave it to you whether you want to try that and rebase this patch
> set on it.
>
I see ltp_clone_quick as wrapper of ltp_clone, since it's using
ltp_alloc_stack without calling it explicitly all the times before 
ltp_clone.
Richard Palethorpe Sept. 2, 2022, 9:05 a.m. UTC | #3
Hello,

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

> Hi!
>
> On 8/11/22 11:53, Richard Palethorpe wrote:
>> Hello,
>>
>> Andrea Cervesato via ltp <ltp@lists.linux.it> writes:
>>
>>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>>> ---
>>>   runtest/containers                         |   3 +-
>>>   testcases/kernel/containers/mqns/common.h  | 101 +++++++++++
>>>   testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++--------------
>>>   3 files changed, 166 insertions(+), 131 deletions(-)
>>>   create mode 100644 testcases/kernel/containers/mqns/common.h
>>>
>>> diff --git a/runtest/containers b/runtest/containers
>>> index 2637b62fe..863a964ad 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/common.h b/testcases/kernel/containers/mqns/common.h
>>> new file mode 100644
>>> index 000000000..92a77b566
>>> --- /dev/null
>>> +++ b/testcases/kernel/containers/mqns/common.h
>>> @@ -0,0 +1,101 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>> +/*
>>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>>> + */
>>> +
>>> +#ifndef MQNS_H
>>> +#define MQNS_H
>>> +
>>> +#include <stdlib.h>
>>> +#include "lapi/namespaces_constants.h"
>>> +#include "tst_test.h"
>>> +#include "tst_safe_posix_ipc.h"
>>> +
>>> +enum {
>>> +	T_CLONE,
>>> +	T_UNSHARE,
>>> +	T_NONE,
>>> +};
>>> +
>>> +static int dummy_child1(void *v)
>>> +{
>>> +	(void)v;
>>> +	return 0;
>>> +}
>>> +
>>> +static inline void check_newipc(void)
>>> +{
>>> +	int pid, status;
>>> +
>>> +	pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1,
>>>   NULL);
>> ltp_clone_quick is still part of the old API and only uses clone2. I
>> think it should be replaced with tst_clone. This may require extending
>> tst_clone. In fact we probably need a test variant to switch between the
>> clone2 and clone3 syscalls when using tst_clone.
>>
>> I'll leave it to you whether you want to try that and rebase this patch
>> set on it.
>>
> I see ltp_clone_quick as wrapper of ltp_clone, since it's using
> ltp_alloc_stack without calling it explicitly all the times before
> ltp_clone.

ltp_clone is also part of the old API. At some point we should remove that.
diff mbox series

Patch

diff --git a/runtest/containers b/runtest/containers
index 2637b62fe..863a964ad 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/common.h b/testcases/kernel/containers/mqns/common.h
new file mode 100644
index 000000000..92a77b566
--- /dev/null
+++ b/testcases/kernel/containers/mqns/common.h
@@ -0,0 +1,101 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+#ifndef MQNS_H
+#define MQNS_H
+
+#include <stdlib.h>
+#include "lapi/namespaces_constants.h"
+#include "tst_test.h"
+#include "tst_safe_posix_ipc.h"
+
+enum {
+	T_CLONE,
+	T_UNSHARE,
+	T_NONE,
+};
+
+static int dummy_child1(void *v)
+{
+	(void)v;
+	return 0;
+}
+
+static inline void check_newipc(void)
+{
+	int pid, status;
+
+	pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1, NULL);
+	if (pid < 0)
+		tst_brk(TCONF | TERRNO, "CLONE_NEWIPC not supported");
+
+	SAFE_WAITPID(pid, &status, 0);
+}
+
+static inline int get_clone_unshare_enum(const char* str_op)
+{
+	if (!str_op)
+		return T_NONE;
+	else if (!strcmp(str_op, "clone"))
+		return T_CLONE;
+	else if (!strcmp(str_op, "unshare"))
+		return T_UNSHARE;
+
+	return T_NONE;
+}
+
+static void clone_test(unsigned long clone_flags, int (*fn1)(void *arg), void *arg1)
+{
+	int pid;
+
+	pid = ltp_clone_quick(clone_flags | SIGCHLD, fn1, arg1);
+	if (pid < 0)
+		tst_brk(TBROK | TERRNO, "ltp_clone_quick error");
+}
+
+static void unshare_test(unsigned long clone_flags, int (*fn1)(void *arg), void *arg1)
+{
+	int pid;
+
+	pid = SAFE_FORK();
+	if (!pid) {
+		SAFE_UNSHARE(clone_flags);
+
+		fn1(arg1);
+		exit(0);
+	}
+}
+
+static void plain_test(int (*fn1)(void *arg), void *arg1)
+{
+	int pid;
+
+	pid = SAFE_FORK();
+	if (!pid) {
+		fn1(arg1);
+		exit(0);
+	}
+}
+
+static void clone_unshare_test(int use_clone, unsigned long clone_flags,
+			       int (*fn1)(void *arg), void *arg1)
+{
+	switch (use_clone) {
+	case T_NONE:
+		plain_test(fn1, arg1);
+	break;
+	case T_CLONE:
+		clone_test(clone_flags, fn1, arg1);
+	break;
+	case T_UNSHARE:
+		unshare_test(clone_flags, fn1, arg1);
+	break;
+	default:
+		tst_brk(TBROK, "%s: bad use_clone option: %d", __FUNCTION__, use_clone);
+	break;
+	}
+}
+
+#endif /* MQNS_H */
diff --git a/testcases/kernel/containers/mqns/mqns_01.c b/testcases/kernel/containers/mqns/mqns_01.c
index 1d109e020..a34dc4f66 100644
--- a/testcases/kernel/containers/mqns/mqns_01.c
+++ b/testcases/kernel/containers/mqns/mqns_01.c
@@ -1,148 +1,81 @@ 
+// 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: 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
-
-***************************************************************************/
-
-#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_01";
-int TST_TOTAL = 1;
-
-int p1[2];
-int p2[2];
-
-int check_mqueue(void *vtest)
+ * Copyright (c) International Business Machines Corp., 2009
+ * Copyright (c) Nadia Derbey, 2009 <Nadia.Derbey@bull.net>
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Create a mqueue inside the parent and check if it can be accessed from
+ * the isolated/forked child namespace.
+ */
+
+#include "common.h"
+
+#define MQNAME "/MQ1"
+
+static mqd_t mqd;
+static char *str_op;
+static int use_clone;
+
+static int check_mqueue(LTP_ATTRIBUTE_UNUSED void *vtest)
 {
-	char buf[30];
-	mqd_t mqd;
+	mqd_t mqd1;
 
-	(void) vtest;
+	mqd1 = mq_open(MQNAME, O_RDONLY);
 
-	close(p1[1]);
-	close(p2[0]);
+	if (use_clone == T_NONE) {
+		if (mqd1 == -1)
+			tst_res(TFAIL, "Queue has been accessed form plain cloned process");
+		else
+			tst_res(TPASS, "Can't access to queue from plain cloned 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);
-		}
-	} 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);
-		}
+		return 0;
 	}
 
-	exit(0);
+	if (mqd1 == -1)
+		tst_res(TPASS, "Can't access to queue from isolated process");
+	else
+		tst_res(TFAIL, "Queue has been accessed from isolated process");
+
+	return 0;
 }
 
-static void setup(void)
+static void run(void)
 {
-	tst_require_root();
-	check_mqns();
+	tst_res(TINFO, "Checking namespaces isolation from parent to child");
+
+	clone_unshare_test(use_clone, CLONE_NEWIPC, check_mqueue, NULL);
 }
 
-int main(int argc, char *argv[])
+static void setup(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");
-	}
+	use_clone = get_clone_unshare_enum(str_op);
 
-	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 (use_clone != T_NONE)
+		check_newipc();
 
-	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");
-		}
-	}
+	mqd = SAFE_MQ_OPEN(MQNAME, O_RDWR | O_CREAT | O_EXCL, 0777, NULL);
+}
 
-	/* destroy the mqueue */
-	if (mq_close(mqd) == -1) {
-		tst_brkm(TBROK | TERRNO, NULL, "mq_close failed");
+static void cleanup(void)
+{
+	if (mqd != -1) {
+		SAFE_MQ_CLOSE(mqd);
+		SAFE_MQ_UNLINK(MQNAME);
 	}
-	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, "Test execution mode <clone|unshare>" },
+		{},
+	},
+};