[LTP] [PATCH v2 0/8] Rewrite userns testing suite using new LTP API

[LTP] [PATCH v2 0/8] Rewrite userns testing suite using new LTP API

[Andrea Cervesato]

Removed TST_CHECKPOINT_INIT from tests and replaced libclone usage with the
internal LTP API clone API.

Andrea Cervesato (8):
  Rewrite userns01.c using new LTP API
  Rewrite userns02.c using new LTP API
  Rewrite userns03.c using new LTP API
  Rewrite userns04.c using new LTP API
  Rewrite userns05.c using new LTP API
  Rewrite userns06.c using new LTP API
  Rewrite userns07.c using new LTP API
  Rewrite userns08.c using new LTP API

 testcases/kernel/containers/userns/common.h   |  58 ++++
 testcases/kernel/containers/userns/userns01.c | 120 ++++----
 testcases/kernel/containers/userns/userns02.c | 139 ++++-----
 testcases/kernel/containers/userns/userns03.c | 266 ++++++++----------
 testcases/kernel/containers/userns/userns04.c | 139 ++++-----
 testcases/kernel/containers/userns/userns05.c | 148 ++++------
 testcases/kernel/containers/userns/userns06.c | 179 ++++++------
 .../containers/userns/userns06_capcheck.c     |  75 +++--
 testcases/kernel/containers/userns/userns07.c | 127 ++++-----
 testcases/kernel/containers/userns/userns08.c |  17 +-
 10 files changed, 586 insertions(+), 682 deletions(-)
 create mode 100644 testcases/kernel/containers/userns/common.h

-- 
2.35.1


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

[LTP] [PATCH v2 1/8] Rewrite userns01.c using new LTP API

[Andrea Cervesato]

Added common.h to be used instead of userns_helper.h by all userns
tests.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 testcases/kernel/containers/userns/common.h   |  58 +++++++++
 testcases/kernel/containers/userns/userns01.c | 120 ++++++++----------
 2 files changed, 113 insertions(+), 65 deletions(-)
 create mode 100644 testcases/kernel/containers/userns/common.h

diff --git a/testcases/kernel/containers/userns/common.h b/testcases/kernel/containers/userns/common.h
new file mode 100644
index 000000000..95160c8cc
--- /dev/null
+++ b/testcases/kernel/containers/userns/common.h
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) Huawei Technologies Co., Ltd., 2015
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+#ifndef COMMON_H
+#define COMMON_H
+
+#include "tst_test.h"
+#include "lapi/namespaces_constants.h"
+
+#define UID_MAP 0
+#define GID_MAP 1
+
+static int dummy_child(void *v)
+{
+	(void)v;
+	return 0;
+}
+
+static inline int check_newuser(void)
+{
+	int pid, status;
+
+	if (tst_kvercmp(3, 8, 0) < 0)
+		tst_brk(TCONF, "CLONE_NEWUSER not supported");
+
+	pid = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, dummy_child, NULL);
+	if (pid == -1)
+		tst_brk(TCONF | TTERRNO, "CLONE_NEWUSER not supported");
+
+	SAFE_WAIT(&status);
+
+	return 0;
+}
+
+static inline void updatemap(int cpid, bool type, int idnum, int parentmappid)
+{
+	char path[BUFSIZ];
+	char content[BUFSIZ];
+	int fd;
+
+	if (type == UID_MAP)
+		sprintf(path, "/proc/%d/uid_map", cpid);
+	else if (type == GID_MAP)
+		sprintf(path, "/proc/%d/gid_map", cpid);
+	else
+		tst_brk(TBROK, "invalid type parameter");
+
+	sprintf(content, "%d %d 1", idnum, parentmappid);
+
+	fd = SAFE_OPEN(path, O_WRONLY, 0644);
+	SAFE_WRITE(1, fd, content, strlen(content));
+	SAFE_CLOSE(fd);
+}
+
+#endif
diff --git a/testcases/kernel/containers/userns/userns01.c b/testcases/kernel/containers/userns/userns01.c
index 1c8cf570d..9e52ebece 100644
--- a/testcases/kernel/containers/userns/userns01.c
+++ b/testcases/kernel/containers/userns/userns01.c
@@ -1,115 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) Huawei Technologies Co., Ltd., 2015
- * 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.
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * Verify that:
- *  If a user ID has no mapping inside the namespace, user ID and group
- * ID will be the value defined in the file /proc/sys/kernel/overflowuid(65534)
- * and /proc/sys/kernel/overflowgid(65534). A child process has a full set
- * of permitted and effective capabilities, even though the program was
- * run from an unprivileged account.
+/*\
+ * [Description]
+ *
+ * Verify that if a user ID has no mapping inside the namespace, user ID and
+ * group ID will be the value defined in the file /proc/sys/kernel/overflowuid(65534)
+ * and /proc/sys/kernel/overflowgid(65534). A child process has a full set of
+ * permitted and effective capabilities, even though the program was run from an
+ * unprivileged account.
  */
 
+#include "tst_test.h"
+
+#ifdef HAVE_LIBCAP
 #define _GNU_SOURCE
-#include <sys/wait.h>
-#include <assert.h>
+
 #include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include "userns_helper.h"
-#include "test.h"
+#include <stdbool.h>
+#include "common.h"
 #include "config.h"
-#if HAVE_SYS_CAPABILITY_H
 #include <sys/capability.h>
-#endif
 
 #define OVERFLOWUIDPATH "/proc/sys/kernel/overflowuid"
 #define OVERFLOWGIDPATH "/proc/sys/kernel/overflowgid"
 
-char *TCID = "user_namespace1";
-int TST_TOTAL = 1;
-
 static long overflowuid;
 static long overflowgid;
 
 /*
  * child_fn1() - Inside a new user namespace
  */
-static int child_fn1(void *arg LTP_ATTRIBUTE_UNUSED)
+static int child_fn1(LTP_ATTRIBUTE_UNUSED void *arg)
 {
-	int exit_val = 0;
 	int uid, gid;
-#ifdef HAVE_LIBCAP
 	cap_t caps;
 	int i, last_cap;
 	cap_flag_value_t flag_val;
-#endif
 
 	uid = geteuid();
 	gid = getegid();
 
-	tst_resm(TINFO, "USERNS test is running in a new user namespace.");
+	tst_res(TINFO, "USERNS test is running in a new user namespace.");
 
-	if (uid != overflowuid || gid != overflowgid) {
-		printf("Got unexpected result of uid=%d gid=%d\n", uid, gid);
-		exit_val = 1;
-	}
+	if (uid != overflowuid || gid != overflowgid)
+		tst_res(TFAIL, "got unexpected uid=%d gid=%d", uid, gid);
+	else
+		tst_res(TPASS, "got expected uid and gid");
 
-#ifdef HAVE_LIBCAP
 	caps = cap_get_proc();
-	SAFE_FILE_SCANF(NULL, "/proc/sys/kernel/cap_last_cap", "%d", &last_cap);
+
+	SAFE_FILE_SCANF("/proc/sys/kernel/cap_last_cap", "%d", &last_cap);
+
 	for (i = 0; i <= last_cap; i++) {
 		cap_get_flag(caps, i, CAP_EFFECTIVE, &flag_val);
-		if (flag_val == 0)
+		if (!flag_val)
 			break;
+
 		cap_get_flag(caps, i, CAP_PERMITTED, &flag_val);
-		if (flag_val == 0)
+		if (!flag_val)
 			break;
 	}
 
-	if (flag_val == 0) {
-		printf("unexpected effective/permitted caps at %d\n", i);
-		exit_val = 1;
-	}
-#else
-	printf("System is missing libcap.\n");
-#endif
-	return exit_val;
+	if (!flag_val)
+		tst_res(TFAIL, "unexpected effective/permitted caps at %d", i);
+	else
+		tst_res(TPASS, "expected capabilities");
+
+	return 0;
 }
 
 static void setup(void)
 {
 	check_newuser();
-	SAFE_FILE_SCANF(NULL, OVERFLOWUIDPATH, "%ld", &overflowuid);
-	SAFE_FILE_SCANF(NULL, OVERFLOWGIDPATH, "%ld", &overflowgid);
+
+	SAFE_FILE_SCANF(OVERFLOWUIDPATH, "%ld", &overflowuid);
+	SAFE_FILE_SCANF(OVERFLOWGIDPATH, "%ld", &overflowgid);
 }
 
-int main(int argc, char *argv[])
+static void run(void)
 {
-	int lc;
+	int pid;
 
-	tst_parse_opts(argc, argv, NULL, NULL);
-	setup();
+	pid = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, child_fn1, NULL);
+	if (pid < 0)
+		tst_brk(TBROK | TTERRNO, "clone failed");
+}
 
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		TEST(do_clone_unshare_test(T_CLONE, CLONE_NEWUSER,
-			child_fn1, NULL));
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = run,
+	.needs_root = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_USER_NS",
+		NULL,
+	},
+};
 
-		if (TEST_RETURN == -1)
-			tst_brkm(TFAIL | TTERRNO, NULL, "clone failed");
-		tst_record_childstatus(NULL, -1);
-	}
-	tst_exit();
-}
+#else
+TST_TEST_TCONF("System is missing libcap");
+#endif
-- 
2.35.1


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

[LTP] [PATCH v2 1/2] Remove libclone from userns test suite

[Andrea Cervesato]

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
This patch goes after [PATCH v2 8/8] Rewrite userns08.c using new LTP API

 testcases/kernel/containers/userns/Makefile | 23 ++++-----------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/testcases/kernel/containers/userns/Makefile b/testcases/kernel/containers/userns/Makefile
index 80681096d..c1f10de20 100644
--- a/testcases/kernel/containers/userns/Makefile
+++ b/testcases/kernel/containers/userns/Makefile
@@ -1,26 +1,11 @@
-###############################################################################
-#                                                                            ##
-# Copyright (c) Huawei Technologies Co., Ltd., 2015                          ##
-#                                                                            ##
-# 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.                                                   ##
-###############################################################################
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) Huawei Technologies Co., Ltd., 2015
+# 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			:= -lclone $(LDLIBS) $(CAP_LIBS)
+LDLIBS  := $(CAP_LIBS) $(LDLIBS)
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
-- 
2.35.1


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

[LTP] [PATCH v2 2/2] Remove obsolete userns_helper.h from userns

[Andrea Cervesato]

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
This patch goes after [PATCH v2 8/8] Rewrite userns08.c using new LTP API

 .../kernel/containers/userns/userns_helper.h  | 62 -------------------
 1 file changed, 62 deletions(-)
 delete mode 100644 testcases/kernel/containers/userns/userns_helper.h

diff --git a/testcases/kernel/containers/userns/userns_helper.h b/testcases/kernel/containers/userns/userns_helper.h
deleted file mode 100644
index 12b491f62..000000000
--- a/testcases/kernel/containers/userns/userns_helper.h
+++ /dev/null
@@ -1,62 +0,0 @@
-/*
- * Copyright (c) Huawei Technologies Co., Ltd., 2015
- * 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.
- */
-
-#include "../libclone/libclone.h"
-#include "test.h"
-#include "safe_macros.h"
-#include <stdbool.h>
-
-#define UID_MAP 0
-#define GID_MAP 1
-
-static int dummy_child(void *v)
-{
-	(void) v;
-	return 0;
-}
-
-static int check_newuser(void)
-{
-	int pid, status;
-
-	if (tst_kvercmp(3, 8, 0) < 0)
-		tst_brkm(TCONF, NULL, "CLONE_NEWUSER not supported");
-
-	pid = do_clone_unshare_test(T_CLONE, CLONE_NEWUSER, dummy_child, NULL);
-	if (pid == -1)
-		tst_brkm(TCONF | TERRNO, NULL, "CLONE_NEWUSER not supported");
-	SAFE_WAIT(NULL, &status);
-
-	return 0;
-}
-
-LTP_ATTRIBUTE_UNUSED static int updatemap(int cpid, bool type, int idnum,
-	int parentmappid, void (*cleanup)(void))
-{
-	char path[BUFSIZ];
-	char content[BUFSIZ];
-	int fd;
-
-	if (type == UID_MAP)
-		sprintf(path, "/proc/%d/uid_map", cpid);
-	else if (type == GID_MAP)
-		sprintf(path, "/proc/%d/gid_map", cpid);
-	else
-		tst_brkm(TBROK, cleanup, "invalid type parameter");
-
-	sprintf(content, "%d %d 1", idnum, parentmappid);
-	fd = SAFE_OPEN(cleanup, path, O_WRONLY, 0644);
-	SAFE_WRITE(cleanup, 1, fd, content, strlen(content));
-	SAFE_CLOSE(cleanup, fd);
-	return 0;
-}
-- 
2.35.1


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

