[CRIU] Re: [PATCH 0/4] tty, restore orphaned slaves

Cyrill Gorcunov gorcunov at openvz.org
Fri Sep 14 05:17:24 EDT 2012


On Fri, Sep 14, 2012 at 12:41:12AM +0400, Andrey Wagin wrote:
> Dump should fail, if a master is outside. This case should be dumped
> only with "--external-master".

Guys, could you please take a look on this patch. It depends on
reworked tty-info verification which I attach as well here.
Now i need to figure out how to test ext-tty case.

(I believe I did some code duplication in tty module, i'll brush it
 up, just wanted to share it earlier).

	Cyrill
-------------- next part --------------
>From 3b2df0f8f899af2de94a962cfaeb507903a66077 Mon Sep 17 00:00:00 2001
From: Cyrill Gorcunov <gorcunov at openvz.org>
Date: Fri, 14 Sep 2012 11:22:33 +0400
Subject: [PATCH 3/5] tty: Rework tty entry verification

Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 tty.c |   55 +++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/tty.c b/tty.c
index b0f9737..9a13598 100644
--- a/tty.c
+++ b/tty.c
@@ -638,7 +638,7 @@ static int tty_setup_slavery(void)
 	return 0;
 }
 
-static int veirfy_termios(u32 id, TermiosEntry *e)
+static int verify_termios(u32 id, TermiosEntry *e)
 {
 	if (e && e->n_c_cc < TERMIOS_NCC) {
 		pr_err("pty ID %#x n_c_cc (%d) has wrong value\n",
@@ -648,6 +648,47 @@ static int veirfy_termios(u32 id, TermiosEntry *e)
 	return 0;
 }
 
+#define term_opts_missing_cmp(p, op)		\
+	(!(p)->tie->termios		op	\
+	 !(p)->tie->termios_locked	op	\
+	 !(p)->tie->winsize)
+
+#define term_opts_missing_any(p)		\
+	term_opts_missing_cmp(p, ||)
+
+#define term_opts_missing_all(p)		\
+	term_opts_missing_cmp(p, &&)
+
+static int verify_info(struct tty_info *info)
+{
+	/*
+	 * Master peer must have all parameters present,
+	 * while slave peer must have either all parameters present
+	 * or don't have them at all.
+	 */
+	if (term_opts_missing_any(info)) {
+		if (pty_is_master(info)) {
+			pr_err("Corrupted master peer %x\n", info->tfe->id);
+			return -1;
+		} else if (!term_opts_missing_all(info)) {
+			pr_err("Corrupted slave peer %x\n", info->tfe->id);
+			return -1;
+		}
+	}
+
+	if (verify_termios(info->tfe->id, info->tie->termios_locked) ||
+	    verify_termios(info->tfe->id, info->tie->termios))
+		return -1;
+
+	if (!pty_is_master(info)) {
+		if (info->tie->sid || info->tie->pgrp)
+			pr_err("Found sid %d pgrp %d on slave peer %x\n",
+			       info->tie->sid, info->tie->pgrp, info->tfe->id);
+	}
+
+	return 0;
+}
+
 static TtyInfoEntry *lookup_tty_info_entry(u32 id)
 {
 	struct tty_info_entry *e;
@@ -700,18 +741,8 @@ static int collect_one_tty(void *obj, ProtobufCMessage *msg)
 	INIT_LIST_HEAD(&info->sibling);
 	info->major = major(info->tie->rdev);
 
-	/*
-	 * Verify data obtained from the image.
-	 */
-	if (veirfy_termios(info->tfe->id, info->tie->termios))
-		return -1;
-	else if (veirfy_termios(info->tfe->id, info->tie->termios_locked))
+	if (verify_info(info))
 		return -1;
-	else if (!pty_is_master(info) && (info->tie->sid || info->tie->pgrp)) {
-		pr_err("Found sid %d pgrp %d on slave peer %x\n",
-			info->tie->sid, info->tie->pgrp, info->tfe->id);
-		return -1;
-	}
 
 	pr_info("Collected tty ID %#x\n", info->tfe->id);
 
-- 
1.7.7.6

-------------- next part --------------
>From 9d11f5ac273c00593c7f8a98fc95fd5f6f3f291b Mon Sep 17 00:00:00 2001
From: Cyrill Gorcunov <gorcunov at openvz.org>
Date: Thu, 13 Sep 2012 21:07:57 +0400
Subject: [PATCH 4/5] tty: Restore orphan slavery peers

In case if there is no master peer associated
with a slave peer we have two cases

 - the master peer was closed before slave
 - we just have no master peer at all, but
   only slave one

In first case we just create a fake master peer
and connect slaves on it.

The second case is a bit more complex. Because
we have no master peer -- it means the slaves
are connected to some external master peer which
doesn't belong us. In a sake of security in such
case a user must provide "--ext-tty" option on
restore granting the program to connect to existing
external master peer.

Suggested-by: Andrey Vagin <avagin at openvz.org>
Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 cr-restore.c      |    4 +
 crtools.c         |    4 +
 include/crtools.h |    1 +
 include/tty.h     |    1 +
 tty.c             |  165 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 169 insertions(+), 6 deletions(-)

diff --git a/cr-restore.c b/cr-restore.c
index 4ce2336..4f5089a 100644
--- a/cr-restore.c
+++ b/cr-restore.c
@@ -144,6 +144,10 @@ static int prepare_shared(void)
 		goto err;
 
 	mark_pipe_master();
+	ret = tty_setup_orphan_slavery(&opts);
+	if (ret)
+		goto err;
+
 
 	ret = tty_prepare_shared();
 	if (ret)
diff --git a/crtools.c b/crtools.c
index 1a5eb6d..7695478 100644
--- a/crtools.c
+++ b/crtools.c
@@ -96,6 +96,7 @@ int main(int argc, char *argv[])
 			{ "evasive-devices", no_argument, 0, 45},
 			{ "pidfile", required_argument, 0, 46},
 			{ "veth-pair", required_argument, 0, 47},
+			{ "ext-tty", no_argument, 0, 48},
 			{ },
 		};
 
@@ -198,6 +199,9 @@ int main(int argc, char *argv[])
 				list_add(&n->node, &opts.veth_pairs);
 			}
 			break;
+		case 48:
+			opts.ext_tty = true;
+			break;
 		case 'V':
 			pr_msg("Version: %d.%d\n", CRIU_VERSION_MAJOR, CRIU_VERSION_MINOR);
 			return 0;
diff --git a/include/crtools.h b/include/crtools.h
index 81eca5b..c79dc72 100644
--- a/include/crtools.h
+++ b/include/crtools.h
@@ -88,6 +88,7 @@ struct cr_options {
 	bool			show_pages_content;
 	bool			restore_detach;
 	bool			ext_unix_sk;
+	bool			ext_tty;
 	bool			tcp_established_ok;
 	bool			evasive_devices;
 	unsigned int		namespaces_flags;
diff --git a/include/tty.h b/include/tty.h
index 2a8f746..dbb1856 100644
--- a/include/tty.h
+++ b/include/tty.h
@@ -17,5 +17,6 @@ extern int dump_tty(struct fd_parms *p, int lfd, const struct cr_fdset *set);
 extern int collect_tty(void);
 extern int tty_is_master(struct fdinfo_list_entry *le);
 extern int tty_prepare_shared(void);
+extern int tty_setup_orphan_slavery(const struct cr_options *opts);
 
 #endif /* CR_TTY_H__ */
diff --git a/tty.c b/tty.c
index 9a13598..c3a504f 100644
--- a/tty.c
+++ b/tty.c
@@ -80,6 +80,10 @@ struct tty_info {
 
 	struct list_head		sibling;
 	int				major;
+
+	bool				create;
+	bool				external;
+	bool				noparams;
 };
 
 static LIST_HEAD(all_tty_info_entries);
@@ -520,6 +524,42 @@ static int receive_tty(struct tty_info *info)
 	return fd;
 }
 
+static int pty_open_fake_ptmx(struct tty_info *slave)
+{
+	int master = -1, ret = -1, fd = -1;
+	char pts_name[64];
+
+	snprintf(pts_name, sizeof(pts_name), PTS_FMT, slave->tie->pty->index);
+
+	master = pty_open_ptmx_index(O_RDONLY, slave->tie->pty->index);
+	if (master < 0) {
+		pr_perror("Can't open fale %x (index %d)",
+			  slave->tfe->id, slave->tie->pty->index);
+		return -1;
+	}
+
+	unlock_pty(master);
+
+	fd = open(pts_name, slave->tfe->flags);
+	if (fd < 0) {
+		pr_perror("Can't open slave %s", pts_name);
+		goto err;
+	}
+
+	if (restore_tty_params(fd, slave))
+		goto err;
+
+	if (pty_open_slaves(slave))
+		goto err;
+
+	ret = fd;
+	fd = -1;
+err:
+	close_safe(&master);
+	close_safe(&fd);
+	return ret;
+}
+
 static int pty_open_ptmx(struct tty_info *info)
 {
 	int master = -1;
@@ -561,15 +601,47 @@ err:
 	return -1;
 }
 
+static int pty_open_external(struct tty_info *slave)
+{
+	int ret = -1, fd = -1;
+	char pts_name[64];
+
+	snprintf(pts_name, sizeof(pts_name), PTS_FMT, slave->tie->pty->index);
+
+	fd = open(pts_name, slave->tfe->flags);
+	if (fd < 0) {
+		pr_perror("Can't open slave %s", pts_name);
+		goto err;
+	}
+
+	if (restore_tty_params(fd, slave))
+		goto err;
+
+	if (pty_open_slaves(slave))
+		goto err;
+
+	ret = fd;
+	fd = -1;
+err:
+	close_safe(&fd);
+	return ret;
+}
+
 static int tty_open(struct file_desc *d)
 {
 	struct tty_info *info = container_of(d, struct tty_info, d);
 
 	tty_show_pty_info("open", info);
 
-	if (!pty_is_master(info))
+	if (!info->create)
 		return receive_tty(info);
 
+	if (!pty_is_master(info)) {
+		if (!info->external)
+			return pty_open_fake_ptmx(info);
+		return pty_open_external(info);
+	}
+
 	return pty_open_ptmx(info);
 
 }
@@ -577,7 +649,7 @@ static int tty_open(struct file_desc *d)
 static int tty_transport(FdinfoEntry *fe, struct file_desc *d)
 {
 	struct tty_info *info = container_of(d, struct tty_info, d);
-	return pty_is_master(info) == false;
+	return !info->create;
 }
 
 static struct file_desc_ops tty_desc_ops = {
@@ -607,6 +679,82 @@ static int tty_find_restoring_task(struct tty_info *info)
 	return -1;
 }
 
+static int tty_verify_slavery_chain(struct tty_info *head, const struct cr_options *opts)
+{
+	struct tty_info *peer;
+
+	if (!head->noparams)
+		head->external = true;
+
+	list_for_each_entry(peer, &head->sibling, sibling) {
+		if (peer->noparams ^ head->noparams) {
+			pr_err("Unexpected status of parameters (%x: %d %x: %d)\n",
+			       head->tfe->id, head->noparams,
+			       peer->tfe->id, peer->noparams);
+			return -1;
+		} else if (!peer->noparams)
+			peer->external = true;
+	}
+
+	/*
+	 * At this point we know if all slaves on a list refer to
+	 * closed master or they all refer to external ttys. Thus
+	 * we need to verify if user allowed us to proceed.
+	 */
+	if (head->external) {
+		if (!opts->ext_tty) {
+			pr_err("The external slave peers found.\n"
+			       "But restoration is not allowed until\n"
+			       "explicitly requested. Try pass appropriate\n"
+			       "command line option\n");
+			return -1;
+		}
+	} else {
+		head->create = true;
+		pr_debug("Found orphan slave fake leader (%#x)\n", head->tfe->id);
+	}
+
+	return 0;
+}
+
+int tty_setup_orphan_slavery(const struct cr_options *opts)
+{
+	struct tty_info *info, *peer, *m;
+
+	list_for_each_entry(info, &all_ttys, list) {
+		struct fdinfo_list_entry *a, *b;
+		bool has_leader = false;
+
+		if (pty_is_master(info))
+			continue;
+
+		a = file_master(&info->d);
+		m = info;
+
+		list_for_each_entry(peer, &info->sibling, sibling) {
+			if (pty_is_master(peer)) {
+				has_leader = true;
+				break;
+			}
+
+			b = file_master(&peer->d);
+			if (a->pid > b->pid ||
+			    (a->pid == b->pid && a->fe->fd > b->fe->fd)) {
+				a = b;
+				m = peer;
+			}
+		}
+
+		/* Here is new one */
+		if (!has_leader) {
+			if (tty_verify_slavery_chain(m, opts))
+				return -1;
+		}
+	}
+
+	return 0;
+}
+
 static int tty_setup_slavery(void)
 {
 	struct tty_info *info, *peer, *m;
@@ -674,11 +822,13 @@ static int verify_info(struct tty_info *info)
 			pr_err("Corrupted slave peer %x\n", info->tfe->id);
 			return -1;
 		}
-	}
 
-	if (verify_termios(info->tfe->id, info->tie->termios_locked) ||
-	    verify_termios(info->tfe->id, info->tie->termios))
-		return -1;
+		info->noparams = true;
+	} else {
+		if (verify_termios(info->tfe->id, info->tie->termios_locked) ||
+		    verify_termios(info->tfe->id, info->tie->termios))
+			return -1;
+	}
 
 	if (!pty_is_master(info)) {
 		if (info->tie->sid || info->tie->pgrp)
@@ -740,6 +890,9 @@ static int collect_one_tty(void *obj, ProtobufCMessage *msg)
 
 	INIT_LIST_HEAD(&info->sibling);
 	info->major = major(info->tie->rdev);
+	info->create = (info->major == TTYAUX_MAJOR);
+	info->external = (info->tie->termios != NULL);
+	info->noparams = false;
 
 	if (verify_info(info))
 		return -1;
-- 
1.7.7.6



More information about the CRIU mailing list