[lxc-devel] [lxc/master] lxc-attach: attach even without sigwinch handler

brauner on Github lxc-bot at linuxcontainers.org
Thu Apr 7 15:33:08 UTC 2016


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1603 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160407/741166d9/attachment.bin>
-------------- next part --------------
From 206e310b2d8022d7e1cd1cc6fa17df18fea659ee Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at mailbox.org>
Date: Thu, 7 Apr 2016 17:10:20 +0200
Subject: [PATCH] lxc-attach: attach even without sigwinch handler

lxc_console_create() calls lxc_console_peer_default() which in turn calls
lxc_console_sigwinch_init() which sets up the lxc_tty_state for the present pty.
Prior to this commit lxc_console_sigwinch_init() would consider failures to
install a SIGWINCH handler fatal and and return NULL. This commit makes failures
to install a SIGWINCH handler non-fatal. In such cases the lxc_tty_state struct
will still be set up but the sigfd member, which contains the fd which received
SIGWINCH events, will be set to -1. (This also entails that the sigset_t oldmaks
field is meaningless.) Callers of lxc_console_sigwinch_init() and
lxc_console_sigwinch_fini() should thus make sure that sigfd >= 0 or sigfd != -1
before trying to register a SIGWINCH handler in e.g. an lxc_mainloop (cf.
lxc_attach.c), or resetting the sigmask (cf. lxc_attach.c).

These changes also imply that lxc_console_sigwinch_init() only failes with
ENOMEM. Thus, all cases where lxc_console_sigwinch_init() returns NULL are to be
considered fatal. This wasn't the case before this commit.

Prior to this commit lxc_console_peer_default() did return void. This commit
makes lxc_console_peer_default() return a status code which can be used to
detect errors.

Signed-off-by: Christian Brauner <christian.brauner at mailbox.org>
---
 src/lxc/console.c    | 58 ++++++++++++++++++++++++++--------------------------
 src/lxc/console.h    |  4 +++-
 src/lxc/lxc_attach.c | 21 +++++++++++--------
 3 files changed, 45 insertions(+), 38 deletions(-)

diff --git a/src/lxc/console.c b/src/lxc/console.c
index 5df73e4..ebf9473 100644
--- a/src/lxc/console.c
+++ b/src/lxc/console.c
@@ -118,9 +118,9 @@ struct lxc_tty_state *lxc_console_sigwinch_init(int srcfd, int dstfd)
 		return NULL;
 
 	memset(ts, 0, sizeof(*ts));
-	ts->stdinfd  = srcfd;
+	ts->stdinfd = srcfd;
 	ts->masterfd = dstfd;
-	ts->sigfd    = -1;
+	ts->sigfd = -1;
 
 	/* add tty to list to be scanned at SIGWINCH time */
 	lxc_list_add_elem(&ts->node, ts);
@@ -129,26 +129,19 @@ struct lxc_tty_state *lxc_console_sigwinch_init(int srcfd, int dstfd)
 	sigemptyset(&mask);
 	sigaddset(&mask, SIGWINCH);
 	if (sigprocmask(SIG_BLOCK, &mask, &ts->oldmask)) {
-		SYSERROR("failed to block SIGWINCH");
-		goto err1;
+		SYSERROR("failed to block SIGWINCH.");
+		ts->sigfd = -1;
 	}
 
 	ts->sigfd = signalfd(-1, &mask, 0);
 	if (ts->sigfd < 0) {
-		SYSERROR("failed to get signalfd");
-		goto err2;
+		SYSERROR("failed to get signalfd.");
+		sigprocmask(SIG_SETMASK, &ts->oldmask, NULL);
+		ts->sigfd = -1;
 	}
 
-	DEBUG("%d got SIGWINCH fd %d", getpid(), ts->sigfd);
-	goto out;
-
-err2:
-	sigprocmask(SIG_SETMASK, &ts->oldmask, NULL);
-err1:
-	lxc_list_del(&ts->node);
-	free(ts);
-	ts = NULL;
-out:
+	if (ts->sigfd != -1)
+		DEBUG("%d got SIGWINCH fd %d", getpid(), ts->sigfd);
 	return ts;
 }
 
@@ -207,7 +200,7 @@ static void lxc_console_mainloop_add_peer(struct lxc_console *console)
 			WARN("console peer not added to mainloop");
 	}
 