Re: [LTP] [PATCH v2 1/8] Rewrite userns01.c using new LTP API

[Petr Vorel]

Hi Andrea,

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

nit: I'd sometimes use underscore to make names more readable
i.e. update_map, parent_map_pid, OVERFLOW_UID_PATH, OVERFLOW_GID_PATH

> +static inline int check_newuser(void)
> +{
> +	int pid, status;
> +
> +	if (tst_kvercmp(3, 8, 0) < 0)
> +		tst_brk(TCONF, "CLONE_NEWUSER not supported");
Is this limitation needed, when we have the same check with ltp_clone_quick()?
> +
> +	pid = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, dummy_child, NULL);
> +	if (pid == -1)
> +		tst_brk(TCONF | TTERRNO, "CLONE_NEWUSER not supported");
> +
> +	SAFE_WAIT(&status);
> +
> +	return 0;
> +}
> +
> +static inline void updatemap(int cpid, bool type, int idnum, int parentmappid)
> +{
> +	char path[BUFSIZ];
> +	char content[BUFSIZ];
> +	int fd;
> +
> +	if (type == UID_MAP)
> +		sprintf(path, "/proc/%d/uid_map", cpid);
> +	else if (type == GID_MAP)
> +		sprintf(path, "/proc/%d/gid_map", cpid);
> +	else
> +		tst_brk(TBROK, "invalid type parameter");

nit: maybe switch would be more readable.

...

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 1/8] Rewrite userns01.c using new LTP API

[Petr Vorel]

Hi Andrea,

>  testcases/kernel/containers/userns/common.h   |  58 +++++++++
...
> +static inline int check_newuser(void)
check_newuser() always return 0, it's return value is not used in any test.
Maybe pid was planned to be used in original implementation, but for now I'd
change return type to void.

> +{
> +	int pid, status;
> +
> +	if (tst_kvercmp(3, 8, 0) < 0)
> +		tst_brk(TCONF, "CLONE_NEWUSER not supported");
> +
> +	pid = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, dummy_child, NULL);
> +	if (pid == -1)
> +		tst_brk(TCONF | TTERRNO, "CLONE_NEWUSER not supported");
> +
> +	SAFE_WAIT(&status);
> +
> +	return 0;
> +}

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 1/8] Rewrite userns01.c using new LTP API

[Petr Vorel]

Hi Andrea,

...
> +static inline void updatemap(int cpid, bool type, int idnum, int parentmappid)
nit: This header should #include <stdbool.h>, not all tests (they use UID_MAP,
thus they don't need it).

Kind regards,
Petr

> +{
> +	char path[BUFSIZ];
> +	char content[BUFSIZ];
> +	int fd;
> +
> +	if (type == UID_MAP)
> +		sprintf(path, "/proc/%d/uid_map", cpid);
> +	else if (type == GID_MAP)
> +		sprintf(path, "/proc/%d/gid_map", cpid);
> +	else
> +		tst_brk(TBROK, "invalid type parameter");
> +
> +	sprintf(content, "%d %d 1", idnum, parentmappid);
> +
> +	fd = SAFE_OPEN(path, O_WRONLY, 0644);
> +	SAFE_WRITE(1, fd, content, strlen(content));
> +	SAFE_CLOSE(fd);
> +}

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

Re: [LTP] [PATCH v2 1/8] Rewrite userns01.c using new LTP API

[Andrea Cervesato via ltp]

[-- Attachment #1.1: Type: text/plain, Size: 897 bytes --]

Hi Petr,

On 3/24/22 21:50, Petr Vorel wrote:
> Hi Andrea,
>
> ...
>> +static inline void updatemap(int cpid, bool type, int idnum, int parentmappid)
> nit: This header should #include <stdbool.h>, not all tests (they use UID_MAP,
> thus they don't need it).
I think this will be replaced with integer, according also with the 
if/else statement which is below and use switch to check its value as well.
> Kind regards,
> Petr
>
>> +{
>> +	char path[BUFSIZ];
>> +	char content[BUFSIZ];
>> +	int fd;
>> +
>> +	if (type == UID_MAP)
>> +		sprintf(path, "/proc/%d/uid_map", cpid);
>> +	else if (type == GID_MAP)
>> +		sprintf(path, "/proc/%d/gid_map", cpid);
>> +	else
>> +		tst_brk(TBROK, "invalid type parameter");
>> +
>> +	sprintf(content, "%d %d 1", idnum, parentmappid);
>> +
>> +	fd = SAFE_OPEN(path, O_WRONLY, 0644);
>> +	SAFE_WRITE(1, fd, content, strlen(content));
>> +	SAFE_CLOSE(fd);
>> +}

[-- Attachment #1.2: Type: text/html, Size: 1653 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

Re: [LTP] [PATCH v2 1/8] Rewrite userns01.c using new LTP API

[Petr Vorel]

Hi Andrea,

> > > +static inline void updatemap(int cpid, bool type, int idnum, int parentmappid)
> > nit: This header should #include <stdbool.h>, not all tests (they use UID_MAP,
> > thus they don't need it).
> I think this will be replaced with integer, according also with the if/else
> statement which is below and use switch to check its value as well.

+1

Kind regards,
Petr

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

[LTP] [PATCH v2 2/8] Rewrite userns02.c using new LTP API

[Andrea Cervesato]

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 testcases/kernel/containers/userns/userns02.c | 139 ++++++++----------
 1 file changed, 59 insertions(+), 80 deletions(-)

diff --git a/testcases/kernel/containers/userns/userns02.c b/testcases/kernel/containers/userns/userns02.c
index ae49a1599..eaf29a1fd 100644
--- a/testcases/kernel/containers/userns/userns02.c
+++ b/testcases/kernel/containers/userns/userns02.c
@@ -1,74 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) Huawei Technologies Co., Ltd., 2015
- * 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.
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * Verify that:
- *  The user ID and group ID, which are inside a container, can be modified
- * by its parent process.
+/*\
+ * [Description]
+ *
+ * Verify that the user ID and group ID, which are inside a container,
+ * can be modified by its parent process.
  */
 
 #define _GNU_SOURCE
-#include <sys/wait.h>
-#include <assert.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include "userns_helper.h"
-#include "test.h"
-
-char *TCID = "user_namespace2";
-int TST_TOTAL = 1;
 
-static void cleanup(void)
-{
-	tst_rmdir();
-}
+#include <stdio.h>
+#include <stdbool.h>
+#include "common.h"
+#include "tst_test.h"
 
 /*
  * child_fn1() - Inside a new user namespace
  */
 static int child_fn1(void)
 {
-	int exit_val;
 	int uid, gid;
 
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAIT(0);
+
 	uid = geteuid();
 	gid = getegid();
 
-	if (uid == 100 && gid == 100) {
-		printf("Got expected uid and gid.\n");
-		exit_val = 0;
-	} else {
-		printf("Got unexpected result of uid=%d gid=%d\n", uid, gid);
-		exit_val = 1;
-	}
+	if (uid == 100 && gid == 100)
+		tst_res(TPASS, "got expected uid and gid");
+	else
+		tst_res(TFAIL, "got unexpected uid=%d gid=%d", uid, gid);
 
-	return exit_val;
+	return 0;
 }
 
 static void setup(void)
 {
 	check_newuser();
-	tst_tmpdir();
-	TST_CHECKPOINT_INIT(NULL);
 }
 
-int main(int argc, char *argv[])
+static void run(void)
 {
-	int lc;
 	int childpid;
 	int parentuid;
 	int parentgid;
@@ -76,42 +52,45 @@ int main(int argc, char *argv[])
 	char content[BUFSIZ];
 	int fd;
 
-	tst_parse_opts(argc, argv, NULL, NULL);
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		childpid = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
-			(void *)child_fn1, NULL);
-
-		if (childpid < 0)
-			tst_brkm(TFAIL | TERRNO, cleanup, "clone failed");
-
-		parentuid = geteuid();
-		parentgid = getegid();
-		sprintf(path, "/proc/%d/uid_map", childpid);
-		sprintf(content, "100 %d 1", parentuid);
-		fd = SAFE_OPEN(cleanup, path, O_WRONLY, 0644);
-		SAFE_WRITE(cleanup, 1, fd, content, strlen(content));
-		SAFE_CLOSE(cleanup, fd);
-
-		if (access("/proc/self/setgroups", F_OK) == 0) {
-			sprintf(path, "/proc/%d/setgroups", childpid);
-			fd = SAFE_OPEN(cleanup, path, O_WRONLY, 0644);
-			SAFE_WRITE(cleanup, 1, fd, "deny", 4);
-			SAFE_CLOSE(cleanup, fd);
-		}
-
-		sprintf(path, "/proc/%d/gid_map", childpid);
-		sprintf(content, "100 %d 1", parentgid);
-		fd = SAFE_OPEN(cleanup, path, O_WRONLY, 0644);
-		SAFE_WRITE(cleanup, 1, fd, content, strlen(content));
-		SAFE_CLOSE(cleanup, fd);
-
-		TST_SAFE_CHECKPOINT_WAKE(cleanup, 0);
-
-		tst_record_childstatus(cleanup, childpid);
+	childpid = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn1, NULL);
+	if (childpid < 0)
+		tst_brk(TBROK | TTERRNO, "clone failed");
+
+	parentuid = geteuid();
+	parentgid = getegid();
+
+	sprintf(path, "/proc/%d/uid_map", childpid);
+	sprintf(content, "100 %d 1", parentuid);
+
+	fd = SAFE_OPEN(path, O_WRONLY, 0644);
+	SAFE_WRITE(1, fd, content, strlen(content));
+	SAFE_CLOSE(fd);
+
+	if (access("/proc/self/setgroups", F_OK) == 0) {
+		sprintf(path, "/proc/%d/setgroups", childpid);
+
+		fd = SAFE_OPEN(path, O_WRONLY, 0644);
+		SAFE_WRITE(1, fd, "deny", 4);
+		SAFE_CLOSE(fd);
 	}
-	cleanup();
-	tst_exit();
+
+	sprintf(path, "/proc/%d/gid_map", childpid);
+	sprintf(content, "100 %d 1", parentgid);
+
+	fd = SAFE_OPEN(path, O_WRONLY, 0644);
+	SAFE_WRITE(1, fd, content, strlen(content));
+	SAFE_CLOSE(fd);
+
+	TST_CHECKPOINT_WAKE(0);
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = run,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_USER_NS",
+		NULL,
+	},
+};
-- 
2.35.1


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

Re: [LTP] [PATCH v2 2/8] Rewrite userns02.c using new LTP API

[Petr Vorel]

Hi Andrea,

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

Thanks!

Kind regards,
Petr

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

[LTP] [PATCH v2 3/8] Rewrite userns03.c using new LTP API

