Hello!

In attachment there are 2 patches to fix some issues with inherited
fd's. Instead of just exiting with error, maybe it's better to set
O_CLOEXEC flag.

1-st patch changes lxc-init and lxc-attach to use clone instead of fork.
Reason for it: clone permits to set custom flags, so you can call it
without CLONE_FILES and all fd's with O_CLOEXEC flag won't be passed to
child.

I'm not sure who and when should free child's stack. It seems to me that
Linux handles it by itself, so stack is malloced, but not freed.

2-nd patch changes lxc-start to set CLOEXEC instead of just returning
with error.

It seems to me that fd's are not passed to lxc-init.

-- 
Best regards, Vladimir Smirnov.

From bfdbefce2561134d5fd3d67a2047dc205b6e0974 Mon Sep 17 00:00:00 2001
From: Vladimir Smirnov <ci...@yandex-team.ru>
Date: Mon, 22 Aug 2011 15:14:07 +0400
Subject: [PATCH 1/2] Modify lxc_attach and lxc_init to use clone instead of
 fork.

Clone allows more flexible control. Currently, if there is any inherited fd,
lxc-start exits with error. With clone it's possible not to pass open fd's to childs.

Signed-off-by: Vladimir Smirnov <ci...@yandex-team.ru>
---
 src/lxc/lxc_attach.c |   79 +++++++++++++++++++++++++++++---------------------
 src/lxc/lxc_init.c   |   46 ++++++++++++++++++-----------
 2 files changed, 75 insertions(+), 50 deletions(-)

diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
index ed3d5a4..930865e 100644
--- a/src/lxc/lxc_attach.c
+++ b/src/lxc/lxc_attach.c
@@ -20,6 +20,7 @@
  * License along with this library; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */
+#define STACKSIZE 16384
 
 #define _GNU_SOURCE
 #include <unistd.h>
@@ -29,6 +30,7 @@
 #include <sys/param.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <sched.h>
 
 #include "commands.h"
 #include "arguments.h"
@@ -56,14 +58,48 @@ Options :\n\
 	.checker  = NULL,
 };
 
+static char *envp_global;
+
+int child_main()
+{
+	struct passwd *passwd;
+	uid_t uid;
+
+	if (my_args.argc) {
+		execve(my_args.argv[0], my_args.argv, envp_global);
+		SYSERROR("failed to exec '%s'", my_args.argv[0]);
+		return -1;
+	}
+
+	uid = getuid();
+
+	passwd = getpwuid(uid);
+	if (!passwd) {
+		SYSERROR("failed to get passwd "		\
+			 "entry for uid '%d'", uid);
+		return -1;
+	}
+
+	{
+		char *const args[] = {
+			passwd->pw_shell,
+			NULL,
+		};
+
+		execve(args[0], args, envp_global);
+		SYSERROR("failed to exec '%s'", args[0]);
+		return -1;
+	}
+}
+
 int main(int argc, char *argv[], char *envp[])
 {
 	int ret;
 	pid_t pid;
-	struct passwd *passwd;
-	uid_t uid;
 	char *curdir;
 
+	envp_global = envp;
+
 	ret = lxc_caps_init();
 	if (ret)
 		return ret;
@@ -96,7 +132,14 @@ int main(int argc, char *argv[], char *envp[])
 
 	free(curdir);
 
-	pid = fork();
+	void **newstack;
+	newstack = (void **) malloc(STACKSIZE);
+	if (!newstack)
+		exit(newstack);
+
+	newstack = (void **)(STACKSIZE + (char *)newstack);
+	*--newstack = 0;
+	pid = clone(child_main, newstack, CLONE_VM | CLONE_FS | CLONE_SIGHAND, 0);
 
 	if (pid < 0) {
 		SYSERROR("failed to fork");
@@ -120,35 +163,5 @@ int main(int argc, char *argv[], char *envp[])
 		return -1;
 	}
 
-	if (!pid) {
-
-		if (my_args.argc) {
-			execve(my_args.argv[0], my_args.argv, envp);
-			SYSERROR("failed to exec '%s'", my_args.argv[0]);
-			return -1;
-		}
-
-		uid = getuid();
-
-		passwd = getpwuid(uid);
-		if (!passwd) {
-			SYSERROR("failed to get passwd "		\
-				 "entry for uid '%d'", uid);
-			return -1;
-		}
-
-		{
-			char *const args[] = {
-				passwd->pw_shell,
-				NULL,
-			};
-
-			execve(args[0], args, envp);
-			SYSERROR("failed to exec '%s'", args[0]);
-			return -1;
-		}
-
-	}
-
 	return 0;
 }
diff --git a/src/lxc/lxc_init.c b/src/lxc/lxc_init.c
index a534b51..7debd6f 100644
--- a/src/lxc/lxc_init.c
+++ b/src/lxc/lxc_init.c
@@ -20,6 +20,7 @@
  * License along with this library; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */
+#define STACKSIZE 16384
 
 #include <stdio.h>
 #include <unistd.h>
@@ -32,6 +33,7 @@
 #include <sys/wait.h>
 #define _GNU_SOURCE
 #include <getopt.h>
+#include <sched.h>
 
 #include "log.h"
 #include "caps.h"
@@ -40,6 +42,9 @@
 
 lxc_log_define(lxc_init, lxc);
 
+static sigset_t mask, omask;
+static char **aargv;
+
 static int quiet;
 
 static struct option options[] = {
@@ -49,6 +54,23 @@ static struct option options[] = {
 
 static	int was_interrupted = 0;
 
+int child_main()
+{
+	int i;
+	int err = -1;
+	/* restore default signal handlers */
+	for (i = 1; i < NSIG; i++)
+		signal(i, SIG_DFL);
+
+	sigprocmask(SIG_SETMASK, &omask, NULL);
+
+	NOTICE("about to exec '%s'", aargv[0]);
+
+	execvp(aargv[0], aargv);
+	ERROR("failed to exec: '%s' : %m", aargv[0]);
+	exit(err);
+}
+
 int main(int argc, char *argv[])
 {
 
@@ -61,8 +83,6 @@ int main(int argc, char *argv[])
 	pid_t pid;
 	int nbargs = 0;
 	int err = -1;
-	char **aargv;
-	sigset_t mask, omask;
 	int i, shutdown = 0;
 
 	while (1) {
@@ -115,25 +135,17 @@ int main(int argc, char *argv[])
 	if (lxc_caps_reset())
 		exit(err);
 
-	pid = fork();
-
-	if (pid < 0)
+	void **newstack;
+	newstack = (void **) malloc(STACKSIZE);
+	if (!newstack)
 		exit(err);
 
-	if (!pid) {
-
-		/* restore default signal handlers */
-		for (i = 1; i < NSIG; i++)
-			signal(i, SIG_DFL);
+	newstack = (void **)(STACKSIZE + (char *)newstack);
+	*--newstack = 0;
+	pid = clone(child_main, newstack, CLONE_VM | CLONE_FS | CLONE_SIGHAND, 0);
 
-		sigprocmask(SIG_SETMASK, &omask, NULL);
-
-		NOTICE("about to exec '%s'", aargv[0]);
-
-		execvp(aargv[0], aargv);
-		ERROR("failed to exec: '%s' : %m", aargv[0]);
+	if (pid < 0)
 		exit(err);
-	}
 
 	/* let's process the signals now */
 	sigdelset(&omask, SIGALRM);
-- 
1.7.6

From 60588293338576adc086e5f507cf86732439bfa5 Mon Sep 17 00:00:00 2001
From: Vladimir Smirnov <ci...@yandex-team.ru>
Date: Mon, 22 Aug 2011 15:16:18 +0400
Subject: [PATCH 2/2] lxc-start shouldn't exit with error, if there is
 inherited fd's.

Previous patch fixed behaviour with clone, so it's now safe just to set O_CLOEXEC flag on
all inherited fd's.

Signed-off-by: Vladimir Smirnov <ci...@yandex-team.ru>
---
 src/lxc/start.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/lxc/start.c b/src/lxc/start.c
index b8ceff6..6df70dc 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -154,6 +154,7 @@ int lxc_check_inherited(int fd_to_ignore)
 	while (!readdir_r(dir, &dirent, &direntp)) {
 		char procpath[64];
 		char path[PATH_MAX];
+		int flags;
 
 		if (!direntp)
 			break;
@@ -174,14 +175,24 @@ int lxc_check_inherited(int fd_to_ignore)
 		/*
 		 * found inherited fd
 		 */
-		ret = -1;
+		flags = fcntl(fd, F_GETFD);
+		if (flags < 0) {
+			ret = -1;
+			ERROR("failed to get flags, fd %d on %s", fd, path);
+		}
+
+		fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
+		if (flags < 0) {
+			ret = -1;
+			ERROR("failed to set CLOEXEC, fd %d on %s", fd, path);
+		}
 
 		snprintf(procpath, sizeof(procpath), "/proc/self/fd/%d", fd);
 
 		if (readlink(procpath, path, sizeof(path)) == -1)
 			ERROR("readlink(%s) failed : %m", procpath);
 		else
-			ERROR("inherited fd %d on %s", fd, path);
+			WARN("inherited fd %d on %s", fd, path);
 	}
 
 	if (closedir(dir))
-- 
1.7.6

Attachment: signature.asc
Description: OpenPGP digital signature

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, 
user administration capabilities and model configuration. Take 
the hassle out of deploying and managing Subversion and the 
tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel

Reply via email to