-	if (console->tty_state) {
+	if (console->tty_state && console->tty_state->sigfd != -1) {
 		if (lxc_mainloop_add_handler(console->descr,
 					     console->tty_state->sigfd,
 					     lxc_console_cb_sigwinch_fd,
@@ -291,7 +284,7 @@ int lxc_setup_tios(int fd, struct termios *oldtios)
 
 static void lxc_console_peer_proxy_free(struct lxc_console *console)
 {
-	if (console->tty_state) {
+	if (console->tty_state && console->tty_state->sigfd != -1) {
 		lxc_console_sigwinch_fini(console->tty_state);
 		console->tty_state = NULL;
 	}
@@ -414,7 +407,7 @@ void lxc_console_free(struct lxc_conf *conf, int fd)
 	}
 }
 
-static void lxc_console_peer_default(struct lxc_console *console)
+static int lxc_console_peer_default(struct lxc_console *console)
 {
 	struct lxc_tty_state *ts;
 	const char *path = console->path;
@@ -446,9 +439,11 @@ static void lxc_console_peer_default(struct lxc_console *console)
 		goto err1;
 
 	ts = lxc_console_sigwinch_init(console->peer, console->master);
-	if (!ts)
-		WARN("Unable to install SIGWINCH");
 	console->tty_state = ts;
+	if (!ts) {
+		WARN("Unable to install SIGWINCH");
+		goto err1;
+	}
 
 	lxc_console_winsz(console->peer, console->master);
 
@@ -461,7 +456,7 @@ static void lxc_console_peer_default(struct lxc_console *console)
 	if (lxc_setup_tios(console->peer, console->tios) < 0)
 		goto err2;
 
-	return;
+	return 0;
 
 err2:
 	free(console->tios);
@@ -471,6 +466,7 @@ static void lxc_console_peer_default(struct lxc_console *console)
 	console->peer = -1;
 out:
 	DEBUG("no console peer");
+	return -1;
 }
 
 void lxc_console_delete(struct lxc_console *console)
@@ -528,7 +524,8 @@ int lxc_console_create(struct lxc_conf *conf)
 		goto err;
 	}
 
-	lxc_console_peer_default(console);
+	if (lxc_console_peer_default(console) < 0)
+		goto err;
 
 	if (console->log_path) {
 		console->log_fd = lxc_unpriv(open(console->log_path,
@@ -695,11 +692,13 @@ int lxc_console(struct lxc_container *c, int ttynum,
 		goto err3;
 	}
 
-	ret = lxc_mainloop_add_handler(&descr, ts->sigfd,
-				       lxc_console_cb_sigwinch_fd, ts);
-	if (ret) {
-		ERROR("failed to add handler for SIGWINCH fd");
-		goto err4;
+	if (ts->sigfd != -1) {
+		ret = lxc_mainloop_add_handler(&descr, ts->sigfd,
+				lxc_console_cb_sigwinch_fd, ts);
+		if (ret) {
+			ERROR("failed to add handler for SIGWINCH fd");
+			goto err4;
+		}
 	}
 
 	ret = lxc_mainloop_add_handler(&descr, ts->stdinfd,
@@ -727,7 +726,8 @@ int lxc_console(struct lxc_container *c, int ttynum,
 err4:
 	lxc_mainloop_close(&descr);
 err3:
-	lxc_console_sigwinch_fini(ts);
+	if (ts->sigfd != -1)
+		lxc_console_sigwinch_fini(ts);
 err2:
 	close(masterfd);
 	close(ttyfd);
diff --git a/src/lxc/console.h b/src/lxc/console.h
index 1f0a070..d3ccd8b 100644
--- a/src/lxc/console.h
+++ b/src/lxc/console.h
@@ -47,7 +47,9 @@ struct lxc_tty_state
 	const char *winch_proxy;
 	/* Path of the container to forward the SIGWINCH event to. */
 	const char *winch_proxy_lxcpath;
-	/* File descriptor that accepts SIGWINCH signals. */
+	/* File descriptor that accepts SIGWINCH signals. If set to -1 no
+	 * SIGWINCH handler could be installed. This also means that
+	 * the sigset_t oldmask member is meaningless. */
 	int sigfd;
 	sigset_t oldmask;
 };
diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
index ffcd3d1..cdc56d6 100644
--- a/src/lxc/lxc_attach.c
+++ b/src/lxc/lxc_attach.c
@@ -264,7 +264,6 @@ static int get_pty_on_host(struct lxc_container *c, struct wrapargs *wrap, int *
 	 * always do the correct thing. strdup() must be used since console.path
 	 * is free()ed when we call lxc_container_put(). */
 	free(conf->console.path);
-	conf->console.path = NULL;
 	conf->console.path = strdup("/dev/tty");
 	if (!conf->console.path)
 		return -1;
@@ -326,12 +325,17 @@ static int get_pty_on_host(struct lxc_container *c, struct wrapargs *wrap, int *
 		goto err2;
 	}
 
-	/* Register sigwinch handler in mainloop. */
-	ret = lxc_mainloop_add_handler(&descr, ts->sigfd,
-			lxc_console_cb_sigwinch_fd, ts);
-	if (ret) {
-		ERROR("failed to add handler for SIGWINCH fd");
-		goto err3;
+	/* Register sigwinch handler in mainloop. When ts->sigfd == -1 it means
+	 * we weren't able to install a sigwinch handler in
+	 * lxc_console_create(). We don't consider this fatal and just move on.
+	 */
+	if (ts->sigfd != -1) {
+		ret = lxc_mainloop_add_handler(&descr, ts->sigfd,
+				lxc_console_cb_sigwinch_fd, ts);
+		if (ret) {
+			ERROR("failed to add handler for SIGWINCH fd");
+			goto err3;
+		}
 	}
 
 	/* Register i/o callbacks in mainloop. */
@@ -359,7 +363,8 @@ static int get_pty_on_host(struct lxc_container *c, struct wrapargs *wrap, int *
 err3:
 	lxc_mainloop_close(&descr);
 err2:
-	lxc_console_sigwinch_fini(ts);
+	if (ts->sigfd != -1)
+		lxc_console_sigwinch_fini(ts);
 err1:
 	lxc_console_delete(&conf->console);
 


More information about the lxc-devel mailing list