[Andrea Cervesato]

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 testcases/kernel/containers/userns/userns03.c | 266 ++++++++----------
 1 file changed, 121 insertions(+), 145 deletions(-)

diff --git a/testcases/kernel/containers/userns/userns03.c b/testcases/kernel/containers/userns/userns03.c
index be511fec8..1084bef72 100644
--- a/testcases/kernel/containers/userns/userns03.c
+++ b/testcases/kernel/containers/userns/userns03.c
@@ -1,19 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) Huawei Technologies Co., Ltd., 2015
- * 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.
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * Verify that:
- * /proc/PID/uid_map and /proc/PID/gid_map contains three values separated by
- * white space:
+/*\
+ * [Description]
+ *
+ * Verify that /proc/PID/uid_map and /proc/PID/gid_map contains three values
+ * separated by white space:
  * ID-inside-ns   ID-outside-ns   length
  *
  * ID-outside-ns is interpreted according to which process is opening the file.
@@ -26,29 +21,23 @@
  * The string "deny" would be written to /proc/self/setgroups before GID
  * check if setgroups is allowed, see kernel commits:
  *
- *   commit 9cc46516ddf497ea16e8d7cb986ae03a0f6b92f8
- *   Author: Eric W. Biederman <ebiederm@xmission.com>
- *   Date:   Tue Dec 2 12:27:26 2014 -0600
- *     userns: Add a knob to disable setgroups on a per user namespace basis
- *
- *   commit 66d2f338ee4c449396b6f99f5e75cd18eb6df272
- *   Author: Eric W. Biederman <ebiederm@xmission.com>
- *   Date:   Fri Dec 5 19:36:04 2014 -0600
- *     userns: Allow setting gid_maps without privilege when setgroups is disabled
+ * commit 9cc46516ddf497ea16e8d7cb986ae03a0f6b92f8
+ * Author: Eric W. Biederman <ebiederm@xmission.com>
+ * Date:   Tue Dec 2 12:27:26 2014 -0600
+ * userns: Add a knob to disable setgroups on a per user namespace basis
  *
+ * commit 66d2f338ee4c449396b6f99f5e75cd18eb6df272
+ * Author: Eric W. Biederman <ebiederm@xmission.com>
+ * Date:   Fri Dec 5 19:36:04 2014 -0600
+ * userns: Allow setting gid_maps without privilege when setgroups is disabled
  */
 
 #define _GNU_SOURCE
-#include <sys/wait.h>
-#include <assert.h>
+
 #include <stdio.h>
-#include <stdlib.h>
 #include <stdbool.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include "userns_helper.h"
-#include "test.h"
+#include "common.h"
+#include "tst_test.h"
 
 #define CHILD1UID 0
 #define CHILD1GID 0
@@ -57,16 +46,16 @@
 #define UID_MAP 0
 #define GID_MAP 1
 
-char *TCID = "user_namespace3";
-int TST_TOTAL = 1;
-static int cpid1, parentuid, parentgid;
+static int cpid1;
+static int parentuid;
+static int parentgid;
 
 /*
  * child_fn1() - Inside a new user namespace
  */
 static int child_fn1(void)
 {
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAIT(0);
 	return 0;
 }
 
@@ -75,161 +64,148 @@ static int child_fn1(void)
  */
 static int child_fn2(void)
 {
-	int exit_val = 0;
 	int uid, gid;
 	char cpid1uidpath[BUFSIZ];
 	char cpid1gidpath[BUFSIZ];
 	int idinsidens, idoutsidens, length;
 
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 1);
+	TST_CHECKPOINT_WAIT(1);
 
 	uid = geteuid();
 	gid = getegid();
 
-	if (uid != CHILD2UID || gid != CHILD2GID) {
-		printf("unexpected uid=%d gid=%d\n", uid, gid);
-		exit_val = 1;
-	}
+	if (uid != CHILD2UID || gid != CHILD2GID)
+		tst_res(TFAIL, "unexpected uid=%d gid=%d", uid, gid);
+	else
+		tst_res(TPASS, "expected uid and gid");
 
-	/*Get the uid parameters of the child_fn2 process.*/
-	SAFE_FILE_SCANF(NULL, "/proc/self/uid_map", "%d %d %d", &idinsidens,
-		&idoutsidens, &length);
+	/* Get the uid parameters of the child_fn2 process */
+	SAFE_FILE_SCANF("/proc/self/uid_map", "%d %d %d", &idinsidens, &idoutsidens, &length);
 
 	/* map file format:ID-inside-ns   ID-outside-ns   length
-	If the process opening the file is in the same user namespace as
-	the process PID, then ID-outside-ns is defined with respect to the
-	 parent user namespace.*/
+	 * If the process opening the file is in the same user namespace as
+	 * the process PID, then ID-outside-ns is defined with respect to the
+	 * parent user namespace
+	 */
 	if (idinsidens != CHILD2UID || idoutsidens != parentuid) {
-		printf("child_fn2 checks /proc/cpid2/uid_map:\n");
-		printf("unexpected: idinsidens=%d idoutsidens=%d\n",
-			idinsidens, idoutsidens);
-		exit_val = 1;
+		tst_res(TINFO, "child2 checks /proc/cpid2/uid_map");
+		tst_res(TFAIL, "unexpected: namespace ID inside=%d outside=%d", idinsidens, idoutsidens);
+	} else {
+		tst_res(TPASS, "expected namespaces IDs");
 	}
 
 	sprintf(cpid1uidpath, "/proc/%d/uid_map", cpid1);
-	SAFE_FILE_SCANF(NULL, cpid1uidpath, "%d %d %d", &idinsidens,
-		&idoutsidens, &length);
+	SAFE_FILE_SCANF(cpid1uidpath, "%d %d %d", &idinsidens, &idoutsidens, &length);
 
 	/* If the process opening the file is in a different user namespace,
-	then ID-outside-ns is defined with respect to the user namespace
-	of the process opening the file.*/
+	 * then ID-outside-ns is defined with respect to the user namespace
+	 * of the process opening the file
+	 */
 	if (idinsidens != CHILD1UID || idoutsidens != CHILD2UID) {
-		printf("child_fn2 checks /proc/cpid1/uid_map:\n");
-		printf("unexpected: idinsidens=%d idoutsidens=%d\n",
-			idinsidens, idoutsidens);
-		exit_val = 1;
+		tst_res(TINFO, "child2 checks /proc/cpid1/uid_map");
+		tst_res(TFAIL, "unexpected: namespace ID inside=%d outside=%d", idinsidens, idoutsidens);
+	} else {
+		tst_res(TPASS, "expected namespaces IDs");
 	}
 
 	sprintf(cpid1gidpath, "/proc/%d/gid_map", cpid1);
-	SAFE_FILE_SCANF(NULL, "/proc/self/gid_map", "%d %d %d",
-		 &idinsidens, &idoutsidens, &length);
+	SAFE_FILE_SCANF("/proc/self/gid_map", "%d %d %d", &idinsidens, &idoutsidens, &length);
 
 	if (idinsidens != CHILD2GID || idoutsidens != parentgid) {
-		printf("child_fn2 checks /proc/cpid2/gid_map:\n");
-		printf("unexpected: idinsidens=%d idoutsidens=%d\n",
-			idinsidens, idoutsidens);
-		exit_val = 1;
+		tst_res(TINFO, "child2 checks /proc/cpid2/gid_map");
+		tst_res(TFAIL, "unexpected: namespace ID inside=%d outside=%d", idinsidens, idoutsidens);
+	} else {
+		tst_res(TPASS, "expected namespaces IDs");
 	}
 
-	SAFE_FILE_SCANF(NULL, cpid1gidpath, "%d %d %d", &idinsidens,
-		&idoutsidens, &length);
+	SAFE_FILE_SCANF(cpid1gidpath, "%d %d %d", &idinsidens, &idoutsidens, &length);
 
 	if (idinsidens != CHILD1GID || idoutsidens != CHILD2GID) {
-		printf("child_fn1 checks /proc/cpid1/gid_map:\n");
-		printf("unexpected: idinsidens=%d idoutsidens=%d\n",
-			idinsidens, idoutsidens);
-		exit_val = 1;
+		tst_res(TINFO, "child1 checks /proc/cpid1/gid_map");
+		tst_res(TFAIL, "unexpected: namespace ID inside=%d outside=%d", idinsidens, idoutsidens);
+	} else {
+		tst_res(TPASS, "expected namespaces IDs");
 	}
 
-	TST_SAFE_CHECKPOINT_WAKE(NULL, 0);
-	TST_SAFE_CHECKPOINT_WAKE(NULL, 1);
-	return exit_val;
-}
+	TST_CHECKPOINT_WAKE(0);
+	TST_CHECKPOINT_WAKE(1);
 
-static void cleanup(void)
-{
-	tst_rmdir();
+	return 0;
 }
 
 static void setup(void)
 {
 	check_newuser();
-	tst_tmpdir();
-	TST_CHECKPOINT_INIT(NULL);
 }
 
-int main(int argc, char *argv[])
+static void run(void)
 {
 	pid_t cpid2;
 	char path[BUFSIZ];
-	int lc;
 	int fd;
 	int ret;
 
-	tst_parse_opts(argc, argv, NULL, NULL);
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		parentuid = geteuid();
-		parentgid = getegid();
-
-		cpid1 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
-			(void *)child_fn1, NULL);
-		if (cpid1 < 0)
-			tst_brkm(TBROK | TERRNO, cleanup,
-				"cpid1 clone failed");
-
-		cpid2 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
-			(void *)child_fn2, NULL);
-		if (cpid2 < 0)
-			tst_brkm(TBROK | TERRNO, cleanup,
-				"cpid2 clone failed");
-
-		if (access("/proc/self/setgroups", F_OK) == 0) {
-			sprintf(path, "/proc/%d/setgroups", cpid1);
-			fd = SAFE_OPEN(cleanup, path, O_WRONLY, 0644);
-			SAFE_WRITE(cleanup, 1, fd, "deny", 4);
-			SAFE_CLOSE(cleanup, fd);
-			/* If the setgroups file has the value "deny",
-			 * then the setgroups(2) system call can't
-			 * subsequently be reenabled (by writing "allow" to
-			 * the file) in this user namespace.  (Attempts to
-			 * do so will fail with the error EPERM.)
-			*/
-
-			/* test that setgroups can't be re-enabled */
-			fd = SAFE_OPEN(cleanup, path, O_WRONLY, 0644);
-			ret = write(fd, "allow", 5);
-
-			if (ret != -1) {
-				tst_brkm(TBROK | TERRNO, cleanup,
-					"write action should fail");
-			} else if (errno != EPERM) {
-				tst_brkm(TBROK | TERRNO, cleanup,
-					"unexpected error: \n");
-			}
-			SAFE_CLOSE(cleanup, fd);
-			tst_resm(TPASS, "setgroups can't be re-enabled");
-
-			sprintf(path, "/proc/%d/setgroups", cpid2);
-			fd = SAFE_OPEN(cleanup, path, O_WRONLY, 0644);
-			SAFE_WRITE(cleanup, 1, fd, "deny", 4);
-			SAFE_CLOSE(cleanup, fd);
-		}
-
-		updatemap(cpid1, UID_MAP, CHILD1UID, parentuid, cleanup);
-		updatemap(cpid2, UID_MAP, CHILD2UID, parentuid, cleanup);
-
-		updatemap(cpid1, GID_MAP, CHILD1GID, parentgid, cleanup);
-		updatemap(cpid2, GID_MAP, CHILD2GID, parentgid, cleanup);
-
-		TST_SAFE_CHECKPOINT_WAKE_AND_WAIT(cleanup, 1);
-
-		tst_record_childstatus(cleanup, cpid1);
-		tst_record_childstatus(cleanup, cpid2);
+	parentuid = geteuid();
+	parentgid = getegid();
+
+	cpid1 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn1, NULL);
+	if (cpid1 < 0)
+		tst_brk(TBROK | TTERRNO, "cpid1 clone failed");
+
+	cpid2 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn2, NULL);
+	if (cpid2 < 0)
+		tst_brk(TBROK | TTERRNO, "cpid2 clone failed");
+
+	if (access("/proc/self/setgroups", F_OK) == 0) {
+		sprintf(path, "/proc/%d/setgroups", cpid1);
+
+		fd = SAFE_OPEN(path, O_WRONLY, 0644);
+		SAFE_WRITE(1, fd, "deny", 4);
+		SAFE_CLOSE(fd);
+
+		/* If the setgroups file has the value "deny",
+		 * then the setgroups(2) system call can't
+		 * subsequently be reenabled (by writing "allow" to
+		 * the file) in this user namespace.  (Attempts to
+		 * do so will fail with the error EPERM.)
+		 */
+
+		/* test that setgroups can't be re-enabled */
+		fd = SAFE_OPEN(path, O_WRONLY, 0644);
+		ret = write(fd, "allow", 5);
+
+		if (ret >= 0)
+			tst_brk(TBROK, "write action should fail");
+		else if (errno != EPERM)
+			tst_brk(TBROK | TTERRNO, "unexpected error");
+
+		SAFE_CLOSE(fd);
+
+		tst_res(TPASS, "setgroups can't be re-enabled");
+
+		sprintf(path, "/proc/%d/setgroups", cpid2);
+
+		fd = SAFE_OPEN(path, O_WRONLY, 0644);
+		SAFE_WRITE(1, fd, "deny", 4);
+		SAFE_CLOSE(fd);
 	}
