fix handler use-after-free

The problem here is that __lxc_start frees the handler, so any use
afterwards is invalid. Since we don't have access to the actual struct
lxc_container object in __lxc_start, let's pass a pointer to error_num in
so it can be returned.

Unfortunately, I'm a little too paranoid to change the return type of
lxc_start, since it returns failure if some of the cleanup fails, which
may be useful in some cases. So let's keep this out of band.

Closes #2218
Closes #2219

Reported-by: Felix Abecassis <fabecassis@nvidia.com>
Signed-off-by: Tycho Andersen <tycho@tycho.ws>

diff --git a/src/lxc/execute.c b/src/lxc/execute.c
index 3eaa7b1ffd951e6a7df7c31c356c8be2d26464a2..3e0a99e06f5a823dcf098b78e76adb912bdbe78b 100644 (file)
--- a/src/lxc/execute.c
+++ b/src/lxc/execute.c
@@ -122,7 +122,7 @@ static struct lxc_operations execute_start_ops = {
 
 int lxc_execute(const char *name, char *const argv[], int quiet,
                struct lxc_handler *handler, const char *lxcpath,
-               bool backgrounded)
+               bool backgrounded, int *error_num)
 {
        struct execute_args args = {.argv = argv, .quiet = quiet};
 
@@ -131,5 +131,5 @@ int lxc_execute(const char *name, char *const argv[], int quiet,
 
        handler->conf->is_execute = 1;
        return __lxc_start(name, handler, &execute_start_ops, &args, lxcpath,
-                          backgrounded);
+                          backgrounded, error_num);
 }

diff --git a/src/lxc/lxc.h b/src/lxc/lxc.h
index c3c55828a8d7b54ff9f8d34a1498d8b42a75fef1..604e193a2450e15e7da706e0be855455eb886a06 100644 (file)
--- a/src/lxc/lxc.h
+++ b/src/lxc/lxc.h
@@ -54,7 +54,7 @@ struct lxc_handler;
  */
 extern int lxc_start(const char *name, char *const argv[],
                     struct lxc_handler *handler, const char *lxcpath,
-                    bool backgrounded);
+                    bool backgrounded, int *error_num);
 
 /*
  * Start the specified command inside an application container
@@ -67,7 +67,7 @@ extern int lxc_start(const char *name, char *const argv[],
  */
 extern int lxc_execute(const char *name, char *const argv[], int quiet,
                       struct lxc_handler *handler, const char *lxcpath,
-                      bool backgrounded);
+                      bool backgrounded, int *error_num);
 
 /*
  * Close the fd associated with the monitoring

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index d1f23c4205105d486e7fc595cfff70ecfd7705c9..e63dc264c9db03cd27697f13747eadcb4ebac76a 100644 (file)
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -1035,10 +1035,9 @@ reboot:
        }
 
        if (useinit)
-               ret = lxc_execute(c->name, argv, 1, handler, c->config_path, daemonize);
+               ret = lxc_execute(c->name, argv, 1, handler, c->config_path, daemonize, &c->error_num);
        else
-               ret = lxc_start(c->name, argv, handler, c->config_path, daemonize);
-       c->error_num = handler->exit_status;
+               ret = lxc_start(c->name, argv, handler, c->config_path, daemonize, &c->error_num);
 
        if (conf->reboot == 1) {
                INFO("Container requested reboot");

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 93a51e811faff928a444fa1b121a4c8f871add93..a92afe6c19f085202355a3506bc93bc747f61421 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -1737,7 +1737,7 @@ out_abort:
 
 int __lxc_start(const char *name, struct lxc_handler *handler,
                struct lxc_operations* ops, void *data, const char *lxcpath,
-               bool backgrounded)
+               bool backgrounded, int *error_num)
 {
        int status;
        int err = -1;
@@ -1842,6 +1842,8 @@ int __lxc_start(const char *name, struct lxc_handler *handler,
 
        lxc_monitor_send_exit_code(name, status, handler->lxcpath);
        lxc_error_set_and_log(handler->pid, status);
+       if (error_num)
+               *error_num = handler->exit_status;
 
 out_fini:
        lxc_delete_network(handler);
@@ -1887,14 +1889,14 @@ static struct lxc_operations start_ops = {
 };
 
 int lxc_start(const char *name, char *const argv[], struct lxc_handler *handler,
-             const char *lxcpath, bool backgrounded)
+             const char *lxcpath, bool backgrounded, int *error_num)
 {
        struct start_args start_arg = {
                .argv = argv,
        };
 
        handler->conf->need_utmp_watch = 1;
-       return __lxc_start(name, handler, &start_ops, &start_arg, lxcpath, backgrounded);
+       return __lxc_start(name, handler, &start_ops, &start_arg, lxcpath, backgrounded, error_num);
 }
 
 static void lxc_destroy_container_on_signal(struct lxc_handler *handler,

diff --git a/src/lxc/start.h b/src/lxc/start.h
index cbcdf2f2ac24a97375ec8a1951deae8c3ade3ca4..833584994ea63866052e9b81f1d52d6fa4fa3a93 100644 (file)
--- a/src/lxc/start.h
+++ b/src/lxc/start.h
@@ -152,7 +152,8 @@ extern void lxc_fini(const char *name, struct lxc_handler *handler);
 extern int lxc_check_inherited(struct lxc_conf *conf, bool closeall,
                               int *fds_to_ignore, size_t len_fds);
 extern int __lxc_start(const char *, struct lxc_handler *,
-                      struct lxc_operations *, void *, const char *, bool);
+                      struct lxc_operations *, void *, const char *, bool,
+                      int *);
 
 extern int resolve_clone_flags(struct lxc_handler *handler);