fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/8] Extend idmapped mount testsuite
@ 2021-08-12 16:01 Christian Brauner
  2021-08-12 16:01 ` [PATCH v3 1/8] idmapped-mounts: use die() helper Christian Brauner
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Christian Brauner @ 2021-08-12 16:01 UTC (permalink / raw)
  To: fstests, Eryu Guan, Christoph Hellwig; +Cc: Christian Brauner

From: Christian Brauner <christian.brauner@ubuntu.com>

Hey everyone,

This is v3 with the changes requested by Christoph and his Reviewed-bys
added. I've also added Josef's Reviewed-by for the newly added btrfs
specific idmapped mount testsuite.
There are no major changes to v2.

This time around I've put everyone in To: to make sure that they receive
all patches. The list apparently still refues patches if they are fairly
huge. The series can be pulled from three locations:

git@gitolite.kernel.org:pub/scm/linux/kernel/git/brauner/xfstests-dev fs.idmapped.nested_userns
git@gitlab.com:brauner/xfstests.git fs.idmapped.nested_userns
git@github.com:brauner/xfstests.git fs.idmapped.nested_userns

This adds three new tests:
- a regression test for vfs capabilities
- a new test with nested and complex idmapping layouts
- a new btrfs specific idmapped mount testsuite

Since v2 patches to support idmapped mounts on btrfs have been merged
into btrfs' for-next tree and so are on track to be included for v5.15.
I'd like to get the btrfs tests merged now so linux-next can be tested
with the them. I hope that's ok.

Thanks!
Christian

Christian Brauner (8):
  idmapped-mounts: use die() helper
  idmapped-mounts: switch to getopt_long_only()
  idmapped-mounts: introduce an explicit command line switch for
    testsuite
  generic/640: add fscaps regression test
  idmapped-mounts: refactor helpers
  idmapped-mounts: add nested userns creation helpers
  generic/641: add nested user namespace tests
  btrfs/244: introduce btrfs specific idmapped mounts tests

 configure.ac                          |   10 +-
 src/idmapped-mounts/idmapped-mounts.c | 4747 ++++++++++++++++++++++++-
 src/idmapped-mounts/mount-idmapped.c  |  229 +-
 src/idmapped-mounts/utils.c           |  359 +-
 src/idmapped-mounts/utils.h           |  102 +-
 tests/btrfs/244                       |   34 +
 tests/btrfs/244.out                   |    2 +
 tests/generic/633                     |    3 +-
 tests/generic/640                     |   28 +
 tests/generic/640.out                 |    2 +
 tests/generic/641                     |   28 +
 tests/generic/641.out                 |    2 +
 12 files changed, 5231 insertions(+), 315 deletions(-)
 create mode 100755 tests/btrfs/244
 create mode 100644 tests/btrfs/244.out
 create mode 100755 tests/generic/640
 create mode 100644 tests/generic/640.out
 create mode 100755 tests/generic/641
 create mode 100644 tests/generic/641.out


base-commit: dad0c0a852d1b10e7da285f29e99397dec0efec1
-- 
2.30.2


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

* [PATCH v3 1/8] idmapped-mounts: use die() helper
  2021-08-12 16:01 [PATCH v3 0/8] Extend idmapped mount testsuite Christian Brauner
@ 2021-08-12 16:01 ` Christian Brauner
  2021-08-14  8:39   ` Christoph Hellwig
  2021-08-12 16:01 ` [PATCH v3 2/8] idmapped-mounts: switch to getopt_long_only() Christian Brauner
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Christian Brauner @ 2021-08-12 16:01 UTC (permalink / raw)
  To: fstests, Eryu Guan, Christoph Hellwig; +Cc: Christian Brauner

From: Christian Brauner <christian.brauner@ubuntu.com>

Use the dedicated helper to report an error and exit with failure
instead of hand-rolling it.

Cc: Christoph Hellwig <hch@lst.de>
Cc: fstests@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
patch not present

/* v3 */
- Christoph Hellwig <hch@lst.de>:
  - Split into separate patch.
---
 src/idmapped-mounts/idmapped-mounts.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
index 2c212131..69dcc027 100644
--- a/src/idmapped-mounts/idmapped-mounts.c
+++ b/src/idmapped-mounts/idmapped-mounts.c
@@ -8804,10 +8804,8 @@ static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
 
 		if (pid == 0) {
 			ret = t->test();
-			if (ret) {
-				fprintf(stderr, "failure: %s\n", t->description);
-				exit(EXIT_FAILURE);
-			}
+			if (ret)
+				die("failure: %s", t->description);
 
 			exit(EXIT_SUCCESS);
 		}
-- 
2.30.2


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

* [PATCH v3 2/8] idmapped-mounts: switch to getopt_long_only()
  2021-08-12 16:01 [PATCH v3 0/8] Extend idmapped mount testsuite Christian Brauner
  2021-08-12 16:01 ` [PATCH v3 1/8] idmapped-mounts: use die() helper Christian Brauner
@ 2021-08-12 16:01 ` Christian Brauner
  2021-08-14  8:42   ` Christoph Hellwig
  2021-08-12 16:01 ` [PATCH v3 3/8] idmapped-mounts: introduce an explicit command line switch for testsuite Christian Brauner
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Christian Brauner @ 2021-08-12 16:01 UTC (permalink / raw)
  To: fstests, Eryu Guan, Christoph Hellwig; +Cc: Christian Brauner

From: Christian Brauner <christian.brauner@ubuntu.com>

We're not using the shortopts anywhere anyway so just rely on longopts.

Cc: fstests@vger.kernel.org
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
patch not present

/* v3 */
- Christoph Hellwig <hch@lst.de>:
  - Split into separate patch.
---
 src/idmapped-mounts/idmapped-mounts.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
index 69dcc027..e565246e 100644
--- a/src/idmapped-mounts/idmapped-mounts.c
+++ b/src/idmapped-mounts/idmapped-mounts.c
@@ -8717,8 +8717,11 @@ static void usage(void)
 	fprintf(stderr, "    Run idmapped mount tests\n\n");
 
 	fprintf(stderr, "Arguments:\n");
-	fprintf(stderr, "-d --device        Device used in the tests\n");
-	fprintf(stderr, "-m --mountpoint    Mountpoint of device\n");
+	fprintf(stderr, "--device        Device used in the tests\n");
+	fprintf(stderr, "--fstype        Filesystem type used in the tests\n");
+	fprintf(stderr, "--help          Print help\n");
+	fprintf(stderr, "--mountpoint    Mountpoint of device\n");
+	fprintf(stderr, "--supported     Test whether idmapped mounts are supported on this filesystem\n");
 
 	_exit(EXIT_SUCCESS);
 }
@@ -8826,7 +8829,7 @@ int main(int argc, char *argv[])
 	int index = 0;
 	bool supported = false;
 