-	cleanup();
-	tst_exit();
+
+	updatemap(cpid1, UID_MAP, CHILD1UID, parentuid);
+	updatemap(cpid2, UID_MAP, CHILD2UID, parentuid);
+
+	updatemap(cpid1, GID_MAP, CHILD1GID, parentgid);
+	updatemap(cpid2, GID_MAP, CHILD2GID, parentgid);
+
+	TST_CHECKPOINT_WAKE_AND_WAIT(1);
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = run,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_USER_NS",
+		NULL,
+	},
+};
-- 
2.35.1


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

Re: [LTP] [PATCH v2 3/8] Rewrite userns03.c using new LTP API

[Petr Vorel]

Hi Andrea,

generally LGTM, few notes below.

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

>  testcases/kernel/containers/userns/userns03.c | 266 ++++++++----------
> +/*\
> + * [Description]
> + *
> + * Verify that /proc/PID/uid_map and /proc/PID/gid_map contains three values
> + * separated by white space:
>   * ID-inside-ns   ID-outside-ns   length
>   *
>   * ID-outside-ns is interpreted according to which process is opening the file.
> @@ -26,29 +21,23 @@
>   * The string "deny" would be written to /proc/self/setgroups before GID
>   * check if setgroups is allowed, see kernel commits:
>   *
> - *   commit 9cc46516ddf497ea16e8d7cb986ae03a0f6b92f8
> - *   Author: Eric W. Biederman <ebiederm@xmission.com>
> - *   Date:   Tue Dec 2 12:27:26 2014 -0600
> - *     userns: Add a knob to disable setgroups on a per user namespace basis
> - *
> - *   commit 66d2f338ee4c449396b6f99f5e75cd18eb6df272
> - *   Author: Eric W. Biederman <ebiederm@xmission.com>
> - *   Date:   Fri Dec 5 19:36:04 2014 -0600
> - *     userns: Allow setting gid_maps without privilege when setgroups is disabled

> + * commit 9cc46516ddf497ea16e8d7cb986ae03a0f6b92f8
> + * Author: Eric W. Biederman <ebiederm@xmission.com>
> + * Date:   Tue Dec 2 12:27:26 2014 -0600
> + * userns: Add a knob to disable setgroups on a per user namespace basis
>   *
> + * commit 66d2f338ee4c449396b6f99f5e75cd18eb6df272
> + * Author: Eric W. Biederman <ebiederm@xmission.com>
> + * Date:   Fri Dec 5 19:36:04 2014 -0600
> + * userns: Allow setting gid_maps without privilege when setgroups is disabled
Commits like these two will be very badly formatted in html/pdf output.
I'd also add some blank lines so that text will be split in paragraphs.
Thus I suggest:

/*\
 * [Description]
 *
 * Verify that /proc/PID/uid_map and /proc/PID/gid_map contains three values
 * separated by white space:
 *
 * ID-inside-ns   ID-outside-ns   length
 *
 * ID-outside-ns is interpreted according to which process is opening the file.
 *
 * If the process opening the file is in the same user namespace as the process
 * PID, then ID-outside-ns is defined with respect to the parent user namespace.
 *
 * If the process opening the file is in a different user namespace, then
 * ID-outside-ns is defined with respect to the user namespace of the process
 * opening the file.
 *
 * The string "deny" would be written to /proc/self/setgroups before GID
 * check if setgroups is allowed, see kernel commits:
 *
 * * 9cc46516ddf4 ("userns: Add a knob to disable setgroups on a per user namespace basis")
 * * 66d2f338ee4c ("userns: Allow setting gid_maps without privilege when setgroups is disabled")
 */


> @@ -75,161 +64,148 @@ static int child_fn1(void)
...
>  	/* map file format:ID-inside-ns   ID-outside-ns   length
> -	If the process opening the file is in the same user namespace as
> -	the process PID, then ID-outside-ns is defined with respect to the
> -	 parent user namespace.*/
> +	 * If the process opening the file is in the same user namespace as
> +	 * the process PID, then ID-outside-ns is defined with respect to the
> +	 * parent user namespace
> +	 */
>  	if (idinsidens != CHILD2UID || idoutsidens != parentuid) {
> -		printf("child_fn2 checks /proc/cpid2/uid_map:\n");
> -		printf("unexpected: idinsidens=%d idoutsidens=%d\n",
> -			idinsidens, idoutsidens);
> -		exit_val = 1;
> +		tst_res(TINFO, "child2 checks /proc/cpid2/uid_map");
nit: I'd put this TINFO before if, so that it's printed also for TPASS.
Also I'd wrote CPID2 to be obvious it's supposed to be a number, or even put
pid value.

> +		tst_res(TFAIL, "unexpected: namespace ID inside=%d outside=%d", idinsidens, idoutsidens);
> +	} else {
> +		tst_res(TPASS, "expected namespaces IDs");
>  	}

>  	sprintf(cpid1uidpath, "/proc/%d/uid_map", cpid1);
> -	SAFE_FILE_SCANF(NULL, cpid1uidpath, "%d %d %d", &idinsidens,
> -		&idoutsidens, &length);
> +	SAFE_FILE_SCANF(cpid1uidpath, "%d %d %d", &idinsidens, &idoutsidens, &length);

>  	/* If the process opening the file is in a different user namespace,
> -	then ID-outside-ns is defined with respect to the user namespace
> -	of the process opening the file.*/
> +	 * then ID-outside-ns is defined with respect to the user namespace
> +	 * of the process opening the file
> +	 */
>  	if (idinsidens != CHILD1UID || idoutsidens != CHILD2UID) {
> -		printf("child_fn2 checks /proc/cpid1/uid_map:\n");
> -		printf("unexpected: idinsidens=%d idoutsidens=%d\n",
> -			idinsidens, idoutsidens);
> -		exit_val = 1;
> +		tst_res(TINFO, "child2 checks /proc/cpid1/uid_map");
And here as well.

> +		tst_res(TFAIL, "unexpected: namespace ID inside=%d outside=%d", idinsidens, idoutsidens);
> +	} else {
> +		tst_res(TPASS, "expected namespaces IDs");
>  	}

>  	sprintf(cpid1gidpath, "/proc/%d/gid_map", cpid1);
> -	SAFE_FILE_SCANF(NULL, "/proc/self/gid_map", "%d %d %d",
> -		 &idinsidens, &idoutsidens, &length);
> +	SAFE_FILE_SCANF("/proc/self/gid_map", "%d %d %d", &idinsidens, &idoutsidens, &length);

>  	if (idinsidens != CHILD2GID || idoutsidens != parentgid) {
> -		printf("child_fn2 checks /proc/cpid2/gid_map:\n");
> -		printf("unexpected: idinsidens=%d idoutsidens=%d\n",
> -			idinsidens, idoutsidens);
> -		exit_val = 1;
> +		tst_res(TINFO, "child2 checks /proc/cpid2/gid_map");
And here.
> +		tst_res(TFAIL, "unexpected: namespace ID inside=%d outside=%d", idinsidens, idoutsidens);
> +	} else {
> +		tst_res(TPASS, "expected namespaces IDs");
>  	}

> -	SAFE_FILE_SCANF(NULL, cpid1gidpath, "%d %d %d", &idinsidens,
> -		&idoutsidens, &length);
> +	SAFE_FILE_SCANF(cpid1gidpath, "%d %d %d", &idinsidens, &idoutsidens, &length);

>  	if (idinsidens != CHILD1GID || idoutsidens != CHILD2GID) {
> -		printf("child_fn1 checks /proc/cpid1/gid_map:\n");
> -		printf("unexpected: idinsidens=%d idoutsidens=%d\n",
> -			idinsidens, idoutsidens);
> -		exit_val = 1;
> +		tst_res(TINFO, "child1 checks /proc/cpid1/gid_map");
And here.
> +		tst_res(TFAIL, "unexpected: namespace ID inside=%d outside=%d", idinsidens, idoutsidens);
> +	} else {
> +		tst_res(TPASS, "expected namespaces IDs");
>  	}

> -int main(int argc, char *argv[])
> +static void run(void)
>  {
> +	parentuid = geteuid();
> +	parentgid = getegid();
> +
> +	cpid1 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn1, NULL);
> +	if (cpid1 < 0)
> +		tst_brk(TBROK | TTERRNO, "cpid1 clone failed");
> +
> +	cpid2 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn2, NULL);
> +	if (cpid2 < 0)
> +		tst_brk(TBROK | TTERRNO, "cpid2 clone failed");
> +
> +	if (access("/proc/self/setgroups", F_OK) == 0) {
> +		sprintf(path, "/proc/%d/setgroups", cpid1);
> +
> +		fd = SAFE_OPEN(path, O_WRONLY, 0644);
> +		SAFE_WRITE(1, fd, "deny", 4);
> +		SAFE_CLOSE(fd);
> +
> +		/* If the setgroups file has the value "deny",
> +		 * then the setgroups(2) system call can't
> +		 * subsequently be reenabled (by writing "allow" to
> +		 * the file) in this user namespace.  (Attempts to
> +		 * do so will fail with the error EPERM.)
> +		 */
> +
> +		/* test that setgroups can't be re-enabled */
> +		fd = SAFE_OPEN(path, O_WRONLY, 0644);
> +		ret = write(fd, "allow", 5);
> +
> +		if (ret >= 0)
nit: Maybe keep ret != -1 (as was in original?)
> +			tst_brk(TBROK, "write action should fail");
> +		else if (errno != EPERM)
> +			tst_brk(TBROK | TTERRNO, "unexpected error");
> +
> +		SAFE_CLOSE(fd);
> +
> +		tst_res(TPASS, "setgroups can't be re-enabled");
> +
> +		sprintf(path, "/proc/%d/setgroups", cpid2);
> +
> +		fd = SAFE_OPEN(path, O_WRONLY, 0644);
> +		SAFE_WRITE(1, fd, "deny", 4);
> +		SAFE_CLOSE(fd);
...

Kind regards,
Petr

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

[LTP] [PATCH v2 4/8] Rewrite userns04.c using new LTP API

[Andrea Cervesato]

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 testcases/kernel/containers/userns/userns04.c | 139 ++++++------------
 1 file changed, 48 insertions(+), 91 deletions(-)

diff --git a/testcases/kernel/containers/userns/userns04.c b/testcases/kernel/containers/userns/userns04.c
index 66d3388a9..63da215e0 100644
--- a/testcases/kernel/containers/userns/userns04.c
+++ b/testcases/kernel/containers/userns/userns04.c
@@ -1,131 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) Huawei Technologies Co., Ltd., 2015
- * 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.
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * Verify that:
- *  If a namespace isn't another namespace's ancestor, the process in
- *  first namespace does not have the CAP_SYS_ADMIN capability in the
- *  second namespace and the setns() call fails.
+/*\
+ * [Description]
+ *
+ * Verify that if a namespace isn't another namespace's ancestor, the process in
+ * first namespace does not have the CAP_SYS_ADMIN capability in the second
+ * namespace and the setns() call fails.
  */
 
 #define _GNU_SOURCE
