From a3b4f3d68054eb31b86a7192bfc8ffabba011bff Mon Sep 17 00:00:00 2001 From: Tycho Andersen Date: Thu, 15 Mar 2018 15:29:27 +0000 Subject: [PATCH] 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 Signed-off-by: Tycho Andersen --- src/lxc/execute.c | 4 ++-- src/lxc/lxc.h | 4 ++-- src/lxc/lxccontainer.c | 5 ++--- src/lxc/start.c | 8 +++++--- src/lxc/start.h | 3 ++- 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/lxc/execute.c b/src/lxc/execute.c index 40856a337..6adef9bf2 100644 --- 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 c9064ff08..d3c08ddf2 100644 --- 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 ede0be58f..ecb770f48 100644 --- 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 4e2f8a433..c728a62be 100644 --- 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 64e5a937e..e6aabe78c 100644 --- 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);