Skip to content

Commit

Permalink
c/r: re-open fds after clone()
Browse files Browse the repository at this point in the history
If we don't re-open these after clone, the init process has a pointer to the
parent's /dev/{zero,null}. CRIU seese these and wants to dump the parent's
mount namespace, which is unnecessary. Instead, we should just re-open
stdin/out/err after we do the clone and pivot root, to ensure that we have
pointers to the devcies in init's rootfs instead of the host's.

v2: Only close fds if the container was daemonized. This didn't turn out as
    nicely as described on the list because lxc_start() doesn't actually have
    the struct lxc_container, so it cant see the flag. Instead, we just pass it
    down everywhere.

Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
  • Loading branch information
Tycho Andersen authored and stgraber committed Jul 1, 2015
1 parent e945574 commit 97844d1
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/lxc/execute.c
Expand Up @@ -111,7 +111,7 @@ static struct lxc_operations execute_start_ops = {
};

int lxc_execute(const char *name, char *const argv[], int quiet,
struct lxc_conf *conf, const char *lxcpath)
struct lxc_conf *conf, const char *lxcpath, bool backgrounded)
{
struct execute_args args = {
.argv = argv,
Expand All @@ -122,5 +122,5 @@ int lxc_execute(const char *name, char *const argv[], int quiet,
return -1;

conf->is_execute = 1;
return __lxc_start(name, conf, &execute_start_ops, &args, lxcpath);
return __lxc_start(name, conf, &execute_start_ops, &args, lxcpath, backgrounded);
}
22 changes: 13 additions & 9 deletions src/lxc/lxc.h
Expand Up @@ -27,6 +27,7 @@
extern "C" {
#endif

#include <stdbool.h>
#include <stddef.h>
#include <sys/select.h>
#include <sys/types.h>
Expand All @@ -44,24 +45,27 @@ struct lxc_arguments;

/*
* Start the specified command inside a system container
* @name : the name of the container
* @argv : an array of char * corresponding to the commande line
* @conf : configuration
* @name : the name of the container
* @argv : an array of char * corresponding to the commande line
* @conf : configuration
* @backgrounded : whether or not the container is daemonized
* Returns 0 on success, < 0 otherwise
*/
extern int lxc_start(const char *name, char *const argv[], struct lxc_conf *conf,
const char *lxcpath);
const char *lxcpath, bool backgrounded);

/*
* Start the specified command inside an application container
* @name : the name of the container
* @argv : an array of char * corresponding to the commande line
* @quiet : if != 0 then lxc-init won't produce any output
* @conf : configuration
* @name : the name of the container
* @argv : an array of char * corresponding to the commande line
* @quiet : if != 0 then lxc-init won't produce any output
* @conf : configuration
* @backgrounded : whether or not the container is daemonized
* Returns 0 on success, < 0 otherwise
*/
extern int lxc_execute(const char *name, char *const argv[], int quiet,
struct lxc_conf *conf, const char *lxcpath);
struct lxc_conf *conf, const char *lxcpath,
bool backgrounded);

/*
* Open the monitoring mechanism for a specific container
Expand Down
2 changes: 1 addition & 1 deletion src/lxc/lxc_execute.c
Expand Up @@ -139,7 +139,7 @@ int main(int argc, char *argv[])
if (lxc_config_define_load(&defines, conf))
return 1;

ret = lxc_execute(my_args.name, my_args.argv, my_args.quiet, conf, my_args.lxcpath[0]);
ret = lxc_execute(my_args.name, my_args.argv, my_args.quiet, conf, my_args.lxcpath[0], false);

lxc_conf_free(conf);

Expand Down
10 changes: 2 additions & 8 deletions src/lxc/lxccontainer.c
Expand Up @@ -584,7 +584,7 @@ static bool lxcapi_start(struct lxc_container *c, int useinit, char * const argv
container_mem_unlock(c);

if (useinit) {
ret = lxc_execute(c->name, argv, 1, conf, c->config_path);
ret = lxc_execute(c->name, argv, 1, conf, c->config_path, daemonize);
return ret == 0 ? true : false;
}

Expand Down Expand Up @@ -642,12 +642,6 @@ static bool lxcapi_start(struct lxc_container *c, int useinit, char * const argv
return false;
}
lxc_check_inherited(conf, true, -1);
close(0);
close(1);
close(2);
open("/dev/zero", O_RDONLY);
open("/dev/null", O_RDWR);
open("/dev/null", O_RDWR);
setsid();
} else {
if (!am_single_threaded()) {
Expand Down Expand Up @@ -687,7 +681,7 @@ static bool lxcapi_start(struct lxc_container *c, int useinit, char * const argv
goto out;
}

ret = lxc_start(c->name, argv, conf, c->config_path);
ret = lxc_start(c->name, argv, conf, c->config_path, daemonize);
c->error_num = ret;

if (conf->reboot) {
Expand Down
17 changes: 14 additions & 3 deletions src/lxc/start.c
Expand Up @@ -759,6 +759,15 @@ static int do_start(void *data)

close(handler->sigfd);

if (handler->backgrounded) {
close(0);
close(1);
close(2);
open("/dev/zero", O_RDONLY);
open("/dev/null", O_RDWR);
open("/dev/null", O_RDWR);
}

/* after this call, we are in error because this
* ops should not return as it execs */
handler->ops->start(handler, handler->data);
Expand Down Expand Up @@ -1112,7 +1121,8 @@ int get_netns_fd(int pid)
}

int __lxc_start(const char *name, struct lxc_conf *conf,
struct lxc_operations* ops, void *data, const char *lxcpath)
struct lxc_operations* ops, void *data, const char *lxcpath,
bool backgrounded)
{
struct lxc_handler *handler;
int err = -1;
Expand All @@ -1126,6 +1136,7 @@ int __lxc_start(const char *name, struct lxc_conf *conf,
}
handler->ops = ops;
handler->data = data;
handler->backgrounded = backgrounded;

if (must_drop_cap_sys_boot(handler->conf)) {
#if HAVE_SYS_CAPABILITY_H
Expand Down Expand Up @@ -1257,12 +1268,12 @@ static struct lxc_operations start_ops = {
};

int lxc_start(const char *name, char *const argv[], struct lxc_conf *conf,
const char *lxcpath)
const char *lxcpath, bool backgrounded)
{
struct start_args start_arg = {
.argv = argv,
};

conf->need_utmp_watch = 1;
return __lxc_start(name, conf, &start_ops, &start_arg, lxcpath);
return __lxc_start(name, conf, &start_ops, &start_arg, lxcpath, backgrounded);
}
3 changes: 2 additions & 1 deletion src/lxc/start.h
Expand Up @@ -74,6 +74,7 @@ struct lxc_handler {
const char *lxcpath;
void *cgroup_data;
int ttysock[2]; // socketpair for child->parent tty fd passing
bool backgrounded; // indicates whether should we close std{in,out,err} on start
};


Expand All @@ -85,7 +86,7 @@ extern void lxc_fini(const char *name, struct lxc_handler *handler);

extern int lxc_check_inherited(struct lxc_conf *conf, bool closeall, int fd_to_ignore);
int __lxc_start(const char *, struct lxc_conf *, struct lxc_operations *,
void *, const char *);
void *, const char *, bool);

extern void resolve_clone_flags(struct lxc_handler *handler);
#endif
Expand Down

0 comments on commit 97844d1

Please sign in to comment.