-#include <sys/wait.h>
-#include <assert.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include "userns_helper.h"
-#include "test.h"
 
-char *TCID = "user_namespace4";
-int TST_TOTAL = 1;
+#include <stdio.h>
+#include <stdbool.h>
+#include "common.h"
+#include "tst_test.h"
+#include "lapi/syscalls.h"
 
 static void setup(void)
 {
 	check_newuser();
 	tst_syscall(__NR_setns, -1, 0);
-	tst_tmpdir();
-	TST_CHECKPOINT_INIT(NULL);
 }
 
-static void cleanup(void)
+static int child_fn1(LTP_ATTRIBUTE_UNUSED void *arg)
 {
-	tst_rmdir();
-}
-
-static int child_fn1(void *arg LTP_ATTRIBUTE_UNUSED)
-{
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAIT(0);
 	return 0;
 }
 
 static int child_fn2(void *arg)
 {
-	int exit_val = 0;
-	int ret;
-
-	ret = tst_syscall(__NR_setns, ((long)arg), CLONE_NEWUSER);
-	if (ret != -1) {
-		printf("child2 setns() unexpected success\n");
-		exit_val = 1;
-	} else if (errno != EPERM) {
-		printf("child2 setns() unexpected error: (%d) %s\n",
-			errno, strerror(errno));
-		exit_val = 1;
-	}
+	TEST(tst_syscall(__NR_setns, ((long)arg), CLONE_NEWUSER));
+	if (TST_RET != -1 || TST_ERR != EPERM)
+		tst_res(TFAIL | TERRNO, "child2 setns() error");
+	else
+		tst_res(TPASS, "child2 setns() failed as expected");
+
+	TST_CHECKPOINT_WAIT(1);
 
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 1);
-	return exit_val;
+	return 0;
 }
 
-static void test_cap_sys_admin(void)
+static void run(void)
 {
 	pid_t cpid1, cpid2, cpid3;
 	char path[BUFSIZ];
 	int fd;
 
-	/* child 1 */
-	cpid1 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
-		(void *)child_fn1, NULL);
+	cpid1 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn1, NULL);
 	if (cpid1 < 0)
-		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
+		tst_brk(TBROK | TTERRNO, "clone failed");
 
-	/* child 2 */
 	sprintf(path, "/proc/%d/ns/user", cpid1);
-	fd = SAFE_OPEN(cleanup, path, O_RDONLY, 0644);
-	cpid2 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
-		(void *)child_fn2, (void *)((long)fd));
+
+	fd = SAFE_OPEN(path, O_RDONLY, 0644);
+	cpid2 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn2, (void *)((long)fd));
 	if (cpid2 < 0)
-		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
+		tst_brk(TBROK | TTERRNO, "clone failed");
 
 	/* child 3 - throw-away process changing ns to child1 */
-	switch (cpid3 = fork()) {
-	case -1:
-		tst_brkm(TBROK | TERRNO, cleanup, "fork");
-	case 0:
-		if (tst_syscall(__NR_setns, fd, CLONE_NEWUSER) == -1) {
-			printf("parent pid setns failure: (%d) %s",
-				errno, strerror(errno));
-			exit(1);
-		}
-		exit(0);
+	cpid3 = SAFE_FORK();
+	if (!cpid3) {
+		TST_EXP_PASS(tst_syscall(__NR_setns, fd, CLONE_NEWUSER));
+		return;
 	}
 
-	TST_SAFE_CHECKPOINT_WAKE(cleanup, 0);
-	TST_SAFE_CHECKPOINT_WAKE(cleanup, 1);
-
-	tst_record_childstatus(cleanup, cpid1);
-	tst_record_childstatus(cleanup, cpid2);
-	tst_record_childstatus(cleanup, cpid3);
-
-	SAFE_CLOSE(cleanup, fd);
+	TST_CHECKPOINT_WAKE(0);
+	TST_CHECKPOINT_WAKE(1);
 
+	SAFE_CLOSE(fd);
 }
 
-int main(int argc, char *argv[])
-{
-	int lc;
-
-	setup();
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		test_cap_sys_admin();
-	}
-
-	cleanup();
-	tst_exit();
-}
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = run,
+	.needs_root = 1,
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_USER_NS",
+		NULL,
+	},
+};
-- 
2.35.1


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

Re: [LTP] [PATCH v2 4/8] Rewrite userns04.c using new LTP API

[Petr Vorel]

Hi Andrea,

LGTM, thanks!

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

>  testcases/kernel/containers/userns/userns04.c | 139 ++++++------------
..
> +#include <stdio.h>
> +#include <stdbool.h>
<stdbool.h> belongs to common.h.

> +#include "common.h"
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"

...
> +static void run(void)
>  {
>  	pid_t cpid1, cpid2, cpid3;
>  	char path[BUFSIZ];
>  	int fd;
...
> +	cpid1 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn1, NULL);
>  	if (cpid1 < 0)
> -		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
> +		tst_brk(TBROK | TTERRNO, "clone failed");
We could create SAFE_LTP_CLONE_QUICK() (as a separate effort).

> -	/* child 2 */
>  	sprintf(path, "/proc/%d/ns/user", cpid1);
> -	fd = SAFE_OPEN(cleanup, path, O_RDONLY, 0644);
> -	cpid2 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
> -		(void *)child_fn2, (void *)((long)fd));
> +
> +	fd = SAFE_OPEN(path, O_RDONLY, 0644);
> +	cpid2 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn2, (void *)((long)fd));
>  	if (cpid2 < 0)
> -		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
> +		tst_brk(TBROK | TTERRNO, "clone failed");
...

Kind regards,
Petr

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

[LTP] [PATCH v2 5/8] Rewrite userns05.c using new LTP API

[Andrea Cervesato]

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 testcases/kernel/containers/userns/userns05.c | 148 ++++++++----------
 1 file changed, 62 insertions(+), 86 deletions(-)

diff --git a/testcases/kernel/containers/userns/userns05.c b/testcases/kernel/containers/userns/userns05.c
index be77cb7e9..94434043b 100644
--- a/testcases/kernel/containers/userns/userns05.c
+++ b/testcases/kernel/containers/userns/userns05.c
@@ -1,51 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) Huawei Technologies Co., Ltd., 2015
- * 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.
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * Verify that:
- * A process created via fork(2) or clone(2) without the
- * CLONE_NEWUSER flag is a member of the same user namespace as its
- * parent.
- * When unshare an user namespace, the calling process is moved into
- * a new user namespace which is not shared with any previously
- * existing process.
+/*\
+ * [Description]
+ *
+ * Verify that if a process created via fork(2) or clone(2) without the
+ * CLONE_NEWUSER flag is a member of the same user namespace as its parent.
+ * When unshare an user namespace, the calling process is moved into a new user
+ * namespace which is not shared with any previously existing process.
  */
 
 #define _GNU_SOURCE
-#include <sys/wait.h>
-#include <assert.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include "userns_helper.h"
-#include "test.h"
-
-char *TCID = "user_namespace5";
-int TST_TOTAL = 1;
 
-static void cleanup(void)
-{
-	tst_rmdir();
-}
+#include <stdio.h>
+#include <stdbool.h>
+#include "tst_test.h"
+#include "common.h"
 
 /*
  * child_fn1() - Inside a new user namespace
  */
 static int child_fn1(void)
 {
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAIT(0);
 	return 0;
 }
 
@@ -57,86 +37,82 @@ static unsigned int getusernsidbypid(int pid)
 
 	sprintf(path, "/proc/%d/ns/user", pid);
 
-	if (readlink(path, userid, BUFSIZ) == -1)
-		tst_resm(TFAIL | TERRNO, "readlink failure.");
+	SAFE_READLINK(path, userid, BUFSIZ);
+
+	if (sscanf(userid, "user:[%u]", &id) < 0)
+		tst_brk(TBROK | TERRNO, "sscanf failure");
 
-	if (sscanf(userid, "user:[%u]", &id) != 1)
-		tst_resm(TFAIL, "sscanf failure.");
 	return id;
 }
 
-static void test_userns_id(void)
+static void run(void)
 {
 	int cpid1, cpid2, cpid3;
 	unsigned int parentuserns, cpid1userns, cpid2userns, newparentuserns;
 
 	parentuserns = getusernsidbypid(getpid());
-	cpid1 = ltp_clone_quick(SIGCHLD, (void *)child_fn1,
-		NULL);
+
+	cpid1 = ltp_clone_quick(SIGCHLD, (void *)child_fn1, NULL);
 	if (cpid1 < 0)
-		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
+		tst_brk(TBROK | TTERRNO, "clone failed");
+
 	cpid1userns = getusernsidbypid(cpid1);
-	TST_SAFE_CHECKPOINT_WAKE(cleanup, 0);
+
+	TST_CHECKPOINT_WAKE(0);
 
 	/* A process created via fork(2) or clone(2) without the
-	CLONE_NEWUSER flag is a member of the same user namespace as its
-	parent.*/
+	 * CLONE_NEWUSER flag is a member of the same user namespace as its
+	 * parent
+	 */
 	if (parentuserns != cpid1userns)
-		tst_resm(TFAIL, "userns:parent should be equal to cpid1");
+		tst_res(TFAIL, "userns:parent should be equal to cpid1");
+	else
+		tst_res(TPASS, "userns:parent is equal to cpid1");
 
-	cpid2 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
-		(void *)child_fn1, NULL);
+	cpid2 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn1, NULL);
 	if (cpid2 < 0)
-		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
+		tst_brk(TBROK | TTERRNO, "clone failed");
+
 	cpid2userns = getusernsidbypid(cpid2);
-	TST_SAFE_CHECKPOINT_WAKE(cleanup, 0);
+
+	TST_CHECKPOINT_WAKE(0);
 
 	if (parentuserns == cpid2userns)
-		tst_resm(TFAIL, "userns:parent should be not equal to cpid2");
-
-	switch (cpid3 = fork()) {
-	case -1:
-		tst_brkm(TBROK | TERRNO, cleanup, "fork");
-	case 0:
-		if (unshare(CLONE_NEWUSER) == -1) {
-			printf("parent pid unshare failure: (%d) %s",
-				errno, strerror(errno));
-			exit(1);
-		}
+		tst_res(TFAIL, "userns:parent should be not equal to cpid2");
+	else
+		tst_res(TPASS, "userns:parent is not equal to cpid2");
+
+	cpid3 = SAFE_FORK();
+	if (!cpid3) {
+		SAFE_UNSHARE(CLONE_NEWUSER);
 		newparentuserns = getusernsidbypid(getpid());
 
 		/* When unshare an user namespace, the calling process
-		is moved into a new user namespace which is not shared
-		with any previously existing process.*/
+		 * is moved into a new user namespace which is not shared
+		 * with any previously existing process
+		 */
 		if (parentuserns == newparentuserns)
-			exit(1);
-		exit(0);
-	}
+			tst_res(TFAIL, "unshared namespaces with same id");
+		else
+			tst_res(TPASS, "unshared namespaces with different id");
 
-	tst_record_childstatus(cleanup, cpid1);
-	tst_record_childstatus(cleanup, cpid2);
-	tst_record_childstatus(cleanup, cpid3);
+		return;
+	}
 }
 
 static void setup(void)
 {
 	check_newuser();
-
-	tst_tmpdir();
-	TST_CHECKPOINT_INIT(NULL);
 }
 