-	while ((ret = getopt_long(argc, argv, "", longopts, &index)) != -1) {
+	while ((ret = getopt_long_only(argc, argv, "d:f:m:sh", longopts, &index)) != -1) {
 		switch (ret) {
 		case 'd':
 			t_device = optarg;
-- 
2.30.2


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

* [PATCH v3 3/8] idmapped-mounts: introduce an explicit command line switch for testsuite
  2021-08-12 16:01 [PATCH v3 0/8] Extend idmapped mount testsuite Christian Brauner
  2021-08-12 16:01 ` [PATCH v3 1/8] idmapped-mounts: use die() helper Christian Brauner
  2021-08-12 16:01 ` [PATCH v3 2/8] idmapped-mounts: switch to getopt_long_only() Christian Brauner
@ 2021-08-12 16:01 ` Christian Brauner
  2021-08-14  8:42   ` Christoph Hellwig
  2021-08-12 16:01 ` [PATCH v3 4/8] generic/640: add fscaps regression test Christian Brauner
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Christian Brauner @ 2021-08-12 16:01 UTC (permalink / raw)
  To: fstests, Eryu Guan, Christoph Hellwig; +Cc: Christian Brauner

From: Christian Brauner <christian.brauner@ubuntu.com>

Introduce an explicit command line switch to runs the basic test suite.
This prepares for the introduction of additional command line switches
to run additional tests.

Cc: Christoph Hellwig <hch@lst.de>
Cc: fstests@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
patch not present

/* v3 */
- Christoph Hellwig <hch@lst.de>:
  - Split into separate patch.
---
 src/idmapped-mounts/idmapped-mounts.c | 11 ++++++++---
 tests/generic/633                     |  3 ++-
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
index e565246e..7723a222 100644
--- a/src/idmapped-mounts/idmapped-mounts.c
+++ b/src/idmapped-mounts/idmapped-mounts.c
@@ -8722,6 +8722,7 @@ static void usage(void)
 	fprintf(stderr, "--help          Print help\n");
 	fprintf(stderr, "--mountpoint    Mountpoint of device\n");
 	fprintf(stderr, "--supported     Test whether idmapped mounts are supported on this filesystem\n");
+	fprintf(stderr, "--test-core     Run core idmapped mount testsuite\n");
 
 	_exit(EXIT_SUCCESS);
 }
@@ -8732,7 +8733,8 @@ static const struct option longopts[] = {
 	{"mountpoint",	required_argument,	0,	'm'},
 	{"supported",	no_argument,		0,	's'},
 	{"help",	no_argument,		0,	'h'},
-	{NULL,		0,			0,	0  },
+	{"test-core",	no_argument,		0,	'c'},
+	{NULL,		0,			0,	0},
 };
 
 struct t_idmapped_mounts {
@@ -8827,7 +8829,7 @@ int main(int argc, char *argv[])
 {
 	int fret, ret;
 	int index = 0;
-	bool supported = false;
+	bool supported = false, test_core = false;
 
 	while ((ret = getopt_long_only(argc, argv, "d:f:m:sh", longopts, &index)) != -1) {
 		switch (ret) {
@@ -8843,6 +8845,9 @@ int main(int argc, char *argv[])
 		case 's':
 			supported = true;
 			break;
+		case 'c':
+			test_core = true;
+			break;
 		case 'h':
 			/* fallthrough */
 		default:
@@ -8912,7 +8917,7 @@ int main(int argc, char *argv[])
 
 	fret = EXIT_FAILURE;
 
-	if (!run_test(basic_suite, ARRAY_SIZE(basic_suite)))
+	if (test_core && !run_test(basic_suite, ARRAY_SIZE(basic_suite)))
 		goto out;
 
 	fret = EXIT_SUCCESS;
diff --git a/tests/generic/633 b/tests/generic/633
index 6be8a69e..67501177 100755
--- a/tests/generic/633
+++ b/tests/generic/633
@@ -20,7 +20,8 @@ _require_test
 
 echo "Silence is golden"
 
-$here/src/idmapped-mounts/idmapped-mounts --device "$TEST_DEV" --mount "$TEST_DIR" --fstype "$FSTYP"
+$here/src/idmapped-mounts/idmapped-mounts --test-core --device "$TEST_DEV" \
+	--mount "$TEST_DIR" --fstype "$FSTYP"
 
 status=$?
 exit
-- 
2.30.2


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

* [PATCH v3 4/8] generic/640: add fscaps regression test
  2021-08-12 16:01 [PATCH v3 0/8] Extend idmapped mount testsuite Christian Brauner
                   ` (2 preceding siblings ...)
  2021-08-12 16:01 ` [PATCH v3 3/8] idmapped-mounts: introduce an explicit command line switch for testsuite Christian Brauner
@ 2021-08-12 16:01 ` Christian Brauner
  2021-08-12 16:01 ` [PATCH v3 5/8] idmapped-mounts: refactor helpers Christian Brauner
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2021-08-12 16:01 UTC (permalink / raw)
  To: fstests, Eryu Guan, Christoph Hellwig; +Cc: Christian Brauner

From: Christian Brauner <christian.brauner@ubuntu.com>

Add a test to verify that setting a v3 fscap from an idmapped mount
works as expected. This and other related use-cases were regressed by
commit [1] which was reverted in [2] and the proper fix merged right
before v5.12 was released in [3].

[1]: commit 3b0c2d3eaa83 ("Revert 95ebabde382c ("capabilities: Don't allow writing ambiguous v3 file capabilities")")
[2]: commit 95ebabde382c ("capabilities: Don't allow writing ambiguous v3 file capabilities")
[3]: commit db2e718a4798 ("capabilities: require CAP_SETFCAP to map uid 0")

Cc: fstests@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Eryu Guan <guan@eryu.me>:
  - Don't create symlinks for each test. Instead, use the same binary
    and introduce new options to specify which tests to run.

/* v3 */
unchanged
---
 src/idmapped-mounts/idmapped-mounts.c | 161 +++++++++++++++++++++++---
 tests/generic/640                     |  28 +++++
 tests/generic/640.out                 |   2 +
 3 files changed, 175 insertions(+), 16 deletions(-)
 create mode 100755 tests/generic/640
 create mode 100644 tests/generic/640.out

diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
index 7723a222..0b2d7a52 100644
--- a/src/idmapped-mounts/idmapped-mounts.c
+++ b/src/idmapped-mounts/idmapped-mounts.c
@@ -3199,6 +3199,121 @@ out:
 	return fret;
 }
 
+static int fscaps_idmapped_mounts_in_userns_valid_in_ancestor_userns(void)
+{
+	int fret = -1;
+	int file1_fd = -EBADF, file1_fd2 = -EBADF, open_tree_fd = -EBADF;
+	struct mount_attr attr = {
+		.attr_set = MOUNT_ATTR_IDMAP,
+	};
+	pid_t pid;
+
+	file1_fd = openat(t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, 0644);
+	if (file1_fd < 0) {
+		log_stderr("failure: openat");
+		goto out;
+	}
+
+	/* Skip if vfs caps are unsupported. */
+	if (set_dummy_vfs_caps(file1_fd, 0, 1000))
+		return 0;
+
+	if (fremovexattr(file1_fd, "security.capability")) {
+		log_stderr("failure: fremovexattr");
+		goto out;
+	}
+	if (expected_dummy_vfs_caps_uid(file1_fd, -1)) {
+		log_stderr("failure: expected_dummy_vfs_caps_uid");
+		goto out;
+	}
+	if (errno != ENODATA) {
+		log_stderr("failure: errno");
+		goto out;
+	}
+
+	/* Changing mount properties on a detached mount. */
+	attr.userns_fd	= get_userns_fd(0, 10000, 10000);
+	if (attr.userns_fd < 0) {
+		log_stderr("failure: get_userns_fd");
+		goto out;
+	}
+
+	open_tree_fd = sys_open_tree(t_dir1_fd, "",
+				     AT_EMPTY_PATH |
+				     AT_NO_AUTOMOUNT |
+				     AT_SYMLINK_NOFOLLOW |
+				     OPEN_TREE_CLOEXEC |
+				     OPEN_TREE_CLONE);
+	if (open_tree_fd < 0) {
+		log_stderr("failure: sys_open_tree");
+		goto out;
+	}
+
+	if (sys_mount_setattr(open_tree_fd, "", AT_EMPTY_PATH, &attr, sizeof(attr))) {
+		log_stderr("failure: sys_mount_setattr");
+		goto out;
+	}
+
+	file1_fd2 = openat(open_tree_fd, FILE1, O_RDWR | O_CLOEXEC, 0);
+	if (file1_fd2 < 0) {
+		log_stderr("failure: openat");
+		goto out;
+	}
+
+	/*
+	 * Verify we can set an v3 fscap for real root this was regressed at
+	 * some point. Make sure this doesn't happen again!
+	 */
+	pid = fork();
+	if (pid < 0) {
+		log_stderr("failure: fork");
+		goto out;
+	}
+	if (pid == 0) {
+		if (!switch_userns(attr.userns_fd, 0, 0, false))
+			die("failure: switch_userns");
+
+		if (expected_dummy_vfs_caps_uid(file1_fd2, -1))
+			die("failure: expected_dummy_vfs_caps_uid");
+		if (errno != ENODATA)
+			die("failure: errno");
+
+		if (set_dummy_vfs_caps(file1_fd2, 0, 0))
+			die("failure: set_dummy_vfs_caps");
+
+		if (!expected_dummy_vfs_caps_uid(file1_fd2, 0))
+			die("failure: expected_dummy_vfs_caps_uid");
+
+		if (!expected_dummy_vfs_caps_uid(file1_fd, 0) && errno != EOVERFLOW)
+			die("failure: expected_dummy_vfs_caps_uid");
+
+		exit(EXIT_SUCCESS);
+	}
+
+	if (wait_for_pid(pid))
+		goto out;
+
+	if (!expected_dummy_vfs_caps_uid(file1_fd2, 10000)) {
+		log_stderr("failure: expected_dummy_vfs_caps_uid");
+		goto out;
+	}
+
+	if (!expected_dummy_vfs_caps_uid(file1_fd, 0)) {
+		log_stderr("failure: expected_dummy_vfs_caps_uid");
+		goto out;
+	}
+
+	fret = 0;
+	log_debug("Ran test");
+out:
+	safe_close(attr.userns_fd);
+	safe_close(file1_fd);
+	safe_close(file1_fd2);
+	safe_close(open_tree_fd);
+
+	return fret;
+}
+
 static int fscaps_idmapped_mounts_in_userns_separate_userns(void)
 {
 	int fret = -1;
@@ -8717,24 +8832,26 @@ static void usage(void)
 	fprintf(stderr, "    Run idmapped mount tests\n\n");
 
 	fprintf(stderr, "Arguments:\n");
-	fprintf(stderr, "--device        Device used in the tests\n");
-	fprintf(stderr, "--fstype        Filesystem type used in the tests\n");
-	fprintf(stderr, "--help          Print help\n");
-	fprintf(stderr, "--mountpoint    Mountpoint of device\n");
-	fprintf(stderr, "--supported     Test whether idmapped mounts are supported on this filesystem\n");
-	fprintf(stderr, "--test-core     Run core idmapped mount testsuite\n");
+	fprintf(stderr, "--device                     Device used in the tests\n");
+	fprintf(stderr, "--fstype                     Filesystem type used in the tests\n");
+	fprintf(stderr, "--help                       Print help\n");
+	fprintf(stderr, "--mountpoint                 Mountpoint of device\n");
+	fprintf(stderr, "--supported                  Test whether idmapped mounts are supported on this filesystem\n");
+	fprintf(stderr, "--test-core                  Run core idmapped mount testsuite\n");
+	fprintf(stderr, "--test-fscaps-regression     Run fscap regression tests\n");
 
 	_exit(EXIT_SUCCESS);
 }
 
 static const struct option longopts[] = {
-	{"device",	required_argument,	0,	'd'},
-	{"fstype",	required_argument,	0,	'f'},
-	{"mountpoint",	required_argument,	0,	'm'},
-	{"supported",	no_argument,		0,	's'},
-	{"help",	no_argument,		0,	'h'},
-	{"test-core",	no_argument,		0,	'c'},
-	{NULL,		0,			0,	0},
+	{"device",			required_argument,	0,	'd'},
+	{"fstype",			required_argument,	0,	'f'},
+	{"mountpoint",			required_argument,	0,	'm'},
+	{"supported",			no_argument,		0,	's'},
+	{"help",			no_argument,		0,	'h'},
+	{"test-core",			no_argument,		0,	'c'},
+	{"test-fscaps-regression",	no_argument,		0,	'g'},
+	{NULL,				0,			0,	  0},
 };
 
 struct t_idmapped_mounts {
@@ -8748,7 +8865,7 @@ struct t_idmapped_mounts {
 	{ fscaps,							"fscaps on regular mounts",									},
 	{ fscaps_idmapped_mounts,					"fscaps on idmapped mounts",									},
 	{ fscaps_idmapped_mounts_in_userns,				"fscaps on idmapped mounts in user namespace",							},
-	{ fscaps_idmapped_mounts_in_userns_separate_userns,		"fscaps on idmapped mounts in user namespace with different id mappings ",			},
+	{ fscaps_idmapped_mounts_in_userns_separate_userns,		"fscaps on idmapped mounts in user namespace with different id mappings",			},
 	{ fsids_mapped,							"mapped fsids",											},
 	{ fsids_unmapped,						"unmapped fsids",										},
 	{ hardlink_crossing_mounts,					"cross mount hardlink",										},
@@ -8792,6 +8909,10 @@ struct t_idmapped_mounts {
 	{ threaded_idmapped_mount_interactions,				"threaded operations on idmapped mounts",							},
 };
 
+struct t_idmapped_mounts fscaps_in_ancestor_userns[] = {
+	{ fscaps_idmapped_mounts_in_userns_valid_in_ancestor_userns,	"fscaps on idmapped mounts in user namespace writing fscap valid in ancestor userns",		},
+};
+
 static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
 {
 	int i;
@@ -8829,9 +8950,9 @@ int main(int argc, char *argv[])
 {
 	int fret, ret;
 	int index = 0;
-	bool supported = false, test_core = false;
+	bool supported = false, test_core = false, test_fscaps_regression = false;
 
-	while ((ret = getopt_long_only(argc, argv, "d:f:m:sh", longopts, &index)) != -1) {
+	while ((ret = getopt_long_only(argc, argv, "d:f:m:g:shcg", longopts, &index)) != -1) {
 		switch (ret) {
 		case 'd':
 			t_device = optarg;
@@ -8848,6 +8969,9 @@ int main(int argc, char *argv[])
 		case 'c':
 			test_core = true;
 			break;
+		case 'g':
+			test_fscaps_regression = true;
+			break;
 		case 'h':
 			/* fallthrough */
 		default:
@@ -8920,6 +9044,11 @@ int main(int argc, char *argv[])
 	if (test_core && !run_test(basic_suite, ARRAY_SIZE(basic_suite)))
 		goto out;
 
+	if (test_fscaps_regression &&
+	    !run_test(fscaps_in_ancestor_userns,
+		      ARRAY_SIZE(fscaps_in_ancestor_userns)))
+		goto out;
+
 	fret = EXIT_SUCCESS;
 
 out:
diff --git a/tests/generic/640 b/tests/generic/640
new file mode 100755
index 00000000..a3795b2d
--- /dev/null
+++ b/tests/generic/640
@@ -0,0 +1,28 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Christian Brauner.  All Rights Reserved.
+#
+# FS QA Test 640
+#
+# Test that fscaps on idmapped mounts behave correctly.
+#
+. ./common/preamble
+_begin_fstest auto quick cap idmapped mount
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+_supported_fs generic
+_require_idmapped_mounts
+_require_test
+
+echo "Silence is golden"
+
+$here/src/idmapped-mounts/idmapped-mounts --test-fscaps-regression \
+	--device "$TEST_DEV" --mount "$TEST_DIR" --fstype "$FSTYP"
+
+status=$?
+exit
diff --git a/tests/generic/640.out b/tests/generic/640.out
new file mode 100644
index 00000000..a336a0f5
--- /dev/null
+++ b/tests/generic/640.out
@@ -0,0 +1,2 @@
+QA output created by 640
+Silence is golden
-- 
2.30.2


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

* [PATCH v3 5/8] idmapped-mounts: refactor helpers
  2021-08-12 16:01 [PATCH v3 0/8] Extend idmapped mount testsuite Christian Brauner
                   ` (3 preceding siblings ...)
  2021-08-12 16:01 ` [PATCH v3 4/8] generic/640: add fscaps regression test Christian Brauner
@ 2021-08-12 16:01 ` Christian Brauner
  2021-08-12 16:01 ` [PATCH v3 6/8] idmapped-mounts: add nested userns creation helpers Christian Brauner
  2021-08-12 16:01 ` [PATCH v3 7/8] generic/641: add nested user namespace tests Christian Brauner
  6 siblings, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2021-08-12 16:01 UTC (permalink / raw)
  To: fstests, Eryu Guan, Christoph Hellwig; +Cc: Christian Brauner

From: Christian Brauner <christian.brauner@ubuntu.com>

Make all userns creation helpers share a commond codebase and move a
bunch of code into utils.{c,h}. This simplifies a bunch of things and
makes it easier to create nested user namespaces in follow up patches.

Cc: fstests@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
unchanged

/* v3 */
unchanged
---
 src/idmapped-mounts/mount-idmapped.c | 197 -------------------------
 src/idmapped-mounts/utils.c          | 209 +++++++++++++++++++--------
 src/idmapped-mounts/utils.h          |  73 +++++++++-
 3 files changed, 223 insertions(+), 256 deletions(-)

diff --git a/src/idmapped-mounts/mount-idmapped.c b/src/idmapped-mounts/mount-idmapped.c
index 219104e7..b1209057 100644
--- a/src/idmapped-mounts/mount-idmapped.c
+++ b/src/idmapped-mounts/mount-idmapped.c
@@ -27,77 +27,6 @@
 #include "missing.h"
 #include "utils.h"
 
-/* A few helpful macros. */
-#define STRLITERALLEN(x) (sizeof(""x"") - 1)
-
-#define INTTYPE_TO_STRLEN(type)             \
-	(2 + (sizeof(type) <= 1             \
-		  ? 3                       \
-		  : sizeof(type) <= 2       \
-			? 5                 \
-			: sizeof(type) <= 4 \
-			      ? 10          \
-			      : sizeof(type) <= 8 ? 20 : sizeof(int[-2 * (sizeof(type) > 8)])))
-
-#define syserror(format, ...)                           \
-	({                                              \
-		fprintf(stderr, format, ##__VA_ARGS__); \
-		(-errno);                               \
-	})
-
-#define syserror_set(__ret__, format, ...)                    \
-	({                                                    \
-		typeof(__ret__) __internal_ret__ = (__ret__); \
-		errno = labs(__ret__);                        \
-		fprintf(stderr, format, ##__VA_ARGS__);       \
-		__internal_ret__;                             \
-	})
-
-struct list {
-	void *elem;
-	struct list *next;
-	struct list *prev;
-};
-
-#define list_for_each(__iterator, __list) \
-	for (__iterator = (__list)->next; __iterator != __list; __iterator = __iterator->next)
-
-static inline void list_init(struct list *list)
-{
-	list->elem = NULL;
-	list->next = list->prev = list;
-}
-
-static inline int list_empty(const struct list *list)
-{
-	return list == list->next;
-}
-
-static inline void __list_add(struct list *new, struct list *prev, struct list *next)
-{
-	next->prev = new;
-	new->next = next;
-	new->prev = prev;
-	prev->next = new;
-}
-
-static inline void list_add_tail(struct list *head, struct list *list)
-{
-	__list_add(list, head->prev, head);
-}
-
-typedef enum idmap_type_t {
-	ID_TYPE_UID,
-	ID_TYPE_GID
-} idmap_type_t;
-
-struct id_map {
-	idmap_type_t map_type;
-	__u32 nsid;
-	__u32 hostid;
-	__u32 range;
-};
-
 static struct list active_map;
 
 static int add_map_entry(__u32 id_host,
@@ -166,132 +95,6 @@ static int parse_map(char *map)
 	return 0;
 }
 
-static int write_id_mapping(idmap_type_t map_type, pid_t pid, const char *buf, size_t buf_size)
-{
-	int fd = -EBADF, setgroups_fd = -EBADF;
-	int fret = -1;
-	int ret;
-	char path[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) +
-		  STRLITERALLEN("/setgroups") + 1];
-
-	if (geteuid() != 0 && map_type == ID_TYPE_GID) {
-		ret = snprintf(path, sizeof(path), "/proc/%d/setgroups", pid);
-		if (ret < 0 || ret >= sizeof(path))
-			goto out;
-
-		setgroups_fd = open(path, O_WRONLY | O_CLOEXEC);
-		if (setgroups_fd < 0 && errno != ENOENT) {
-			syserror("Failed to open \"%s\"", path);
-			goto out;
-		}
-
-		if (setgroups_fd >= 0) {
-			ret = write_nointr(setgroups_fd, "deny\n", STRLITERALLEN("deny\n"));
-			if (ret != STRLITERALLEN("deny\n")) {
-				syserror("Failed to write \"deny\" to \"/proc/%d/setgroups\"", pid);
-				goto out;
-			}
-		}
-	}
-
-	ret = snprintf(path, sizeof(path), "/proc/%d/%cid_map", pid, map_type == ID_TYPE_UID ? 'u' : 'g');
-	if (ret < 0 || ret >= sizeof(path))
-		goto out;
-
-	fd = open(path, O_WRONLY | O_CLOEXEC);
-	if (fd < 0) {
-		syserror("Failed to open \"%s\"", path);
-		goto out;
-	}
-
-	ret = write_nointr(fd, buf, buf_size);
-	if (ret != buf_size) {
-		syserror("Failed to write %cid mapping to \"%s\"",
-			 map_type == ID_TYPE_UID ? 'u' : 'g', path);
-		goto out;
-	}
-
-	fret = 0;
-out:
-	if (fd >= 0)
-		close(fd);
-	if (setgroups_fd >= 0)
-		close(setgroups_fd);
-
-	return fret;
-}
-
-static int map_ids_from_idmap(struct list *idmap, pid_t pid)
-{
-	int fill, left;
-	char mapbuf[4096] = {};
-	bool had_entry = false;
-	idmap_type_t map_type, u_or_g;
-
-	for (map_type = ID_TYPE_UID, u_or_g = 'u';
-	     map_type <= ID_TYPE_GID; map_type++, u_or_g = 'g') {
-		char *pos = mapbuf;
-		int ret;
-		struct list *iterator;
-
-
-		list_for_each(iterator, idmap) {
-			struct id_map *map = iterator->elem;
-			if (map->map_type != map_type)
-				continue;
-
-			had_entry = true;
-
-			left = 4096 - (pos - mapbuf);
-			fill = snprintf(pos, left, "%u %u %u\n", map->nsid, map->hostid, map->range);
-			/*
-			 * The kernel only takes <= 4k for writes to
-			 * /proc/<pid>/{g,u}id_map
-			 */
-			if (fill <= 0 || fill >= left)
-				return syserror_set(-E2BIG, "Too many %cid mappings defined", u_or_g);
-
-			pos += fill;
-		}
-		if (!had_entry)
-			continue;
-
-		ret = write_id_mapping(map_type, pid, mapbuf, pos - mapbuf);
-		if (ret < 0)
-			return syserror("Failed to write mapping: %s", mapbuf);
-
-		memset(mapbuf, 0, sizeof(mapbuf));
-	}
-
-	return 0;
-}
-
-static int get_userns_fd_from_idmap(struct list *idmap)
-{
-	int ret;
-	pid_t pid;
-	char path_ns[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) +
-		  STRLITERALLEN("/ns/user") + 1];
-
-	pid = do_clone(get_userns_fd_cb, NULL, CLONE_NEWUSER | CLONE_NEWNS);
-	if (pid < 0)
-		return -errno;
-
-	ret = map_ids_from_idmap(idmap, pid);
-	if (ret < 0)
-		return ret;
-
-	ret = snprintf(path_ns, sizeof(path_ns), "/proc/%d/ns/user", pid);
-	if (ret < 0 || (size_t)ret >= sizeof(path_ns))
-		ret = -EIO;
-	else
-		ret = open(path_ns, O_RDONLY | O_CLOEXEC | O_NOCTTY);
-
-	(void)kill(pid, SIGKILL);
-	(void)wait_for_pid(pid);
-	return ret;
-}
-
 static inline bool strnequal(const char *str, const char *eq, size_t len)
 {
 	return strncmp(str, eq, len) == 0;
diff --git a/src/idmapped-mounts/utils.c b/src/idmapped-mounts/utils.c
index 977443f1..e54f481d 100644
--- a/src/idmapped-mounts/utils.c
+++ b/src/idmapped-mounts/utils.c
@@ -36,99 +36,192 @@ ssize_t write_nointr(int fd, const void *buf, size_t count)
 	return ret;
 }
 
-static int write_file(const char *path, const void *buf, size_t count)
+#define __STACK_SIZE (8 * 1024 * 1024)
+pid_t do_clone(int (*fn)(void *), void *arg, int flags)
 {
-	int fd;
-	ssize_t ret;
+	void *stack;
 
-	fd = open(path, O_WRONLY | O_CLOEXEC | O_NOCTTY | O_NOFOLLOW);
-	if (fd < 0)
-		return -1;
+	stack = malloc(__STACK_SIZE);
+	if (!stack)
+		return -ENOMEM;
 
-	ret = write_nointr(fd, buf, count);
-	close(fd);
-	if (ret < 0 || (size_t)ret != count)
-		return -1;
+#ifdef __ia64__
+	return __clone2(fn, stack, __STACK_SIZE, flags | SIGCHLD, arg, NULL);
+#else
+	return clone(fn, stack + __STACK_SIZE, flags | SIGCHLD, arg, NULL);
+#endif
+}
 
+static int get_userns_fd_cb(void *data)
+{
 	return 0;
 }
 
-static int map_ids(pid_t pid, unsigned long nsid, unsigned long hostid,
-		   unsigned long range)
+int wait_for_pid(pid_t pid)
 {
-	char map[100], procfile[256];
+	int status, ret;
 
-	snprintf(procfile, sizeof(procfile), "/proc/%d/uid_map", pid);
-	snprintf(map, sizeof(map), "%lu %lu %lu", nsid, hostid, range);
-	if (write_file(procfile, map, strlen(map)))
-		return -1;
+again:
+	ret = waitpid(pid, &status, 0);
+	if (ret == -1) {
+		if (errno == EINTR)
+			goto again;
 
+		return -1;
+	}
 
-	snprintf(procfile, sizeof(procfile), "/proc/%d/gid_map", pid);
-	snprintf(map, sizeof(map), "%lu %lu %lu", nsid, hostid, range);
-	if (write_file(procfile, map, strlen(map)))
+	if (!WIFEXITED(status))
 		return -1;
 
-	return 0;
+	return WEXITSTATUS(status);
 }
 
-#define __STACK_SIZE (8 * 1024 * 1024)
-pid_t do_clone(int (*fn)(void *), void *arg, int flags)
+static int write_id_mapping(idmap_type_t map_type, pid_t pid, const char *buf, size_t buf_size)
 {
-	void *stack;
+	int fd = -EBADF, setgroups_fd = -EBADF;
+	int fret = -1;
+	int ret;
+	char path[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) +
+		  STRLITERALLEN("/setgroups") + 1];
+
+	if (geteuid() != 0 && map_type == ID_TYPE_GID) {
+		ret = snprintf(path, sizeof(path), "/proc/%d/setgroups", pid);
+		if (ret < 0 || ret >= sizeof(path))
+			goto out;
+
+		setgroups_fd = open(path, O_WRONLY | O_CLOEXEC);
+		if (setgroups_fd < 0 && errno != ENOENT) {
+			syserror("Failed to open \"%s\"", path);
+			goto out;
+		}
+
+		if (setgroups_fd >= 0) {
+			ret = write_nointr(setgroups_fd, "deny\n", STRLITERALLEN("deny\n"));
+			if (ret != STRLITERALLEN("deny\n")) {
+				syserror("Failed to write \"deny\" to \"/proc/%d/setgroups\"", pid);
+				goto out;
+			}
+		}
+	}
 
-	stack = malloc(__STACK_SIZE);
-	if (!stack)
-		return -ENOMEM;
+	ret = snprintf(path, sizeof(path), "/proc/%d/%cid_map", pid, map_type == ID_TYPE_UID ? 'u' : 'g');
+	if (ret < 0 || ret >= sizeof(path))
+		goto out;
 
-#ifdef __ia64__
-	return __clone2(fn, stack, __STACK_SIZE, flags | SIGCHLD, arg, NULL);
-#else
-	return clone(fn, stack + __STACK_SIZE, flags | SIGCHLD, arg, NULL);
-#endif
+	fd = open(path, O_WRONLY | O_CLOEXEC);
+	if (fd < 0) {
+		syserror("Failed to open \"%s\"", path);
+		goto out;
+	}
+
+	ret = write_nointr(fd, buf, buf_size);
+	if (ret != buf_size) {
+		syserror("Failed to write %cid mapping to \"%s\"",
+			 map_type == ID_TYPE_UID ? 'u' : 'g', path);
+		goto out;
+	}
+
+	fret = 0;
+out:
+	if (fd >= 0)
+		close(fd);
+	if (setgroups_fd >= 0)
+		close(setgroups_fd);
+
+	return fret;
 }
 
-int get_userns_fd_cb(void *data)
+static int map_ids_from_idmap(struct list *idmap, pid_t pid)
 {
-	return kill(getpid(), SIGSTOP);
+	int fill, left;
+	char mapbuf[4096] = {};
+	bool had_entry = false;
+
+	for (idmap_type_t map_type = ID_TYPE_UID, u_or_g = 'u';
+	     map_type <= ID_TYPE_GID; map_type++, u_or_g = 'g') {
+		char *pos = mapbuf;
+		int ret;
+		struct list *iterator;
+
+
+		list_for_each(iterator, idmap) {
+			struct id_map *map = iterator->elem;
+			if (map->map_type != map_type)
+				continue;
+
+			had_entry = true;
+
+			left = 4096 - (pos - mapbuf);
+			fill = snprintf(pos, left, "%u %u %u\n", map->nsid, map->hostid, map->range);
+			/*
+			 * The kernel only takes <= 4k for writes to
+			 * /proc/<pid>/{g,u}id_map
+			 */
+			if (fill <= 0 || fill >= left)
+				return syserror_set(-E2BIG, "Too many %cid mappings defined", u_or_g);
+
+			pos += fill;
+		}
+		if (!had_entry)
+			continue;
+
+		ret = write_id_mapping(map_type, pid, mapbuf, pos - mapbuf);
+		if (ret < 0)
+			return syserror("Failed to write mapping: %s", mapbuf);
+
+		memset(mapbuf, 0, sizeof(mapbuf));
+	}
+
+	return 0;
 }
 
-int get_userns_fd(unsigned long nsid, unsigned long hostid, unsigned long range)
+int get_userns_fd_from_idmap(struct list *idmap)
 {
 	int ret;
 	pid_t pid;
-	char path[256];
+	char path_ns[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) +
+		     STRLITERALLEN("/ns/user") + 1];
 
-	pid = do_clone(get_userns_fd_cb, NULL, CLONE_NEWUSER);
+	pid = do_clone(get_userns_fd_cb, NULL, CLONE_NEWUSER | CLONE_NEWNS);
 	if (pid < 0)
 		return -errno;
 
-	ret = map_ids(pid, nsid, hostid, range);
+	ret = map_ids_from_idmap(idmap, pid);
 	if (ret < 0)
 		return ret;
 
-	snprintf(path, sizeof(path), "/proc/%d/ns/user", pid);
-	ret = open(path, O_RDONLY | O_CLOEXEC);
-	kill(pid, SIGKILL);
-	wait_for_pid(pid);
+	ret = snprintf(path_ns, sizeof(path_ns), "/proc/%d/ns/user", pid);
+	if (ret < 0 || (size_t)ret >= sizeof(path_ns))
+		ret = -EIO;
+	else
+		ret = open(path_ns, O_RDONLY | O_CLOEXEC | O_NOCTTY);
+
+	(void)kill(pid, SIGKILL);
+	(void)wait_for_pid(pid);
 	return ret;
 }
 
-int wait_for_pid(pid_t pid)
+int get_userns_fd(unsigned long nsid, unsigned long hostid, unsigned long range)
 {
-	int status, ret;
-
-again:
-	ret = waitpid(pid, &status, 0);
-	if (ret == -1) {
-		if (errno == EINTR)
-			goto again;
-
-		return -1;
-	}
-
-	if (!WIFEXITED(status))
-		return -1;
-
-	return WEXITSTATUS(status);
+	struct list head, uid_mapl, gid_mapl;
+	struct id_map uid_map = {
+		.map_type	= ID_TYPE_UID,
+		.nsid		= nsid,
+		.hostid		= hostid,
+		.range		= range,
+	};
+	struct id_map gid_map = {
+		.map_type	= ID_TYPE_GID,
+		.nsid		= nsid,
+		.hostid		= hostid,
+		.range		= range,
+	};
+
+	list_init(&head);
+	uid_mapl.elem = &uid_map;
+	gid_mapl.elem = &gid_map;
+	list_add_tail(&head, &uid_mapl);
+	list_add_tail(&head, &gid_mapl);
+
+	return get_userns_fd_from_idmap(&head);
 }
diff --git a/src/idmapped-mounts/utils.h b/src/idmapped-mounts/utils.h
index efbf3bc3..4f976f9f 100644
--- a/src/idmapped-mounts/utils.h
+++ b/src/idmapped-mounts/utils.h
@@ -19,10 +19,81 @@
 
 #include "missing.h"
 
+/* A few helpful macros. */
+#define STRLITERALLEN(x) (sizeof(""x"") - 1)
+
+#define INTTYPE_TO_STRLEN(type)             \
+	(2 + (sizeof(type) <= 1             \
+		  ? 3                       \
+		  : sizeof(type) <= 2       \
+			? 5                 \
+			: sizeof(type) <= 4 \
+			      ? 10          \
+			      : sizeof(type) <= 8 ? 20 : sizeof(int[-2 * (sizeof(type) > 8)])))
+
+#define syserror(format, ...)                           \
+	({                                              \
+		fprintf(stderr, "%m - " format "\n", ##__VA_ARGS__); \
+		(-errno);                               \
+	})
+
+#define syserror_set(__ret__, format, ...)                    \
+	({                                                    \
+		typeof(__ret__) __internal_ret__ = (__ret__); \
+		errno = labs(__ret__);                        \
+		fprintf(stderr, "%m - " format "\n", ##__VA_ARGS__);       \
+		__internal_ret__;                             \
+	})
+
+typedef enum idmap_type_t {
+	ID_TYPE_UID,
+	ID_TYPE_GID
+} idmap_type_t;
+
+struct id_map {
+	idmap_type_t map_type;
+	__u32 nsid;
+	__u32 hostid;
+	__u32 range;
+};
+
+struct list {
+	void *elem;
+	struct list *next;
+	struct list *prev;
+};
+
+#define list_for_each(__iterator, __list) \
+	for (__iterator = (__list)->next; __iterator != __list; __iterator = __iterator->next)
+
+static inline void list_init(struct list *list)
+{
+	list->elem = NULL;
+	list->next = list->prev = list;
+}
+
+static inline int list_empty(const struct list *list)
+{
+	return list == list->next;
+}
+
+static inline void __list_add(struct list *new, struct list *prev, struct list *next)
+{
+	next->prev = new;
+	new->next = next;
+	new->prev = prev;
+	prev->next = new;
+}
+
+static inline void list_add_tail(struct list *head, struct list *list)
+{
+	__list_add(list, head->prev, head);
+}
+
 extern pid_t do_clone(int (*fn)(void *), void *arg, int flags);
-extern int get_userns_fd_cb(void *data);
 extern int get_userns_fd(unsigned long nsid, unsigned long hostid,
 			 unsigned long range);
+extern int get_userns_fd_from_idmap(struct list *idmap);
 extern ssize_t read_nointr(int fd, void *buf, size_t count);
 extern int wait_for_pid(pid_t pid);
 extern ssize_t write_nointr(int fd, const void *buf, size_t count);
-- 
2.30.2


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

* [PATCH v3 6/8] idmapped-mounts: add nested userns creation helpers
  2021-08-12 16:01 [PATCH v3 0/8] Extend idmapped mount testsuite Christian Brauner
                   ` (4 preceding siblings ...)
  2021-08-12 16:01 ` [PATCH v3 5/8] idmapped-mounts: refactor helpers Christian Brauner
@ 2021-08-12 16:01 ` Christian Brauner
  2021-08-12 16:01 ` [PATCH v3 7/8] generic/641: add nested user namespace tests Christian Brauner
  6 siblings, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2021-08-12 16:01 UTC (permalink / raw)
  To: fstests, Eryu Guan, Christoph Hellwig; +Cc: Christian Brauner

From: Christian Brauner <christian.brauner@ubuntu.com>

Add a helper to create a nested userns hierarchy. This will be used in
follow-up tests.

Cc: fstests@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
unchanged

/* v3 */
unchanged
---
 src/idmapped-mounts/idmapped-mounts.c |  14 ---
 src/idmapped-mounts/mount-idmapped.c  |  32 +-----
 src/idmapped-mounts/utils.c           | 160 +++++++++++++++++++++++++-
 src/idmapped-mounts/utils.h           |  25 ++++
 4 files changed, 185 insertions(+), 46 deletions(-)

diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
index 0b2d7a52..49f360ec 100644
--- a/src/idmapped-mounts/idmapped-mounts.c
+++ b/src/idmapped-mounts/idmapped-mounts.c
@@ -388,20 +388,6 @@ static inline bool switch_fsids(uid_t fsuid, gid_t fsgid)
 	return true;
 }
 
-static inline bool switch_ids(uid_t uid, gid_t gid)
-{
-	if (setgroups(0, NULL))
-		return log_errno(false, "failure: setgroups");
-
-	if (setresgid(gid, gid, gid))
-		return log_errno(false, "failure: setresgid");
-
-	if (setresuid(uid, uid, uid))
-		return log_errno(false, "failure: setresuid");
-
-	return true;
-}
-
 static inline bool switch_userns(int fd, uid_t uid, gid_t gid, bool drop_caps)
 {
 	if (setns(fd, CLONE_NEWUSER))
diff --git a/src/idmapped-mounts/mount-idmapped.c b/src/idmapped-mounts/mount-idmapped.c
index b1209057..d8490bed 100644
--- a/src/idmapped-mounts/mount-idmapped.c
+++ b/src/idmapped-mounts/mount-idmapped.c
@@ -29,36 +29,6 @@
 
 static struct list active_map;
 
-static int add_map_entry(__u32 id_host,
-			 __u32 id_ns,
-			 __u32 range,
-			 idmap_type_t map_type)
-{
-	struct list *new_list = NULL;
-	struct id_map *newmap = NULL;
-
-	newmap = malloc(sizeof(*newmap));
-	if (!newmap)
-		return -ENOMEM;
-
-	new_list = malloc(sizeof(struct list));
-	if (!new_list) {
-		free(newmap);
-		return -ENOMEM;
-	}
-
-	*newmap = (struct id_map){
-		.hostid		= id_host,
-		.nsid		= id_ns,
-		.range		= range,
-		.map_type	= map_type,
-	};
-
-	new_list->elem = newmap;
-	list_add_tail(&active_map, new_list);
-	return 0;
-}
-
 static int parse_map(char *map)
 {
 	char types[2] = {'u', 'g'};
@@ -87,7 +57,7 @@ static int parse_map(char *map)
 		else
 			map_type = ID_TYPE_GID;
 
-		ret = add_map_entry(id_host, id_ns, range, map_type);
+		ret = add_map_entry(&active_map, id_host, id_ns, range, map_type);
 		if (ret < 0)
 			return ret;
 	}
diff --git a/src/idmapped-mounts/utils.c b/src/idmapped-mounts/utils.c
index e54f481d..6ffd6a23 100644
--- a/src/idmapped-mounts/utils.c
+++ b/src/idmapped-mounts/utils.c
@@ -3,11 +3,15 @@
 #define _GNU_SOURCE
 #endif
 #include <fcntl.h>
+#include <grp.h>
 #include <linux/limits.h>
+#include <sched.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <sched.h>
+#include <sys/eventfd.h>
 #include <sys/mount.h>
+#include <sys/prctl.h>
+#include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -137,6 +141,9 @@ static int map_ids_from_idmap(struct list *idmap, pid_t pid)
 	char mapbuf[4096] = {};
 	bool had_entry = false;
 
+	if (list_empty(idmap))
+		return 0;
+
 	for (idmap_type_t map_type = ID_TYPE_UID, u_or_g = 'u';
 	     map_type <= ID_TYPE_GID; map_type++, u_or_g = 'g') {
 		char *pos = mapbuf;
@@ -225,3 +232,154 @@ int get_userns_fd(unsigned long nsid, unsigned long hostid, unsigned long range)
 
 	return get_userns_fd_from_idmap(&head);
 }
+
+bool switch_ids(uid_t uid, gid_t gid)
+{
+	if (setgroups(0, NULL))
+		return syserror("failure: setgroups");
+
+	if (setresgid(gid, gid, gid))
+		return syserror("failure: setresgid");
+
+	if (setresuid(uid, uid, uid))
+		return syserror("failure: setresuid");
+
+	return true;
+}
+
+static int userns_fd_cb(void *data)
+{
+	struct userns_hierarchy *h = data;
+	char c;
+	int ret;
+
+	ret = read_nointr(h->fd_event, &c, 1);
+	if (ret < 0)
+		return syserror("failure: read from socketpair");
+
+	/* Only switch ids if someone actually wrote a mapping for us. */
+	if (c == '1') {
+		if (!switch_ids(0, 0))
+			return syserror("failure: switch ids to 0");
+
+		/* Ensure we can access proc files from processes we can ptrace. */
+		ret = prctl(PR_SET_DUMPABLE, 1, 0, 0, 0);
+		if (ret < 0)
+			return syserror("failure: make dumpable");
+	}
+
+	ret = write_nointr(h->fd_event, "1", 1);
+	if (ret < 0)
+		return syserror("failure: write to socketpair");
+
+	ret = create_userns_hierarchy(++h);
+	if (ret < 0)
+		return syserror("failure: userns level %d", h->level);
+
+	return 0;
+}
+
+int create_userns_hierarchy(struct userns_hierarchy *h)
+{
+	int fret = -1;
+	char c;
+	int fd_socket[2];
+	int fd_userns = -EBADF, ret = -1;
+	ssize_t bytes;
+	pid_t pid;
+	char path[256];
+
+	if (h->level == MAX_USERNS_LEVEL)
+		return 0;
+
+	ret = socketpair(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, fd_socket);
+	if (ret < 0)
+		return syserror("failure: create socketpair");
+
+	/* Note the CLONE_FILES | CLONE_VM when mucking with fds and memory. */
+	h->fd_event = fd_socket[1];
+	pid = do_clone(userns_fd_cb, h, CLONE_NEWUSER | CLONE_FILES | CLONE_VM);
+	if (pid < 0) {
+		syserror("failure: userns level %d", h->level);
+		goto out_close;
+	}
+
+	ret = map_ids_from_idmap(&h->id_map, pid);
+	if (ret < 0) {
+		kill(pid, SIGKILL);
+		syserror("failure: writing id mapping for userns level %d for %d", h->level, pid);
+		goto out_wait;
+	}
+
+	if (!list_empty(&h->id_map))
+		bytes = write_nointr(fd_socket[0], "1", 1); /* Inform the child we wrote a mapping. */
+	else
+		bytes = write_nointr(fd_socket[0], "0", 1); /* Inform the child we didn't write a mapping. */
+	if (bytes < 0) {
+		kill(pid, SIGKILL);
+		syserror("failure: write to socketpair");
+		goto out_wait;
+	}
+
+	/* Wait for child to set*id() and become dumpable. */
+	bytes = read_nointr(fd_socket[0], &c, 1);
+	if (bytes < 0) {
+		kill(pid, SIGKILL);
+		syserror("failure: read from socketpair");
+		goto out_wait;
+	}
+
+	snprintf(path, sizeof(path), "/proc/%d/ns/user", pid);
+	fd_userns = open(path, O_RDONLY | O_CLOEXEC);
+	if (fd_userns < 0) {
+		kill(pid, SIGKILL);
+		syserror("failure: open userns level %d for %d", h->level, pid);
+		goto out_wait;
+	}
+
+	fret = 0;
+
+out_wait:
+	if (!wait_for_pid(pid) && !fret) {
+		h->fd_userns = fd_userns;
+		fd_userns = -EBADF;
+	}
+
+out_close:
+	if (fd_userns >= 0)
+		close(fd_userns);
+	close(fd_socket[0]);
+	close(fd_socket[1]);
+	return fret;
+}
+
+int add_map_entry(struct list *head,
+		  __u32 id_host,
+		  __u32 id_ns,
+		  __u32 range,
+		  idmap_type_t map_type)
+{
+	struct list *new_list = NULL;
+	struct id_map *newmap = NULL;
+
+	newmap = malloc(sizeof(*newmap));
+	if (!newmap)
+		return -ENOMEM;
+
+	new_list = malloc(sizeof(struct list));
+	if (!new_list) {
+		free(newmap);
+		return -ENOMEM;
+	}
+
+	*newmap = (struct id_map){
+		.hostid		= id_host,
+		.nsid		= id_ns,
+		.range		= range,
+		.map_type	= map_type,
+	};
+
+	new_list->elem = newmap;
+	list_add_tail(head, new_list);
+	return 0;
+}
diff --git a/src/idmapped-mounts/utils.h b/src/idmapped-mounts/utils.h
index 4f976f9f..9694980e 100644
--- a/src/idmapped-mounts/utils.h
+++ b/src/idmapped-mounts/utils.h
@@ -10,6 +10,7 @@
 #include <linux/types.h>
 #include <sched.h>
 #include <signal.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -19,6 +20,9 @@
 
 #include "missing.h"
 
+/* Maximum number of nested user namespaces in the kernel. */
+#define MAX_USERNS_LEVEL 32
+
 /* A few helpful macros. */
 #define STRLITERALLEN(x) (sizeof(""x"") - 1)
 
@@ -63,6 +67,13 @@ struct list {
 	struct list *prev;
 };
 
+struct userns_hierarchy {
+	int fd_userns;
+	int fd_event;
+	unsigned int level;
+	struct list id_map;
+};
+
 #define list_for_each(__iterator, __list) \
 	for (__iterator = (__list)->next; __iterator != __list; __iterator = __iterator->next)
 
@@ -90,6 +101,16 @@ static inline void list_add_tail(struct list *head, struct list *list)
 	__list_add(list, head->prev, head);
 }
 
+static inline void list_del(struct list *list)
+{
+	struct list *next, *prev;
+
+	next = list->next;
+	prev = list->prev;
+	next->prev = prev;
+	prev->next = next;
+}
+
 extern pid_t do_clone(int (*fn)(void *), void *arg, int flags);
 extern int get_userns_fd(unsigned long nsid, unsigned long hostid,
 			 unsigned long range);
@@ -97,5 +118,9 @@ extern int get_userns_fd_from_idmap(struct list *idmap);
 extern ssize_t read_nointr(int fd, void *buf, size_t count);
 extern int wait_for_pid(pid_t pid);
 extern ssize_t write_nointr(int fd, const void *buf, size_t count);
+extern bool switch_ids(uid_t uid, gid_t gid);
+extern int create_userns_hierarchy(struct userns_hierarchy *h);
+extern int add_map_entry(struct list *head, __u32 id_host, __u32 id_ns,
+			 __u32 range, idmap_type_t map_type);
 
 #endif /* __IDMAP_UTILS_H */
-- 
2.30.2


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

* [PATCH v3 7/8] generic/641: add nested user namespace tests
  2021-08-12 16:01 [PATCH v3 0/8] Extend idmapped mount testsuite Christian Brauner
                   ` (5 preceding siblings ...)
  2021-08-12 16:01 ` [PATCH v3 6/8] idmapped-mounts: add nested userns creation helpers Christian Brauner
@ 2021-08-12 16:01 ` Christian Brauner
  6 siblings, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2021-08-12 16:01 UTC (permalink / raw)
  To: fstests, Eryu Guan, Christoph Hellwig; +Cc: Christian Brauner

From: Christian Brauner <christian.brauner@ubuntu.com>

Test ownership and ownership changes in a complex user namespace
hierarchy.

Cc: fstests@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
 - Eryu Guan <guan@eryu.me>:
   - Don't create symlinks for each test. Instead, use the same binary
     and introduce new options to specify which tests to run.

/* v3 */
unchanged
---
 src/idmapped-mounts/idmapped-mounts.c | 726 +++++++++++++++++++++++++-
 src/idmapped-mounts/utils.h           |   4 +
 tests/generic/641                     |  28 +
 tests/generic/641.out                 |   2 +
 4 files changed, 758 insertions(+), 2 deletions(-)
 create mode 100755 tests/generic/641
 create mode 100644 tests/generic/641.out

diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
index 49f360ec..1d569b89 100644
--- a/src/idmapped-mounts/idmapped-mounts.c
+++ b/src/idmapped-mounts/idmapped-mounts.c
@@ -8812,6 +8812,714 @@ out:
 	return fret;
 }
 
+static int nested_userns(void)
+{
+	int fret = -1;
+	int ret;
+	pid_t pid;
+	struct list *it, *next;
+	struct userns_hierarchy hierarchy[] = {
+		{ .level = 1, .fd_userns = -EBADF, },
+		{ .level = 2, .fd_userns = -EBADF, },
+		{ .level = 3, .fd_userns = -EBADF, },
+		{ .level = 4, .fd_userns = -EBADF, },
+		/* Dummy entry that marks the end. */
+		{ .level = MAX_USERNS_LEVEL, .fd_userns = -EBADF, },
+	};
+	struct mount_attr attr_level1 = {
+		.attr_set	= MOUNT_ATTR_IDMAP,
+		.userns_fd	= -EBADF,
+	};
+	struct mount_attr attr_level2 = {
+		.attr_set	= MOUNT_ATTR_IDMAP,
+		.userns_fd	= -EBADF,
+	};
+	struct mount_attr attr_level3 = {
+		.attr_set	= MOUNT_ATTR_IDMAP,
+		.userns_fd	= -EBADF,
+	};
+	struct mount_attr attr_level4 = {
+		.attr_set	= MOUNT_ATTR_IDMAP,
+		.userns_fd	= -EBADF,
+	};
+	int fd_dir1 = -EBADF,
+	    fd_open_tree_level1 = -EBADF,
+	    fd_open_tree_level2 = -EBADF,
+	    fd_open_tree_level3 = -EBADF,
+	    fd_open_tree_level4 = -EBADF;
+	const unsigned int id_file_range = 10000;
+
+	list_init(&hierarchy[0].id_map);
+	list_init(&hierarchy[1].id_map);
+	list_init(&hierarchy[2].id_map);
+	list_init(&hierarchy[3].id_map);
+
+	/*
+	 * Give a large map to the outermost user namespace so we can create
+	 * comfortable nested maps.
+	 */
+	ret = add_map_entry(&hierarchy[0].id_map, 1000000, 0, 1000000000, ID_TYPE_UID);
+	if (ret) {
+		log_stderr("failure: adding uidmap for userns at level 1");
+		goto out;
+	}
+
+	ret = add_map_entry(&hierarchy[0].id_map, 1000000, 0, 1000000000, ID_TYPE_GID);
+	if (ret) {
+		log_stderr("failure: adding gidmap for userns at level 1");
+		goto out;
+	}
+
+	/* This is uid:0->2000000:100000000 in init userns. */
+	ret = add_map_entry(&hierarchy[1].id_map, 1000000, 0, 100000000, ID_TYPE_UID);
+	if (ret) {
+		log_stderr("failure: adding uidmap for userns at level 2");
+		goto out;
+	}
+
+	/* This is gid:0->2000000:100000000 in init userns. */
+	ret = add_map_entry(&hierarchy[1].id_map, 1000000, 0, 100000000, ID_TYPE_GID);
+	if (ret) {
+		log_stderr("failure: adding gidmap for userns at level 2");
+		goto out;
+	}
+
+	/* This is uid:0->3000000:999 in init userns. */
+	ret = add_map_entry(&hierarchy[2].id_map, 1000000, 0, 999, ID_TYPE_UID);
+	if (ret) {
+		log_stderr("failure: adding uidmap for userns at level 3");
+		goto out;
+	}
+
+	/* This is gid:0->3000000:999 in the init userns. */
+	ret = add_map_entry(&hierarchy[2].id_map, 1000000, 0, 999, ID_TYPE_GID);
+	if (ret) {
+		log_stderr("failure: adding gidmap for userns at level 3");
+		goto out;
+	}
+
+	/* id 999 will remain unmapped. */
+
+	/* This is uid:1000->2001000:1 in init userns. */
+	ret = add_map_entry(&hierarchy[2].id_map, 1000, 1000, 1, ID_TYPE_UID);
+	if (ret) {
+		log_stderr("failure: adding uidmap for userns at level 3");
+		goto out;
+	}
+
+	/* This is gid:1000->2001000:1 in init userns. */
+	ret = add_map_entry(&hierarchy[2].id_map, 1000, 1000, 1, ID_TYPE_GID);
+	if (ret) {
+		log_stderr("failure: adding gidmap for userns at level 3");
+		goto out;
+	}
+
+	/* This is uid:1001->3001001:10000 in init userns. */
+	ret = add_map_entry(&hierarchy[2].id_map, 1001001, 1001, 10000000, ID_TYPE_UID);
+	if (ret) {
+		log_stderr("failure: adding uidmap for userns at level 3");
+		goto out;
+	}
+
+	/* This is gid:1001->3001001:10000 in init userns. */
+	ret = add_map_entry(&hierarchy[2].id_map, 1001001, 1001, 10000000, ID_TYPE_GID);
+	if (ret) {
+		log_stderr("failure: adding gidmap for userns at level 3");
+		goto out;
+	}
+
+	/* Don't write a mapping in the 4th userns. */
+	list_empty(&hierarchy[4].id_map);
+
+	/* Create the actual userns hierarchy. */
+	ret = create_userns_hierarchy(hierarchy);
+	if (ret) {
+		log_stderr("failure: create userns hierarchy");
+		goto out;
+	}
+
+	attr_level1.userns_fd = hierarchy[0].fd_userns;
+	attr_level2.userns_fd = hierarchy[1].fd_userns;
+	attr_level3.userns_fd = hierarchy[2].fd_userns;
+	attr_level4.userns_fd = hierarchy[3].fd_userns;
+
+	/*
+	 * Create one directory where we create files for each uid/gid within
+	 * the first userns.
+	 */
+	if (mkdirat(t_dir1_fd, DIR1, 0777)) {
+		log_stderr("failure: mkdirat");
+		goto out;
+	}
+
+	fd_dir1 = openat(t_dir1_fd, DIR1, O_DIRECTORY | O_CLOEXEC);
+	if (fd_dir1 < 0) {
+		log_stderr("failure: openat");
+		goto out;
+	}
+
+	for (unsigned int id = 0; id <= id_file_range; id++) {
+		char file[256];
+
+		snprintf(file, sizeof(file), DIR1 "/" FILE1 "_%u", id);
+
+		if (mknodat(t_dir1_fd, file, S_IFREG | 0644, 0)) {
+			log_stderr("failure: create %s", file);
+			goto out;
+		}
+
+		if (fchownat(t_dir1_fd, file, id, id, AT_SYMLINK_NOFOLLOW)) {
+			log_stderr("failure: fchownat %s", file);
+			goto out;
+		}
+
+		if (!expected_uid_gid(t_dir1_fd, file, 0, id, id)) {
+			log_stderr("failure: check ownership %s", file);
+			goto out;
+		}
+	}
+
+	/* Create detached mounts for all the user namespaces. */
+	fd_open_tree_level1 = sys_open_tree(t_dir1_fd, DIR1,
+					    AT_NO_AUTOMOUNT |
+					    AT_SYMLINK_NOFOLLOW |
+					    OPEN_TREE_CLOEXEC |
+					    OPEN_TREE_CLONE);
+	if (fd_open_tree_level1 < 0) {
+		log_stderr("failure: sys_open_tree");
+		goto out;
+	}
+
+	fd_open_tree_level2 = sys_open_tree(t_dir1_fd, DIR1,
+					    AT_NO_AUTOMOUNT |
+					    AT_SYMLINK_NOFOLLOW |
+					    OPEN_TREE_CLOEXEC |
+					    OPEN_TREE_CLONE);
+	if (fd_open_tree_level2 < 0) {
+		log_stderr("failure: sys_open_tree");
+		goto out;
+	}
+
+	fd_open_tree_level3 = sys_open_tree(t_dir1_fd, DIR1,
+					    AT_NO_AUTOMOUNT |
+					    AT_SYMLINK_NOFOLLOW |
+					    OPEN_TREE_CLOEXEC |
+					    OPEN_TREE_CLONE);
+	if (fd_open_tree_level3 < 0) {
+		log_stderr("failure: sys_open_tree");
+		goto out;
+	}
+
+	fd_open_tree_level4 = sys_open_tree(t_dir1_fd, DIR1,
+					    AT_NO_AUTOMOUNT |
+					    AT_SYMLINK_NOFOLLOW |
+					    OPEN_TREE_CLOEXEC |
+					    OPEN_TREE_CLONE);
+	if (fd_open_tree_level4 < 0) {
+		log_stderr("failure: sys_open_tree");
+		goto out;
+	}
+
+	/* Turn detached mounts into detached idmapped mounts. */
+	if (sys_mount_setattr(fd_open_tree_level1, "", AT_EMPTY_PATH,
+			      &attr_level1, sizeof(attr_level1))) {
+		log_stderr("failure: sys_mount_setattr");
+		goto out;
+	}
+
+	if (sys_mount_setattr(fd_open_tree_level2, "", AT_EMPTY_PATH,
+			      &attr_level2, sizeof(attr_level2))) {
+		log_stderr("failure: sys_mount_setattr");
+		goto out;
+	}
+
+	if (sys_mount_setattr(fd_open_tree_level3, "", AT_EMPTY_PATH,
+			      &attr_level3, sizeof(attr_level3))) {
+		log_stderr("failure: sys_mount_setattr");
+		goto out;
+	}
+
+	if (sys_mount_setattr(fd_open_tree_level4, "", AT_EMPTY_PATH,
+			      &attr_level4, sizeof(attr_level4))) {
+		log_stderr("failure: sys_mount_setattr");
+		goto out;
+	}
+
+	/* Verify that ownership looks correct for callers in the init userns. */
+	for (unsigned int id = 0; id <= id_file_range; id++) {
+		bool bret;
+		unsigned int id_level1, id_level2, id_level3;
+		char file[256];
+
+		snprintf(file, sizeof(file), FILE1 "_%u", id);
+
+		id_level1 = id + 1000000;
+		if (!expected_uid_gid(fd_open_tree_level1, file, 0, id_level1, id_level1)) {
+			log_stderr("failure: check ownership %s", file);
+			goto out;
+		}
+
+		id_level2 = id + 2000000;
+		if (!expected_uid_gid(fd_open_tree_level2, file, 0, id_level2, id_level2)) {
+			log_stderr("failure: check ownership %s", file);
+			goto out;
+		}
+
+		if (id == 999) {
+			/* This id is unmapped. */
+			bret = expected_uid_gid(fd_open_tree_level3, file, 0, t_overflowuid, t_overflowgid);
+		} else if (id == 1000) {
+			id_level3 = id + 2000000; /* We punched a hole in the map at 1000. */
+			bret = expected_uid_gid(fd_open_tree_level3, file, 0, id_level3, id_level3);
+		} else {
+			id_level3 = id + 3000000; /* Rest is business as usual. */
+			bret = expected_uid_gid(fd_open_tree_level3, file, 0, id_level3, id_level3);
+		}
+		if (!bret) {
+			log_stderr("failure: check ownership %s", file);
+			goto out;
+		}
+
+		if (!expected_uid_gid(fd_open_tree_level4, file, 0, t_overflowuid, t_overflowgid)) {
+			log_stderr("failure: check ownership %s", file);
+			goto out;
+		}
+	}
+
+	/* Verify that ownership looks correct for callers in the first userns. */
+	pid = fork();
+	if (pid < 0) {
+		log_stderr("failure: fork");
+		goto out;
+	}
+	if (pid == 0) {
+		if (!switch_userns(attr_level1.userns_fd, 0, 0, false))
+			die("failure: switch_userns");
+
+		for (unsigned int id = 0; id <= id_file_range; id++) {
+			bool bret;
+			unsigned int id_level1, id_level2, id_level3;
+			char file[256];
+
+			snprintf(file, sizeof(file), FILE1 "_%u", id);
+
+			id_level1 = id;
+			if (!expected_uid_gid(fd_open_tree_level1, file, 0, id_level1, id_level1))
+				die("failure: check ownership %s", file);
+
+			id_level2 = id + 1000000;
+			if (!expected_uid_gid(fd_open_tree_level2, file, 0, id_level2, id_level2))
+				die("failure: check ownership %s", file);
+
+			if (id == 999) {
+				/* This id is unmapped. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, t_overflowuid, t_overflowgid);
+			} else if (id == 1000) {
+				id_level3 = id + 1000000; /* We punched a hole in the map at 1000. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, id_level3, id_level3);
+			} else {
+				id_level3 = id + 2000000; /* Rest is business as usual. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, id_level3, id_level3);
+			}
+			if (!bret)
+				die("failure: check ownership %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level4, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+		}
+
+		exit(EXIT_SUCCESS);
+	}
+	if (wait_for_pid(pid))
+		goto out;
+
+	/* Verify that ownership looks correct for callers in the second userns. */
+	pid = fork();
+	if (pid < 0) {
+		log_stderr("failure: fork");
+		goto out;
+	}
+	if (pid == 0) {
+		if (!switch_userns(attr_level2.userns_fd, 0, 0, false))
+			die("failure: switch_userns");
+
+		for (unsigned int id = 0; id <= id_file_range; id++) {
+			bool bret;
+			unsigned int id_level2, id_level3;
+			char file[256];
+
+			snprintf(file, sizeof(file), FILE1 "_%u", id);
+
+			if (!expected_uid_gid(fd_open_tree_level1, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			id_level2 = id;
+			if (!expected_uid_gid(fd_open_tree_level2, file, 0, id_level2, id_level2))
+				die("failure: check ownership %s", file);
+
+			if (id == 999) {
+				/* This id is unmapped. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, t_overflowuid, t_overflowgid);
+			} else if (id == 1000) {
+				id_level3 = id; /* We punched a hole in the map at 1000. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, id_level3, id_level3);
+			} else {
+				id_level3 = id + 1000000; /* Rest is business as usual. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, id_level3, id_level3);
+			}
+			if (!bret)
+				die("failure: check ownership %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level4, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+		}
+
+		exit(EXIT_SUCCESS);
+	}
+	if (wait_for_pid(pid))
+		goto out;
+
+	/* Verify that ownership looks correct for callers in the third userns. */
+	pid = fork();
+	if (pid < 0) {
+		log_stderr("failure: fork");
+		goto out;
+	}
+	if (pid == 0) {
+		if (!switch_userns(attr_level3.userns_fd, 0, 0, false))
+			die("failure: switch_userns");
+
+		for (unsigned int id = 0; id <= id_file_range; id++) {
+			bool bret;
+			unsigned int id_level2, id_level3;
+			char file[256];
+
+			snprintf(file, sizeof(file), FILE1 "_%u", id);
+
+			if (!expected_uid_gid(fd_open_tree_level1, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			if (id == 1000) {
+				/*
+				 * The idmapping of the third userns has a hole
+				 * at uid/gid 1000. That means:
+				 * - 1000->userns_0(2000000) // init userns
+				 * - 1000->userns_1(2000000) // level 1
+				 * - 1000->userns_2(1000000) // level 2
+				 * - 1000->userns_3(1000)    // level 3 (because level 3 has a hole)
+				 */
+				id_level2 = id;
+				bret = expected_uid_gid(fd_open_tree_level2, file, 0, id_level2, id_level2);
+			} else {
+				bret = expected_uid_gid(fd_open_tree_level2, file, 0, t_overflowuid, t_overflowgid);
+			}
+			if (!bret)
+				die("failure: check ownership %s", file);
+
+
+			if (id == 999) {
+				/* This id is unmapped. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, t_overflowuid, t_overflowgid);
+			} else {
+				id_level3 = id; /* Rest is business as usual. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, id_level3, id_level3);
+			}
+			if (!bret)
+				die("failure: check ownership %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level4, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+		}
+
+		exit(EXIT_SUCCESS);
+	}
+	if (wait_for_pid(pid))
+		goto out;
+
+	/* Verify that ownership looks correct for callers in the fourth userns. */
+	pid = fork();
+	if (pid < 0) {
+		log_stderr("failure: fork");
+		goto out;
+	}
+	if (pid == 0) {
+		if (setns(attr_level4.userns_fd, CLONE_NEWUSER))
+			die("failure: switch_userns");
+
+		for (unsigned int id = 0; id <= id_file_range; id++) {
+			char file[256];
+
+			snprintf(file, sizeof(file), FILE1 "_%u", id);
+
+			if (!expected_uid_gid(fd_open_tree_level1, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level2, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level3, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level4, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+		}
+
+		exit(EXIT_SUCCESS);
+	}
+	if (wait_for_pid(pid))
+		goto out;
+
+	/* Verify that chown works correctly for callers in the first userns. */
+	pid = fork();
+	if (pid < 0) {
+		log_stderr("failure: fork");
+		goto out;
+	}
+	if (pid == 0) {
+		if (!switch_userns(attr_level1.userns_fd, 0, 0, false))
+			die("failure: switch_userns");
+
+		for (unsigned int id = 0; id <= id_file_range; id++) {
+			bool bret;
+			unsigned int id_level1, id_level2, id_level3, id_new;
+			char file[256];
+
+			snprintf(file, sizeof(file), FILE1 "_%u", id);
+
+			id_new = id + 1;
+			if (fchownat(fd_open_tree_level1, file, id_new, id_new, AT_SYMLINK_NOFOLLOW))
+				die("failure: fchownat %s", file);
+
+			id_level1 = id_new;
+			if (!expected_uid_gid(fd_open_tree_level1, file, 0, id_level1, id_level1))
+				die("failure: check ownership %s", file);
+
+			id_level2 = id_new + 1000000;
+			if (!expected_uid_gid(fd_open_tree_level2, file, 0, id_level2, id_level2))
+				die("failure: check ownership %s", file);
+
+			if (id_new == 999) {
+				/* This id is unmapped. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, t_overflowuid, t_overflowgid);
+			} else if (id_new == 1000) {
+				id_level3 = id_new + 1000000; /* We punched a hole in the map at 1000. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, id_level3, id_level3);
+			} else {
+				id_level3 = id_new + 2000000; /* Rest is business as usual. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, id_level3, id_level3);
+			}
+			if (!bret)
+				die("failure: check ownership %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level4, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			/* Revert ownership. */
+			if (fchownat(fd_open_tree_level1, file, id, id, AT_SYMLINK_NOFOLLOW))
+				die("failure: fchownat %s", file);
+		}
+
+		exit(EXIT_SUCCESS);
+	}
+	if (wait_for_pid(pid))
+		goto out;
+
+	/* Verify that chown works correctly for callers in the second userns. */
+	pid = fork();
+	if (pid < 0) {
+		log_stderr("failure: fork");
+		goto out;
+	}
+	if (pid == 0) {
+		if (!switch_userns(attr_level2.userns_fd, 0, 0, false))
+			die("failure: switch_userns");
+
+		for (unsigned int id = 0; id <= id_file_range; id++) {
+			bool bret;
+			unsigned int id_level2, id_level3, id_new;
+			char file[256];
+
+			snprintf(file, sizeof(file), FILE1 "_%u", id);
+
+			id_new = id + 1;
+			if (fchownat(fd_open_tree_level2, file, id_new, id_new, AT_SYMLINK_NOFOLLOW))
+				die("failure: fchownat %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level1, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			id_level2 = id_new;
+			if (!expected_uid_gid(fd_open_tree_level2, file, 0, id_level2, id_level2))
+				die("failure: check ownership %s", file);
+
+			if (id_new == 999) {
+				/* This id is unmapped. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, t_overflowuid, t_overflowgid);
+			} else if (id_new == 1000) {
+				id_level3 = id_new; /* We punched a hole in the map at 1000. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, id_level3, id_level3);
+			} else {
+				id_level3 = id_new + 1000000; /* Rest is business as usual. */
+				bret = expected_uid_gid(fd_open_tree_level3, file, 0, id_level3, id_level3);
+			}
+			if (!bret)
+				die("failure: check ownership %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level4, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			/* Revert ownership. */
+			if (fchownat(fd_open_tree_level2, file, id, id, AT_SYMLINK_NOFOLLOW))
+				die("failure: fchownat %s", file);
+		}
+
+		exit(EXIT_SUCCESS);
+	}
+	if (wait_for_pid(pid))
+		goto out;
+
+	/* Verify that chown works correctly for callers in the third userns. */
+	pid = fork();
+	if (pid < 0) {
+		log_stderr("failure: fork");
+		goto out;
+	}
+	if (pid == 0) {
+		if (!switch_userns(attr_level3.userns_fd, 0, 0, false))
+			die("failure: switch_userns");
+
+		for (unsigned int id = 0; id <= id_file_range; id++) {
+			unsigned int id_new;
+			char file[256];
+
+			snprintf(file, sizeof(file), FILE1 "_%u", id);
+
+			id_new = id + 1;
+			if (id_new == 999 || id_new == 1000) {
+				/*
+				 * We can't change ownership as we can't
+				 * chown from or to an unmapped id.
+				 */
+				if (!fchownat(fd_open_tree_level3, file, id_new, id_new, AT_SYMLINK_NOFOLLOW))
+					die("failure: fchownat %s", file);
+			} else {
+				if (fchownat(fd_open_tree_level3, file, id_new, id_new, AT_SYMLINK_NOFOLLOW))
+					die("failure: fchownat %s", file);
+			}
+
+			if (!expected_uid_gid(fd_open_tree_level1, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			/* There's no id 1000 anymore as we changed ownership for id 1000 to 1001 above. */
+			if (!expected_uid_gid(fd_open_tree_level2, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			if (id_new == 999) {
+				/*
+				 * We did not change ownership as we can't
+				 * chown to an unmapped id.
+				 */
+				if (!expected_uid_gid(fd_open_tree_level3, file, 0, id, id))
+					die("failure: check ownership %s", file);
+			} else if (id_new == 1000) {
+				/*
+				 * We did not change ownership as we can't
+				 * chown from an unmapped id.
+				 */
+				if (!expected_uid_gid(fd_open_tree_level3, file, 0, t_overflowuid, t_overflowgid))
+					die("failure: check ownership %s", file);
+			} else {
+				if (!expected_uid_gid(fd_open_tree_level3, file, 0, id_new, id_new))
+					die("failure: check ownership %s", file);
+			}
+
+			if (!expected_uid_gid(fd_open_tree_level4, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			/* Revert ownership. */
+			if (id_new != 999 && id_new != 1000) {
+				if (fchownat(fd_open_tree_level3, file, id, id, AT_SYMLINK_NOFOLLOW))
+					die("failure: fchownat %s", file);
+			}
+		}
+
+		exit(EXIT_SUCCESS);
+	}
+	if (wait_for_pid(pid))
+		goto out;
+
+	/* Verify that chown works correctly for callers in the fourth userns. */
+	pid = fork();
+	if (pid < 0) {
+		log_stderr("failure: fork");
+		goto out;
+	}
+	if (pid == 0) {
+		if (setns(attr_level4.userns_fd, CLONE_NEWUSER))
+			die("failure: switch_userns");
+
+		for (unsigned int id = 0; id <= id_file_range; id++) {
+			char file[256];
+			unsigned long id_new;
+
+			snprintf(file, sizeof(file), FILE1 "_%u", id);
+
+			id_new = id + 1;
+			if (!fchownat(fd_open_tree_level4, file, id_new, id_new, AT_SYMLINK_NOFOLLOW))
+				die("failure: fchownat %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level1, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level2, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level3, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+			if (!expected_uid_gid(fd_open_tree_level4, file, 0, t_overflowuid, t_overflowgid))
+				die("failure: check ownership %s", file);
+
+		}
+
+		exit(EXIT_SUCCESS);
+	}
+	if (wait_for_pid(pid))
+		goto out;
+
+	fret = 0;
+	log_debug("Ran test");
+
+out:
+	list_for_each_safe(it, &hierarchy[0].id_map, next) {
+		list_del(it);
+		free(it->elem);
+		free(it);
+	}
+
+	list_for_each_safe(it, &hierarchy[1].id_map, next) {
+		list_del(it);
+		free(it->elem);
+		free(it);
+	}
+
+	list_for_each_safe(it, &hierarchy[2].id_map, next) {
+		list_del(it);
+		free(it->elem);
+		free(it);
+	}
+
+	safe_close(hierarchy[0].fd_userns);
+	safe_close(hierarchy[1].fd_userns);
+	safe_close(hierarchy[2].fd_userns);
+	safe_close(fd_dir1);
+	safe_close(fd_open_tree_level1);
+	safe_close(fd_open_tree_level2);
+	safe_close(fd_open_tree_level3);
+	safe_close(fd_open_tree_level4);
+	return fret;
+}
+
 static void usage(void)
 {
 	fprintf(stderr, "Description:\n");
@@ -8825,6 +9533,7 @@ static void usage(void)
 	fprintf(stderr, "--supported                  Test whether idmapped mounts are supported on this filesystem\n");
 	fprintf(stderr, "--test-core                  Run core idmapped mount testsuite\n");
 	fprintf(stderr, "--test-fscaps-regression     Run fscap regression tests\n");
+	fprintf(stderr, "--test-nested-userns         Run nested userns idmapped mount testsuite\n");
 
 	_exit(EXIT_SUCCESS);
 }
@@ -8837,6 +9546,7 @@ static const struct option longopts[] = {
 	{"help",			no_argument,		0,	'h'},
 	{"test-core",			no_argument,		0,	'c'},
 	{"test-fscaps-regression",	no_argument,		0,	'g'},
+	{"test-nested-userns",		no_argument,		0,	'n'},
 	{NULL,				0,			0,	  0},
 };
 
@@ -8899,6 +9609,10 @@ struct t_idmapped_mounts fscaps_in_ancestor_userns[] = {
 	{ fscaps_idmapped_mounts_in_userns_valid_in_ancestor_userns,	"fscaps on idmapped mounts in user namespace writing fscap valid in ancestor userns",		},
 };
 
+struct t_idmapped_mounts t_nested_userns[] = {
+	{ nested_userns,						"test that nested user namespaces behave correctly when attached to idmapped mounts",		},
+};
+
 static bool run_test(struct t_idmapped_mounts suite[], size_t suite_size)
 {
 	int i;
@@ -8936,9 +9650,10 @@ int main(int argc, char *argv[])
 {
 	int fret, ret;
 	int index = 0;
-	bool supported = false, test_core = false, test_fscaps_regression = false;
+	bool supported = false, test_core = false,
+	     test_fscaps_regression = false, test_nested_userns = false;
 
-	while ((ret = getopt_long_only(argc, argv, "d:f:m:g:shcg", longopts, &index)) != -1) {
+	while ((ret = getopt_long_only(argc, argv, "d:f:m:g:shcgn", longopts, &index)) != -1) {
 		switch (ret) {
 		case 'd':
 			t_device = optarg;
@@ -8958,6 +9673,9 @@ int main(int argc, char *argv[])
 		case 'g':
 			test_fscaps_regression = true;
 			break;
+		case 'n':
+			test_nested_userns = true;
+			break;
 		case 'h':
 			/* fallthrough */
 		default:
@@ -9035,6 +9753,10 @@ int main(int argc, char *argv[])
 		      ARRAY_SIZE(fscaps_in_ancestor_userns)))
 		goto out;
 
+	if (test_nested_userns &&
+	    !run_test(t_nested_userns, ARRAY_SIZE(t_nested_userns)))
+		goto out;
+
 	fret = EXIT_SUCCESS;
 
 out:
diff --git a/src/idmapped-mounts/utils.h b/src/idmapped-mounts/utils.h
index 9694980e..afb3c228 100644
--- a/src/idmapped-mounts/utils.h
+++ b/src/idmapped-mounts/utils.h
@@ -77,6 +77,10 @@ struct userns_hierarchy {
 #define list_for_each(__iterator, __list) \
 	for (__iterator = (__list)->next; __iterator != __list; __iterator = __iterator->next)
 
+#define list_for_each_safe(__iterator, __list, __next)               \
+	for (__iterator = (__list)->next, __next = __iterator->next; \
+	     __iterator != __list; __iterator = __next, __next = __next->next)
+
 static inline void list_init(struct list *list)
 {
 	list->elem = NULL;
diff --git a/tests/generic/641 b/tests/generic/641
new file mode 100755
index 00000000..a3a41f02
--- /dev/null
+++ b/tests/generic/641
@@ -0,0 +1,28 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Christian Brauner.  All Rights Reserved.
+#
+# FS QA Test 641
+#
+# Test that idmapped mounts behave correctly with complex user namespaces.
+#
+. ./common/preamble
+_begin_fstest auto quick idmapped mount
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+_supported_fs generic
+_require_idmapped_mounts
+_require_test
+
+echo "Silence is golden"
+
+$here/src/idmapped-mounts/idmapped-mounts --test-nested-userns \
+	--device "$TEST_DEV" --mount "$TEST_DIR" --fstype "$FSTYP"
+
+status=$?
+exit
diff --git a/tests/generic/641.out b/tests/generic/641.out
new file mode 100644
index 00000000..216333fd
--- /dev/null
+++ b/tests/generic/641.out
@@ -0,0 +1,2 @@
+QA output created by 641
+Silence is golden
-- 
2.30.2


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

* Re: [PATCH v3 1/8] idmapped-mounts: use die() helper
  2021-08-12 16:01 ` [PATCH v3 1/8] idmapped-mounts: use die() helper Christian Brauner
@ 2021-08-14  8:39   ` Christoph Hellwig
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2021-08-14  8:39 UTC (permalink / raw)
  To: Christian Brauner
  Cc: fstests, Eryu Guan, Christoph Hellwig, Christian Brauner

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH v3 2/8] idmapped-mounts: switch to getopt_long_only()
  2021-08-12 16:01 ` [PATCH v3 2/8] idmapped-mounts: switch to getopt_long_only() Christian Brauner
@ 2021-08-14  8:42   ` Christoph Hellwig
  2021-08-14 10:13     ` Christian Brauner
  0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2021-08-14  8:42 UTC (permalink / raw)
  To: Christian Brauner
  Cc: fstests, Eryu Guan, Christoph Hellwig, Christian Brauner

> -	while ((ret = getopt_long(argc, argv, "", longopts, &index)) != -1) {
> +	while ((ret = getopt_long_only(argc, argv, "d:f:m:sh", longopts, &index)) != -1) {

Hmm.  From the manpage:

"(If the program accepts only long options, then optstring should be specified
as an  empty  string (""),  not  NULL.)"

So I think the empty opstring should remain.

Also later in the man page:

" getopt_long_only() is like getopt_long(), but '-' as well as "--" can
indicate a long option."

So maybe my advise to switch to getopt_long_only wasn't needed or is
even not that helpful.  Sorry, it's been a while since I wrote programs
with non-trivial option parsing.

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

* Re: [PATCH v3 3/8] idmapped-mounts: introduce an explicit command line switch for testsuite
  2021-08-12 16:01 ` [PATCH v3 3/8] idmapped-mounts: introduce an explicit command line switch for testsuite Christian Brauner
@ 2021-08-14  8:42   ` Christoph Hellwig
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2021-08-14  8:42 UTC (permalink / raw)
  To: Christian Brauner
  Cc: fstests, Eryu Guan, Christoph Hellwig, Christian Brauner

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH v3 2/8] idmapped-mounts: switch to getopt_long_only()
  2021-08-14  8:42   ` Christoph Hellwig
@ 2021-08-14 10:13     ` Christian Brauner
  0 siblings, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2021-08-14 10:13 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Christian Brauner, fstests, Eryu Guan

On Sat, Aug 14, 2021 at 10:42:34AM +0200, Christoph Hellwig wrote:
> > -	while ((ret = getopt_long(argc, argv, "", longopts, &index)) != -1) {
> > +	while ((ret = getopt_long_only(argc, argv, "d:f:m:sh", longopts, &index)) != -1) {
> 
> Hmm.  From the manpage:
> 
> "(If the program accepts only long options, then optstring should be specified
> as an  empty  string (""),  not  NULL.)"
> 
> So I think the empty opstring should remain.
> 
> Also later in the man page:
> 
> " getopt_long_only() is like getopt_long(), but '-' as well as "--" can
> indicate a long option."
> 
> So maybe my advise to switch to getopt_long_only wasn't needed or is
> even not that helpful.  Sorry, it's been a while since I wrote programs
> with non-trivial option parsing.

Nah, that's perfectly fine. I think the getopt_long_only() switch was
good. We don't anywhere use and shouldn't encourage using shortopts.
It's much more descriptive to see

$here/src/idmapped-mounts/idmapped-mounts \
	--test-btrfs \
	--device "$TEST_DEV" \
	--mountpoint "$TEST_DIR" \
	--scratch-device "$SCRATCH_DEV" \
	--scratch-mountpoint "$SCRATCH_MNT"
	--fstype "$FSTYP"

than it is to see e.g.:

$here/src/idmapped-mounts/idmapped-mounts \
	-b 
	-d "$TEST_DEV" \
	-m "$TEST_DIR" \
	-s "$SCRATCH_DEV" \
	-a "$SCRATCH_MNT" \
	-f "$FSTYP"

In the second case I have to go check the source code to make sure that
this is the correct option. In the first case I can just read it in the
test itself. So I think we'll keep it but I'll remove the option string
like you pointed out.

Christian

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

end of thread, other threads:[~2021-08-14 10:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-12 16:01 [PATCH v3 0/8] Extend idmapped mount testsuite Christian Brauner
2021-08-12 16:01 ` [PATCH v3 1/8] idmapped-mounts: use die() helper Christian Brauner
2021-08-14  8:39   ` Christoph Hellwig
2021-08-12 16:01 ` [PATCH v3 2/8] idmapped-mounts: switch to getopt_long_only() Christian Brauner
2021-08-14  8:42   ` Christoph Hellwig
2021-08-14 10:13     ` Christian Brauner
2021-08-12 16:01 ` [PATCH v3 3/8] idmapped-mounts: introduce an explicit command line switch for testsuite Christian Brauner
2021-08-14  8:42   ` Christoph Hellwig
2021-08-12 16:01 ` [PATCH v3 4/8] generic/640: add fscaps regression test Christian Brauner
2021-08-12 16:01 ` [PATCH v3 5/8] idmapped-mounts: refactor helpers Christian Brauner
2021-08-12 16:01 ` [PATCH v3 6/8] idmapped-mounts: add nested userns creation helpers Christian Brauner
2021-08-12 16:01 ` [PATCH v3 7/8] generic/641: add nested user namespace tests Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).