From 16fc8f1c4a745d35a2957cb0a17ae9fab3ef2a7f Mon Sep 17 00:00:00 2001
From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Date: Wed, 18 May 2022 18:25:43 +0300
Subject: [PATCH 1/3] mount-v2: split out restore_one_sharing helper

This helper restores master_id and shared_id of first mount in the
sharing group. It first copies sharing from either external source or
internal parent sharing group and makes master_id from shared_id. Next
it creates new shared_id when needed.

All other mounts except first are just copied from the first one.

Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 criu/mount-v2.c | 60 ++++++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 25 deletions(-)

diff --git a/criu/mount-v2.c b/criu/mount-v2.c
index 623016d428..1d188114f7 100644
--- a/criu/mount-v2.c
+++ b/criu/mount-v2.c
@@ -925,27 +925,25 @@ static int move_mount_set_group(int src_id, char *source, int dst_id)
 	return 0;
 }
 
-static int restore_one_sharing_group(struct sharing_group *sg)
+static int restore_one_sharing(struct sharing_group *sg, struct mount_info *target)
 {
-	struct mount_info *first, *other;
-	char first_path[PATH_MAX];
-	int first_fd;
+	char target_path[PATH_MAX];
+	int target_fd;
 
-	first = get_first_mount(sg);
-	first_fd = fdstore_get(first->mnt_fd_id);
-	BUG_ON(first_fd < 0);
-	snprintf(first_path, sizeof(first_path), "/proc/self/fd/%d", first_fd);
+	target_fd = fdstore_get(target->mnt_fd_id);
+	BUG_ON(target_fd < 0);
+	snprintf(target_path, sizeof(target_path), "/proc/self/fd/%d", target_fd);
 
-	/* Restore first's master_id from shared_id of the source */
+	/* Restore target's master_id from shared_id of the source */
 	if (sg->master_id) {
 		if (sg->parent) {
-			struct mount_info *p;
+			struct mount_info *first;
 
 			/* Get shared_id from parent sharing group */
-			p = get_first_mount(sg->parent);
-			if (move_mount_set_group(p->mnt_fd_id, NULL, first->mnt_fd_id)) {
-				pr_err("Failed to copy sharing from %d to %d\n", p->mnt_id, first->mnt_id);
-				close(first_fd);
+			first = get_first_mount(sg->parent);
+			if (move_mount_set_group(first->mnt_fd_id, NULL, target->mnt_fd_id)) {
+				pr_err("Failed to copy sharing from %d to %d\n", first->mnt_id, target->mnt_id);
+				close(target_fd);
 				return -1;
 			}
 		} else {
@@ -956,30 +954,42 @@ static int restore_one_sharing_group(struct sharing_group *sg)
 			 * or non-shared slave). If source is a private mount
 			 * we would fail.
 			 */
-			if (move_mount_set_group(-1, sg->source, first->mnt_fd_id)) {
-				pr_err("Failed to copy sharing from source %s to %d\n", sg->source, first->mnt_id);
-				close(first_fd);
+			if (move_mount_set_group(-1, sg->source, target->mnt_fd_id)) {
+				pr_err("Failed to copy sharing from source %s to %d\n", sg->source, target->mnt_id);
+				close(target_fd);
 				return -1;
 			}
 		}
 
 		/* Convert shared_id to master_id */
-		if (mount(NULL, first_path, NULL, MS_SLAVE, NULL)) {
-			pr_perror("Failed to make mount %d slave", first->mnt_id);
-			close(first_fd);
+		if (mount(NULL, target_path, NULL, MS_SLAVE, NULL)) {
+			pr_perror("Failed to make mount %d slave", target->mnt_id);
+			close(target_fd);
 			return -1;
 		}
 	}
 
-	/* Restore first's shared_id */
+	/* Restore target's shared_id */
 	if (sg->shared_id) {
-		if (mount(NULL, first_path, NULL, MS_SHARED, NULL)) {
-			pr_perror("Failed to make mount %d shared", first->mnt_id);
-			close(first_fd);
+		if (mount(NULL, target_path, NULL, MS_SHARED, NULL)) {
+			pr_perror("Failed to make mount %d shared", target->mnt_id);
+			close(target_fd);
 			return -1;
 		}
 	}
-	close(first_fd);
+	close(target_fd);
+
+	return 0;
+}
+
+static int restore_one_sharing_group(struct sharing_group *sg)
+{
+	struct mount_info *first, *other;
+
+	first = get_first_mount(sg);
+
+	if (restore_one_sharing(sg, first))
+		return -1;
 
 	/* Restore sharing for other mounts from the sharing group */
 	list_for_each_entry(other, &sg->mnt_list, mnt_sharing) {

From 061ab0d785465747a44174ad034d38c7080842be Mon Sep 17 00:00:00 2001
From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Date: Wed, 18 May 2022 18:25:43 +0300
Subject: [PATCH 2/3] mount-v2: workaround for multiple external bindmounts
 with no common root

It's a problem when while restoring sharing group we need to copy
sharing between two mounts with non-intersecting roots, because kernel
does not allow it.

We have a case https://github.com/opencontainers/runc/pull/3442, where
runc adds different devtmpfs file-bindmounts to container and there is
no fsroot mount in container for this devtmpfs, thus mount-v2 faces the
above problem.

Luckily for the case of external mounts which are in one sharing group
and which have non-intersecting roots, these mounts likely only have
external master with no sharing, so we can just copy sharing from
external source and make it slave as a workaround.

https://github.com/checkpoint-restore/criu/issues/1886

Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 criu/mount-v2.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/criu/mount-v2.c b/criu/mount-v2.c
index 1d188114f7..5d53e9a226 100644
--- a/criu/mount-v2.c
+++ b/criu/mount-v2.c
@@ -996,9 +996,35 @@ static int restore_one_sharing_group(struct sharing_group *sg)
 		if (other == first)
 			continue;
 
-		if (move_mount_set_group(first->mnt_fd_id, NULL, other->mnt_fd_id)) {
-			pr_err("Failed to copy sharing from %d to %d\n", first->mnt_id, other->mnt_id);
-			return -1;
+		if (is_sub_path(other->root, first->root)) {
+			if (move_mount_set_group(first->mnt_fd_id, NULL, other->mnt_fd_id)) {
+				pr_err("Failed to copy sharing from %d to %d\n", first->mnt_id, other->mnt_id);
+				return -1;
+			}
+		} else {
+			/*
+			 * Case where mounts of this sharing group don't have common root.
+			 * For instance we can create two sub-directories .a and .b in some
+			 * shared mount, bindmount them separately somethere and umount the
+			 * original mount. Now we have both bindmounts shared between each
+			 * other. Kernel only allows to copy sharing between mounts when
+			 * source root contains destination root, which is not true for
+			 * these two, so we can't just copy from first to other.
+			 *
+			 * For external sharing (!sg->parent) with only master_id (shared_id
+			 * == 0) we can workaround this by copying from their external source
+			 * instead (same as we did for a first mount).
+			 *
+			 * This is a w/a runc usecase, see https://github.com/opencontainers/runc/pull/3442
+			 */
+			if (!sg->parent && !sg->shared_id) {
+				if (restore_one_sharing(sg, other))
+					return -1;
+			} else {
+				pr_err("Can't copy sharing from %d[%s] to %d[%s]\n", first->mnt_id, first->root,
+				       other->mnt_id, other->root);
+				return -1;
+			}
 		}
 	}
 

From 24bf471d5ba2b8c2c291bfceac2b1218aa0750c1 Mon Sep 17 00:00:00 2001
From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Date: Fri, 20 May 2022 12:11:49 +0300
Subject: [PATCH 3/3] zdtm: test multiple ext bindmounts with no common root
 and same master

Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 test/zdtm/static/Makefile              |   1 +
 test/zdtm/static/mnt_ext_multiple.c    | 118 +++++++++++++++++++++++++
 test/zdtm/static/mnt_ext_multiple.desc |   5 ++
 3 files changed, 124 insertions(+)
 create mode 100644 test/zdtm/static/mnt_ext_multiple.c
 create mode 100644 test/zdtm/static/mnt_ext_multiple.desc

diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile
index 9dc02d4a58..761bf0f6c6 100644
--- a/test/zdtm/static/Makefile
+++ b/test/zdtm/static/Makefile
@@ -406,6 +406,7 @@ TST_DIR		=				\
 		mntns_pivot_root		\
 		mntns_pivot_root_ro		\
 		mnt_ext_sharing			\
+		mnt_ext_multiple		\
 		mount_complex_sharing		\
 		mnt_tracefs			\
 		mntns_deleted			\
diff --git a/test/zdtm/static/mnt_ext_multiple.c b/test/zdtm/static/mnt_ext_multiple.c
new file mode 100644
index 0000000000..7014927ac3
--- /dev/null
+++ b/test/zdtm/static/mnt_ext_multiple.c
@@ -0,0 +1,118 @@
+#include <sched.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <linux/limits.h>
+
+#include "zdtmtst.h"
+
+const char *test_doc = "Check multiple non-common root external mounts with same external master";
+const char *test_author = "Pavel Tikhomirov <ptikhomirov@virtuozzo.com>";
+
+char *dirname = "mnt_ext_multiple.test";
+char *source = "zdtm_ext_multiple";
+char *ext_source = "zdtm_ext_multiple.ext";
+TEST_OPTION(dirname, string, "directory name", 1);
+
+int main(int argc, char **argv)
+{
+	char *root, testdir[PATH_MAX];
+	char dst_a[PATH_MAX], dst_b[PATH_MAX];
+	char src[PATH_MAX], src_a[PATH_MAX], src_b[PATH_MAX];
+	char nsdst_a[PATH_MAX], nsdst_b[PATH_MAX];
+	char *tmp = "/tmp/zdtm_ext_multiple.tmp";
+	char *zdtm_newns = getenv("ZDTM_NEWNS");
+
+	root = getenv("ZDTM_ROOT");
+	if (root == NULL) {
+		pr_perror("root");
+		return 1;
+	}
+
+	if (!zdtm_newns) {
+		pr_perror("ZDTM_NEWNS is not set");
+		return 1;
+	} else if (strcmp(zdtm_newns, "1")) {
+		goto test;
+	}
+
+	/* Prepare directories in test root */
+	sprintf(testdir, "%s/%s", root, dirname);
+	mkdir(testdir, 0755);
+	sprintf(dst_a, "%s/%s/dst_a", root, dirname);
+	mkdir(dst_a, 0755);
+	sprintf(dst_b, "%s/%s/dst_b", root, dirname);
+	mkdir(dst_b, 0755);
+
+	/* Prepare directories in criu root */
+	mkdir(tmp, 0755);
+	if (mount(source, tmp, "tmpfs", 0, NULL)) {
+		pr_perror("mount tmpfs");
+		return 1;
+	}
+	if (mount(NULL, tmp, NULL, MS_PRIVATE, NULL)) {
+		pr_perror("make private");
+		return 1;
+	}
+	sprintf(src, "%s/src", tmp);
+	mkdir(src, 0755);
+
+	/* Create a shared mount in criu mntns */
+	if (mount(ext_source, src, "tmpfs", 0, NULL)) {
+		pr_perror("mount tmpfs");
+		return 1;
+	}
+	if (mount(NULL, src, NULL, MS_PRIVATE, NULL)) {
+		pr_perror("make private");
+		return 1;
+	}
+	if (mount(NULL, src, NULL, MS_SHARED, NULL)) {
+		pr_perror("make shared");
+		return 1;
+	}
+
+	/*
+	 * Create temporary mntns, next mounts will not show up in criu mntns
+	 */
+	if (unshare(CLONE_NEWNS)) {
+		pr_perror("unshare");
+		return 1;
+	}
+
+	/*
+	 * Populate to the tests root subdirectories of the src mount
+	 */
+	sprintf(src_a, "%s/src/a", tmp);
+	mkdir(src_a, 0755);
+	if (mount(src_a, dst_a, NULL, MS_BIND, NULL)) {
+		pr_perror("bind");
+		return 1;
+	}
+	sprintf(src_b, "%s/src/b", tmp);
+	mkdir(src_b, 0755);
+	if (mount(src_b, dst_b, NULL, MS_BIND, NULL)) {
+		pr_perror("bind");
+		return 1;
+	}
+
+test:
+	test_init(argc, argv);
+
+	/* Make "external" mounts to have external master */
+	sprintf(nsdst_a, "/%s/dst_a", dirname);
+	if (mount(NULL, nsdst_a, NULL, MS_SLAVE, NULL)) {
+		pr_perror("make slave");
+		return 1;
+	}
+	sprintf(nsdst_b, "/%s/dst_b", dirname);
+	if (mount(NULL, nsdst_b, NULL, MS_SLAVE, NULL)) {
+		pr_perror("make slave");
+		return 1;
+	}
+
+	test_daemon();
+	test_waitsig();
+
+	pass();
+
+	return 0;
+}
diff --git a/test/zdtm/static/mnt_ext_multiple.desc b/test/zdtm/static/mnt_ext_multiple.desc
new file mode 100644
index 0000000000..fd413ed15c
--- /dev/null
+++ b/test/zdtm/static/mnt_ext_multiple.desc
@@ -0,0 +1,5 @@
+{   'dopts': '--external mnt[/mnt_ext_multiple.test/dst_a]:MNT_A --external mnt[/mnt_ext_multiple.test/dst_b]:MNT_B',
+    'feature': 'mnt_id move_mount_set_group',
+    'flavor': 'ns uns',
+    'flags': 'suid',
+    'ropts': '--external mnt[MNT_A]:/tmp/zdtm_ext_multiple.tmp/src/a --external mnt[MNT_B]:/tmp/zdtm_ext_multiple.tmp/src/b --no-mntns-compat-mode'}