-int main(int argc, char *argv[])
-{
-	int lc;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		test_userns_id();
-	}
-	cleanup();
-	tst_exit();
-}
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = run,
+	.needs_root = 1,
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_USER_NS",
+		NULL,
+	},
+};
-- 
2.35.1


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

Re: [LTP] [PATCH v2 5/8] Rewrite userns05.c using new LTP API

[Petr Vorel]

Hi Andrea,

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

LGTM, just anybody merging this should remove return; at the end of run().

>  testcases/kernel/containers/userns/userns05.c | 148 ++++++++----------
...
> -/*
> - * Verify that:
> - * A process created via fork(2) or clone(2) without the
> - * CLONE_NEWUSER flag is a member of the same user namespace as its
> - * parent.
> - * When unshare an user namespace, the calling process is moved into
> - * a new user namespace which is not shared with any previously
> - * existing process.
> +/*\
> + * [Description]
> + *
> + * Verify that if a process created via fork(2) or clone(2) without the
> + * CLONE_NEWUSER flag is a member of the same user namespace as its parent.

I'd put blank space here to create 2 paragraphs in html/pdf doc.
> + * When unshare an user namespace, the calling process is moved into a new user
> + * namespace which is not shared with any previously existing process.
>   */

...
>  static int child_fn1(void)
>  {
...
> +	SAFE_READLINK(path, userid, BUFSIZ);
> +
> +	if (sscanf(userid, "user:[%u]", &id) < 0)
> +		tst_brk(TBROK | TERRNO, "sscanf failure");
Ah, we still don't have SAFE_SSCANF() (nothing urgent, this is the first test
using new API which would use it).

> +static void run(void)
>  {
...
> +	cpid1 = ltp_clone_quick(SIGCHLD, (void *)child_fn1, NULL);
>  	if (cpid1 < 0)
> +		tst_brk(TBROK | TTERRNO, "clone failed");
Again, once we implement SAFE_LTP_CLONE_QUICK() we should use it here
(as a separate effort).

...
> +	cpid3 = SAFE_FORK();
> +	if (!cpid3) {
> +		SAFE_UNSHARE(CLONE_NEWUSER);
>  		newparentuserns = getusernsidbypid(getpid());

>  		/* When unshare an user namespace, the calling process
> +		 * is moved into a new user namespace which is not shared
> +		 * with any previously existing process
> +		 */
>  		if (parentuserns == newparentuserns)
> +			tst_res(TFAIL, "unshared namespaces with same id");
> +		else
> +			tst_res(TPASS, "unshared namespaces with different id");

> +		return;
Why this empty return?
> +	}
>  }

Kind regards,
Petr

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

[LTP] [PATCH v2 6/8] Rewrite userns06.c using new LTP API

[Andrea Cervesato]

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 testcases/kernel/containers/userns/userns06.c | 179 ++++++++----------
 .../containers/userns/userns06_capcheck.c     |  75 +++++---
 2 files changed, 126 insertions(+), 128 deletions(-)

diff --git a/testcases/kernel/containers/userns/userns06.c b/testcases/kernel/containers/userns/userns06.c
index 29f635de5..2c9af08cc 100644
--- a/testcases/kernel/containers/userns/userns06.c
+++ b/testcases/kernel/containers/userns/userns06.c
@@ -1,65 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) Huawei Technologies Co., Ltd., 2015
- * 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.
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * Verify that:
- * When a process with non-zero user IDs performs an execve(), the process's
- * capability sets are cleared.
+/*\
+ * [Description]
+ *
+ * Verify that when a process with non-zero user IDs performs an execve(),
+ * the process's capability sets are cleared.
  * When a process with zero user IDs performs an execve(), the process's
  * capability sets are set.
- *
  */
 
+#include "tst_test.h"
+#include "config.h"
+
+#ifdef HAVE_LIBCAP
 #define _GNU_SOURCE
-#include <sys/wait.h>
-#include <assert.h>
+
 #include <stdio.h>
-#include <stdlib.h>
 #include <stdbool.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include "libclone.h"
-#include "test.h"
-#include "config.h"
-#include "userns_helper.h"
+#include "common.h"
+
+#define TEST_APP "userns06_capcheck"
 
 #define CHILD1UID 0
 #define CHILD1GID 0
 #define CHILD2UID 200
 #define CHILD2GID 200
 
-char *TCID = "user_namespace6";
-int TST_TOTAL = 1;
-
-static int cpid1, parentuid, parentgid;
-
 /*
  * child_fn1() - Inside a new user namespace
  */
 static int child_fn1(void)
 {
-	int exit_val = 0;
-	char *const args[] = { "userns06_capcheck", "privileged", NULL };
+	char *const args[] = { TEST_APP, "privileged", NULL };
 
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAIT(0);
 
-	if (execve(args[0], args, NULL) == -1) {
-		printf("execvp unexpected error: (%d) %s\n",
-			errno, strerror(errno));
-		exit_val = 1;
-	}
+	/* execv will replace the main function and it will end this child
+	 * accordingly.
+	 */
+	execv(args[0], args);
 
-	return exit_val;
+	return 0;
 }
 
 /*
@@ -67,97 +52,95 @@ static int child_fn1(void)
  */
 static int child_fn2(void)
 {
-	int exit_val = 0;
 	int uid, gid;
-	char *const args[] = { "userns06_capcheck", "unprivileged", NULL };
+	char *const args[] = { TEST_APP, "unprivileged", NULL };
 
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 1);
+	TST_CHECKPOINT_WAIT(1);
 
 	uid = geteuid();
 	gid = getegid();
 
 	if (uid != CHILD2UID || gid != CHILD2GID) {
-		printf("unexpected uid=%d gid=%d\n", uid, gid);
-		exit_val = 1;
+		tst_res(TFAIL, "unexpected uid=%d gid=%d", uid, gid);
+		return 1;
 	}
 
-	if (execve(args[0], args, NULL) == -1) {
-		printf("execvp unexpected error: (%d) %s\n",
-			errno, strerror(errno));
-		exit_val = 1;
-	}
+	tst_res(TPASS, "expected uid and gid");
 
-	return exit_val;
-}
+	/* execv will replace the main function and it will end this child
+	 * accordingly.
+	 */
+	execv(args[0], args);
 
-static void cleanup(void)
-{
-	tst_rmdir();
+	return 0;
 }
 
 static void setup(void)
 {
 	check_newuser();
-	tst_tmpdir();
-	TST_CHECKPOINT_INIT(NULL);
-	TST_RESOURCE_COPY(cleanup, "userns06_capcheck", NULL);
 }
 
-int main(int argc, char *argv[])
+static void run(void)
 {
+	pid_t cpid1;
 	pid_t cpid2;
+	int parentuid;
+	int parentgid;
 	char path[BUFSIZ];
-	int lc;
 	int fd;
 
-	tst_parse_opts(argc, argv, NULL, NULL);
-#ifndef HAVE_LIBCAP
-	tst_brkm(TCONF, NULL, "System is missing libcap.");
-#endif
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
+	parentuid = geteuid();
+	parentgid = getegid();
 
-		parentuid = geteuid();
-		parentgid = getegid();
+	cpid1 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn1, NULL);
+	if (cpid1 < 0)
+		tst_brk(TBROK | TTERRNO, "cpid1 clone failed");
 
-		cpid1 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
-			(void *)child_fn1, NULL);
-		if (cpid1 < 0)
-			tst_brkm(TBROK | TERRNO, cleanup,
-				"cpid1 clone failed");
+	cpid2 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn2, NULL);
+	if (cpid2 < 0)
+		tst_brk(TBROK | TTERRNO, "cpid2 clone failed");
 
-		cpid2 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
-			(void *)child_fn2, NULL);
-		if (cpid2 < 0)
-			tst_brkm(TBROK | TERRNO, cleanup,
-				"cpid2 clone failed");
+	if (access("/proc/self/setgroups", F_OK) == 0) {
+		sprintf(path, "/proc/%d/setgroups", cpid1);
 
-		if (access("/proc/self/setgroups", F_OK) == 0) {
-			sprintf(path, "/proc/%d/setgroups", cpid1);
-			fd = SAFE_OPEN(cleanup, path, O_WRONLY, 0644);
-			SAFE_WRITE(cleanup, 1, fd, "deny", 4);
-			SAFE_CLOSE(cleanup, fd);
+		fd = SAFE_OPEN(path, O_WRONLY, 0644);
+		SAFE_WRITE(1, fd, "deny", 4);
+		SAFE_CLOSE(fd);
 
-			sprintf(path, "/proc/%d/setgroups", cpid2);
-			fd = SAFE_OPEN(cleanup, path, O_WRONLY, 0644);
-			SAFE_WRITE(cleanup, 1, fd, "deny", 4);
-			SAFE_CLOSE(cleanup, fd);
-		}
+		sprintf(path, "/proc/%d/setgroups", cpid2);
 
-		updatemap(cpid1, UID_MAP, CHILD1UID, parentuid, cleanup);
-		updatemap(cpid2, UID_MAP, CHILD2UID, parentuid, cleanup);
+		fd = SAFE_OPEN(path, O_WRONLY, 0644);
+		SAFE_WRITE(1, fd, "deny", 4);
+		SAFE_CLOSE(fd);
+	}
 
-		updatemap(cpid1, GID_MAP, CHILD1GID, parentgid, cleanup);
-		updatemap(cpid2, GID_MAP, CHILD2GID, parentgid, cleanup);
+	updatemap(cpid1, UID_MAP, CHILD1UID, parentuid);
+	updatemap(cpid2, UID_MAP, CHILD2UID, parentuid);
 
-		TST_SAFE_CHECKPOINT_WAKE(cleanup, 0);
-		TST_SAFE_CHECKPOINT_WAKE(cleanup, 1);
+	updatemap(cpid1, GID_MAP, CHILD1GID, parentgid);
+	updatemap(cpid2, GID_MAP, CHILD2GID, parentgid);
 
-		tst_record_childstatus(cleanup, cpid1);
-		tst_record_childstatus(cleanup, cpid2);
-	}
-	cleanup();
-	tst_exit();
+	TST_CHECKPOINT_WAKE(0);
+	TST_CHECKPOINT_WAKE(1);
 }
+
+static const char *const resource_files[] = {
+	TEST_APP,
+	NULL,
+};
+
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = run,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+	.resource_files = resource_files,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_USER_NS",
+		NULL,
+	},
+};
+
+#else
+TST_TEST_TCONF("System is missing libcap");
+#endif
diff --git a/testcases/kernel/containers/userns/userns06_capcheck.c b/testcases/kernel/containers/userns/userns06_capcheck.c
index 31f7e0a25..d8e670fb1 100644
--- a/testcases/kernel/containers/userns/userns06_capcheck.c
+++ b/testcases/kernel/containers/userns/userns06_capcheck.c
@@ -1,62 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) Huawei Technologies Co., Ltd., 2015
- * 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.
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * Verify that:
+/*\
+ * [Description]
+ *
  * When a process with non-zero user IDs performs an execve(), the
  * process's capability sets are cleared. When a process with zero
  * user IDs performs an execve(), the process's capability sets
  * are set.
  */
 
