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 40856a337234b03fe871e84dcab74e6bc0ee2ba2..6adef9bf2121e8ec22caa4e52e67b3afe8a46a9c 100644 (file)
--- a/src/lxc/execute.c
+++ b/src/lxc/execute.c
@@ -120,7 +120,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};
 
@@ -129,5 +129,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 c9064ff08c75e1ed1ee81126d17613f47693a0bf..d3c08ddf22341e1385063af1ddbe58bf96e4889e 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 ede0be58f008a01bd4b59f820032bbba97c79674..ecb770f484880ec483e9b7a202b5056161dfbba3 100644 (file)
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -1066,10 +1066,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 4e2f8a433f37d11792a3b78d51b4c7b58285f223..c728a62be4ca9348db8ceff5fda060bf15bfd16e 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -1824,7 +1824,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 ret, status;
        struct lxc_conf *conf = handler->conf;
@@ -1920,6 +1920,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);
@@ -1965,13 +1967,13 @@ 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,
        };
 
-       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 64e5a937ec9dd675065f6638901f4681a606ecb1..e6aabe78c9125f269f646849ff9c8a339e49be9c 100644 (file)
--- a/src/lxc/start.h
+++ b/src/lxc/start.h
@@ -165,7 +165,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);