-#define _GNU_SOURCE
-#include <sys/wait.h>
-#include <assert.h>
+#include "config.h"
 #include <stdio.h>
 #include <stdlib.h>
+
+#ifdef HAVE_LIBCAP
+#define _GNU_SOURCE
+
+#include <assert.h>
 #include <unistd.h>
 #include <string.h>
 #include <errno.h>
-#include "libclone.h"
-#include "test.h"
-#include "config.h"
-#if HAVE_SYS_CAPABILITY_H
+#include <sys/wait.h>
 #include <sys/capability.h>
-#endif
-
-char *TCID = "userns06_capcheck";
-int TST_TOTAL = 1;
 
 int main(int argc, char *argv[])
 {
-#ifdef HAVE_LIBCAP
+	FILE *f = NULL;
 	cap_t caps;
 	int i, last_cap;
 	cap_flag_value_t flag_val;
 	cap_flag_value_t expected_flag = 1;
-#endif
-	tst_parse_opts(argc, argv, NULL, NULL);
 
-#ifdef HAVE_LIBCAP
+	if (argc < 2) {
+		printf("userns06_capcheck <privileged|unprivileged>\n");
+		goto error;
+	}
+
+	f = fopen("/proc/sys/kernel/cap_last_cap", "r");
+	if (f == NULL) {
+		printf("fopen error: %s\n", strerror(errno));
+		goto error;
+	}
+
+	if (!fscanf(f, "%d", &last_cap)) {
+		printf("fscanf error: %s\n", strerror(errno));
+		goto error;
+	}
+
 	if (strcmp("privileged", argv[1]))
 		expected_flag = 0;
 
 	caps = cap_get_proc();
-	SAFE_FILE_SCANF(NULL, "/proc/sys/kernel/cap_last_cap", "%d", &last_cap);
+
 	for (i = 0; i <= last_cap; i++) {
 		cap_get_flag(caps, i, CAP_EFFECTIVE, &flag_val);
 		if (flag_val != expected_flag)
 			break;
+
 		cap_get_flag(caps, i, CAP_PERMITTED, &flag_val);
 		if (flag_val != expected_flag)
 			break;
@@ -64,11 +68,22 @@ int main(int argc, char *argv[])
 
 	if (flag_val != expected_flag) {
 		printf("unexpected effective/permitted caps at %d\n", i);
-		exit(1);
+		goto error;
 	}
 
+	exit(0);
+
+error:
+	if (f)
+		fclose(f);
+
+	exit(1);
+}
+
 #else
+int main(void)
+{
 	printf("System is missing libcap.\n");
-#endif
-	tst_exit();
+	exit(1);
 }
+#endif
-- 
2.35.1


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

Re: [LTP] [PATCH v2 6/8] Rewrite userns06.c using new LTP API

[Petr Vorel]

Hi Andrea,

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

Kind regards,
Petr

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

[LTP] [PATCH v2 7/8] Rewrite userns07.c using new LTP API

[Andrea Cervesato]

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 testcases/kernel/containers/userns/userns07.c | 127 +++++++-----------
 1 file changed, 51 insertions(+), 76 deletions(-)

diff --git a/testcases/kernel/containers/userns/userns07.c b/testcases/kernel/containers/userns/userns07.c
index 49915969e..806d103eb 100644
--- a/testcases/kernel/containers/userns/userns07.c
+++ b/testcases/kernel/containers/userns/userns07.c
@@ -1,47 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) Huawei Technologies Co., Ltd., 2015
- * 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.
+ * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * Verify that:
- * The kernel imposes a limit of at least 32 nested levels on user namespaces.
+/*\
+ * [Description]
+ *
+ * Verify that the kernel imposes a limit of at least 32 nested levels on
+ * user namespaces.
  */
 
 #define _GNU_SOURCE
-#include <sys/wait.h>
-#include <assert.h>
+
 #include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include "userns_helper.h"
-#include "test.h"
+#include <stdbool.h>
+#include <sys/wait.h>
+#include "common.h"
+#include "tst_test.h"
 
 #define MAXNEST 32
 
-char *TCID = "userns07";
-int TST_TOTAL = 1;
-
 static void setup(void)
 {
 	check_newuser();
-	tst_tmpdir();
-	TST_CHECKPOINT_INIT(NULL);
-}
-
-static void cleanup(void)
-{
-	tst_rmdir();
 }
 
 static int child_fn1(void *arg)
@@ -52,40 +34,40 @@ static int child_fn1(void *arg)
 	int parentuid;
 	int parentgid;
 
-	TST_SAFE_CHECKPOINT_WAIT(NULL, 0);
+	TST_CHECKPOINT_WAIT(0);
 
-	if (level == MAXNEST)
+	if (level == MAXNEST) {
+		tst_res(TPASS, "nested all children");
 		return 0;
-	cpid1 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
-		(void *)child_fn1, (void *)(level + 1));
-	if (cpid1 < 0) {
-		printf("level %ld:unexpected error: (%d) %s\n",
-			level, errno, strerror(errno));
+	}
+
+	TEST(ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn1, (void *)(level + 1)));
+	if (TST_RET < 0) {
+		tst_res(TFAIL | TERRNO, "level %ld, unexpected error", level);
 		return 1;
 	}
 
+	cpid1 = (int)TST_RET;
+
 	parentuid = geteuid();
 	parentgid = getegid();
 
-	updatemap(cpid1, UID_MAP, 0, parentuid, NULL);
-	updatemap(cpid1, GID_MAP, 0, parentgid, NULL);
+	updatemap(cpid1, UID_MAP, 0, parentuid);
+	updatemap(cpid1, GID_MAP, 0, parentgid);
 
-	TST_SAFE_CHECKPOINT_WAKE(cleanup, 0);
+	TST_CHECKPOINT_WAKE(0);
 
-	if (waitpid(cpid1, &status, 0) == -1)
-		return 1;
+	SAFE_WAITPID(cpid1, &status, 0);
+
+	if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
+		tst_brk(TBROK | TERRNO, "child exited abnormally %s", tst_strstatus(status));
+	else if (WIFSIGNALED(status))
+		tst_brk(TBROK | TERRNO, "child was killed with signal = %d", WTERMSIG(status));
 
-	if (WIFEXITED(status) && WEXITSTATUS(status) != 0) {
-		printf("child exited abnormally\n");
-		return 1;
-	} else if (WIFSIGNALED(status)) {
-		printf("child was killed with signal = %d", WTERMSIG(status));
-		return 1;
-	}
 	return 0;
 }
 
-static void test_max_nest(void)
+static void run(void)
 {
 	pid_t cpid1;
 	int parentuid;
@@ -93,41 +75,34 @@ static void test_max_nest(void)
 	int fd;
 	char path[BUFSIZ];
 
-	cpid1 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
-		(void *)child_fn1, (void *)0);
+	cpid1 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, (void *)child_fn1, (void *)0);
 	if (cpid1 < 0)
-		tst_brkm(TBROK | TERRNO, cleanup, "clone failed");
+		tst_brk(TBROK | TTERRNO, "clone failed");
 
 	parentuid = geteuid();
 	parentgid = getegid();
 
 	if (access("/proc/self/setgroups", F_OK) == 0) {
 		sprintf(path, "/proc/%d/setgroups", cpid1);
-		fd = SAFE_OPEN(cleanup, path, O_WRONLY, 0644);
-		SAFE_WRITE(cleanup, 1, fd, "deny", 4);
-		SAFE_CLOSE(cleanup, fd);
-	}
-
-	updatemap(cpid1, UID_MAP, 0, parentuid, cleanup);
-	updatemap(cpid1, GID_MAP, 0, parentgid, cleanup);
 
-	TST_SAFE_CHECKPOINT_WAKE(cleanup, 0);
-	tst_record_childstatus(cleanup, cpid1);
-}
-
-int main(int argc, char *argv[])
-{
-	int lc;
-
-	setup();
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		test_max_nest();
+		fd = SAFE_OPEN(path, O_WRONLY, 0644);
+		SAFE_WRITE(1, fd, "deny", 4);
+		SAFE_CLOSE(fd);
 	}
 
-	cleanup();
-	tst_exit();
+	updatemap(cpid1, UID_MAP, 0, parentuid);
+	updatemap(cpid1, GID_MAP, 0, parentgid);
+
+	TST_CHECKPOINT_WAKE(0);
 }
 
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = run,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_USER_NS",
+		NULL,
+	},
+};
-- 
2.35.1


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

Re: [LTP] [PATCH v2 7/8] Rewrite userns07.c using new LTP API

[Petr Vorel]

Hi Andrea,

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

Kind regards,
Petr

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

[LTP] [PATCH v2 8/8] Rewrite userns08.c using new LTP API

[Andrea Cervesato]

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 testcases/kernel/containers/userns/userns08.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/testcases/kernel/containers/userns/userns08.c b/testcases/kernel/containers/userns/userns08.c
index bde944f22..1180be494 100644
--- a/testcases/kernel/containers/userns/userns08.c
+++ b/testcases/kernel/containers/userns/userns08.c
@@ -17,6 +17,7 @@
  * by (the real) root. So on the second level we reset dumpable to 1.
  *
  */
+
 #define _GNU_SOURCE
 #include <stdio.h>
 #include <stdlib.h>
@@ -30,16 +31,12 @@
 
 static pid_t clone_newuser(void)
 {
-	const struct tst_clone_args cargs = {
-		CLONE_NEWUSER,
-		SIGCHLD
-	};
+	const struct tst_clone_args cargs = { CLONE_NEWUSER, SIGCHLD };
 
 	return SAFE_CLONE(&cargs);
 }
 
-static void write_mapping(const pid_t proc_in_ns,
-			  const char *const id_mapping)
+static void write_mapping(const pid_t proc_in_ns, const char *const id_mapping)
 {
 	char proc_path[PATH_MAX];
 	int proc_dir;
@@ -61,11 +58,11 @@ static void write_mapping(const pid_t proc_in_ns,
 static void ns_level2(void)
 {
 	if (prctl(PR_SET_DUMPABLE, 1, 0, 0, 0))
-		tst_res(TINFO | TERRNO, "Failed to set dumpable flag");
+		tst_brk(TBROK | TTERRNO, "Failed to set dumpable flag");
+
 	TST_CHECKPOINT_WAKE_AND_WAIT(1);
 
-	TST_EXP_FAIL(open("restricted", O_WRONLY), EACCES,
-		     "Denied write access to ./restricted");
+	TST_EXP_FAIL(open("restricted", O_WRONLY), EACCES, "Denied write access to ./restricted");
 
 	exit(0);
 }
@@ -89,7 +86,6 @@ static void ns_level1(void)
 	write_mapping(level2_proc, map_over_5);
 
 	TST_CHECKPOINT_WAKE(1);
-	tst_reap_children();
 
 	exit(0);
 }
@@ -111,7 +107,6 @@ static void run(void)
 	write_mapping(level1_proc, "0 100000 1000");
 
 	TST_CHECKPOINT_WAKE(0);
-	tst_reap_children();
 }
 
 static void setup(void)
-- 
2.35.1


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

Re: [LTP] [PATCH v2 8/8] Rewrite userns08.c using new LTP API

[Petr Vorel]

Hi Andrea,

./userns08 -i50
tst_kconfig.c:82: TINFO: Parsing kernel config '/proc/config.gz'
tst_test.c:1456: TINFO: Timeout per run is 0h 05m 00s

userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
userns08.c:36: TBROK: clone3 failed: ENOSPC (28)

Something needs to be closed after each run.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 8/8] Rewrite userns08.c using new LTP API

[Andrea Cervesato via ltp]

[-- Attachment #1.1: Type: text/plain, Size: 793 bytes --]

Hi Petr,

this is really strange. Thanks for the review, I'm going to check 
where's the bug.

Andrea

On 3/23/22 10:36, Petr Vorel wrote:
> Hi Andrea,
>
> ./userns08 -i50
> tst_kconfig.c:82: TINFO: Parsing kernel config '/proc/config.gz'
> tst_test.c:1456: TINFO: Timeout per run is 0h 05m 00s
>
> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> userns08.c:36: TBROK: clone3 failed: ENOSPC (28)
>
> Something needs to be closed after each run.
>
> Kind regards,
> Petr
>

[-- Attachment #1.2: Type: text/html, Size: 1183 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

Re: [LTP] [PATCH v2 8/8] Rewrite userns08.c using new LTP API

[Cyril Hrubis]

Hi!
> ./userns08 -i50
> tst_kconfig.c:82: TINFO: Parsing kernel config '/proc/config.gz'
> tst_test.c:1456: TINFO: Timeout per run is 0h 05m 00s
> 
> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> userns08.c:36: TBROK: clone3 failed: ENOSPC (28)
> 
> Something needs to be closed after each run.

ENOSPC means that we created too many user namespaces. The problem is
likely that we are creating the namespaces faster than they are being
asynchronously cleaned up in the kernel. Adding sleep(1) to the
clone_newuser() function gives kernel enough time to clean the
namespaces and the test works with any -i. Also note that we get the
exact same failure if we execute the test a few times in a row, i.e.

for i in `seq 10`; do
	./userns08
done

The original test fails in the same way, so while it should be fixed,
it's not really reason to block this patchset.

And the only correct fix would be retrying the clone() on ENOSPC in the
SAFE_CLONE().

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 8/8] Rewrite userns08.c using new LTP API

[Petr Vorel]

> Hi!
> > ./userns08 -i50
> > tst_kconfig.c:82: TINFO: Parsing kernel config '/proc/config.gz'
> > tst_test.c:1456: TINFO: Timeout per run is 0h 05m 00s

> > userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> > userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> > userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> > userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> > userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> > userns08.c:36: TBROK: clone3 failed: ENOSPC (28)

> > Something needs to be closed after each run.

> ENOSPC means that we created too many user namespaces. The problem is
> likely that we are creating the namespaces faster than they are being
> asynchronously cleaned up in the kernel. Adding sleep(1) to the
> clone_newuser() function gives kernel enough time to clean the
> namespaces and the test works with any -i. Also note that we get the
> exact same failure if we execute the test a few times in a row, i.e.

> for i in `seq 10`; do
> 	./userns08
> done
+1

> The original test fails in the same way, so while it should be fixed,
> it's not really reason to block this patchset.
Agree (I forget to write I suspected the problem wasn't new in this patchset).

> And the only correct fix would be retrying the clone() on ENOSPC in the
> SAFE_CLONE().
+1. I suppose Andrea will have look into it (otherwise I'll do it).

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 8/8] Rewrite userns08.c using new LTP API

[Andrea Cervesato via ltp]

[-- Attachment #1.1: Type: text/plain, Size: 1716 bytes --]

Hi Petr,

I can check it, but it will probably take some time because I'm not 
familiar with that particular scenario. Can we go ahead with the 
patch-set anyway?

Andrea

On 3/23/22 13:54, Petr Vorel wrote:
>> Hi!
>>> ./userns08 -i50
>>> tst_kconfig.c:82: TINFO: Parsing kernel config '/proc/config.gz'
>>> tst_test.c:1456: TINFO: Timeout per run is 0h 05m 00s
>>> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
>>> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
>>> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
>>> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
>>> userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
>>> userns08.c:36: TBROK: clone3 failed: ENOSPC (28)
>>> Something needs to be closed after each run.
>> ENOSPC means that we created too many user namespaces. The problem is
>> likely that we are creating the namespaces faster than they are being
>> asynchronously cleaned up in the kernel. Adding sleep(1) to the
>> clone_newuser() function gives kernel enough time to clean the
>> namespaces and the test works with any -i. Also note that we get the
>> exact same failure if we execute the test a few times in a row, i.e.
>> for i in `seq 10`; do
>> 	./userns08
>> done
> +1
>
>> The original test fails in the same way, so while it should be fixed,
>> it's not really reason to block this patchset.
> Agree (I forget to write I suspected the problem wasn't new in this patchset).
>
>> And the only correct fix would be retrying the clone() on ENOSPC in the
>> SAFE_CLONE().
> +1. I suppose Andrea will have look into it (otherwise I'll do it).
>
> Kind regards,
> Petr
>

[-- Attachment #1.2: Type: text/html, Size: 3255 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

Re: [LTP] [PATCH v2 8/8] Rewrite userns08.c using new LTP API

[Petr Vorel]

Hi Andrea,

> Hi Petr,

> I can check it, but it will probably take some time because I'm not familiar
> with that particular scenario. Can we go ahead with the patch-set anyway?

OK, I'll implement the fix in the library.
And for sure this is not a blocker for your patchset.
I've just haven't finishing reviewing it.

Kind regards,
Petr

> Andrea

> On 3/23/22 13:54, Petr Vorel wrote:
> > > Hi!
> > > > ./userns08 -i50
> > > > tst_kconfig.c:82: TINFO: Parsing kernel config '/proc/config.gz'
> > > > tst_test.c:1456: TINFO: Timeout per run is 0h 05m 00s
> > > > userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> > > > userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> > > > userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> > > > userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> > > > userns08.c:65: TPASS: Denied write access to ./restricted : EACCES (13)
> > > > userns08.c:36: TBROK: clone3 failed: ENOSPC (28)
> > > > Something needs to be closed after each run.
> > > ENOSPC means that we created too many user namespaces. The problem is
> > > likely that we are creating the namespaces faster than they are being
> > > asynchronously cleaned up in the kernel. Adding sleep(1) to the
> > > clone_newuser() function gives kernel enough time to clean the
> > > namespaces and the test works with any -i. Also note that we get the
> > > exact same failure if we execute the test a few times in a row, i.e.
> > > for i in `seq 10`; do
> > > 	./userns08
> > > done
> > +1

> > > The original test fails in the same way, so while it should be fixed,
> > > it's not really reason to block this patchset.
> > Agree (I forget to write I suspected the problem wasn't new in this patchset).

> > > And the only correct fix would be retrying the clone() on ENOSPC in the
> > > SAFE_CLONE().
> > +1. I suppose Andrea will have look into it (otherwise I'll do it).

> > Kind regards,
> > Petr


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

Re: [LTP] [PATCH v2 8/8] Rewrite userns08.c using new LTP API

[Petr Vorel]

Hi Andrea,

[Cc Richie]

This subject: "Rewrite userns08.c using new LTP API" is misleading,
userns08.c was already using new API.

You're:
* s/tst_res/tst_brk/ (that would deserve explanation why)
* removing tst_reap_children()
* changing formatting (some of them create too long lines)

...
>  static pid_t clone_newuser(void)
>  {
> -	const struct tst_clone_args cargs = {
> -		CLONE_NEWUSER,
> -		SIGCHLD
> -	};
> +	const struct tst_clone_args cargs = { CLONE_NEWUSER, SIGCHLD };

>  	return SAFE_CLONE(&cargs);
>  }

> -static void write_mapping(const pid_t proc_in_ns,
> -			  const char *const id_mapping)
> +static void write_mapping(const pid_t proc_in_ns, const char *const id_mapping)
>  {
>  	char proc_path[PATH_MAX];
>  	int proc_dir;
> @@ -61,11 +58,11 @@ static void write_mapping(const pid_t proc_in_ns,
>  static void ns_level2(void)
>  {
>  	if (prctl(PR_SET_DUMPABLE, 1, 0, 0, 0))
> -		tst_res(TINFO | TERRNO, "Failed to set dumpable flag");
> +		tst_brk(TBROK | TTERRNO, "Failed to set dumpable flag");
Not sure which one is correct (whether tst_res or tst_brk).
But TTERRNO is obviously wrong, that's for using TEST(). Here should remain
TERRNO.

> +
>  	TST_CHECKPOINT_WAKE_AND_WAIT(1);

> -	TST_EXP_FAIL(open("restricted", O_WRONLY), EACCES,
> -		     "Denied write access to ./restricted");
> +	TST_EXP_FAIL(open("restricted", O_WRONLY), EACCES, "Denied write access to ./restricted");

I'd keep this one.

>  	exit(0);
>  }
> @@ -89,7 +86,6 @@ static void ns_level1(void)
>  	write_mapping(level2_proc, map_over_5);

>  	TST_CHECKPOINT_WAKE(1);
> -	tst_reap_children();
Well, test works without it, but not really sure if it can be removed.

Kind regards,
Petr

>  	exit(0);
>  }
> @@ -111,7 +107,6 @@ static void run(void)
>  	write_mapping(level1_proc, "0 100000 1000");

>  	TST_CHECKPOINT_WAKE(0);
> -	tst_reap_children();

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

Re: [LTP] [PATCH v2 8/8] Rewrite userns08.c using new LTP API

[Andrea Cervesato via ltp]

[-- Attachment #1.1: Type: text/plain, Size: 2046 bytes --]

Hi Petr,

I think this patch can be avoided at this point

Andrea

On 3/25/22 08:18, Petr Vorel wrote:
> Hi Andrea,
>
> [Cc Richie]
>
> This subject: "Rewrite userns08.c using new LTP API" is misleading,
> userns08.c was already using new API.
>
> You're:
> * s/tst_res/tst_brk/ (that would deserve explanation why)
> * removing tst_reap_children()
> * changing formatting (some of them create too long lines)
>
> ...
>>   static pid_t clone_newuser(void)
>>   {
>> -	const struct tst_clone_args cargs = {
>> -		CLONE_NEWUSER,
>> -		SIGCHLD
>> -	};
>> +	const struct tst_clone_args cargs = { CLONE_NEWUSER, SIGCHLD };
>>   	return SAFE_CLONE(&cargs);
>>   }
>> -static void write_mapping(const pid_t proc_in_ns,
>> -			  const char *const id_mapping)
>> +static void write_mapping(const pid_t proc_in_ns, const char *const id_mapping)
>>   {
>>   	char proc_path[PATH_MAX];
>>   	int proc_dir;
>> @@ -61,11 +58,11 @@ static void write_mapping(const pid_t proc_in_ns,
>>   static void ns_level2(void)
>>   {
>>   	if (prctl(PR_SET_DUMPABLE, 1, 0, 0, 0))
>> -		tst_res(TINFO | TERRNO, "Failed to set dumpable flag");
>> +		tst_brk(TBROK | TTERRNO, "Failed to set dumpable flag");
> Not sure which one is correct (whether tst_res or tst_brk).
> But TTERRNO is obviously wrong, that's for using TEST(). Here should remain
> TERRNO.
>
>> +
>>   	TST_CHECKPOINT_WAKE_AND_WAIT(1);
>> -	TST_EXP_FAIL(open("restricted", O_WRONLY), EACCES,
>> -		     "Denied write access to ./restricted");
>> +	TST_EXP_FAIL(open("restricted", O_WRONLY), EACCES, "Denied write access to ./restricted");
> I'd keep this one.
>
>>   	exit(0);
>>   }
>> @@ -89,7 +86,6 @@ static void ns_level1(void)
>>   	write_mapping(level2_proc, map_over_5);
>>   	TST_CHECKPOINT_WAKE(1);
>> -	tst_reap_children();
> Well, test works without it, but not really sure if it can be removed.
>
> Kind regards,
> Petr
>
>>   	exit(0);
>>   }
>> @@ -111,7 +107,6 @@ static void run(void)
>>   	write_mapping(level1_proc, "0 100000 1000");
>>   	TST_CHECKPOINT_WAKE(0);
>> -	tst_reap_children();

[-- Attachment #1.2: Type: text/html, Size: 3717 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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