From 0e1a60b0fb300d152681bd111a83188a15222ee7 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Fri, 23 Feb 2018 15:43:50 +0100 Subject: [PATCH] lxccontainer: do_lxcapi_save_config() If liblxc is used multi-threaded do_lxcapi_save_config() could be called from threads that fork() which to not risk ending up with invalid locking states we should avoid using functions like fopen() that internally allocate memory and use locking. Let's replace it with the async-signal safe combination of open() + write(). Signed-off-by: Christian Brauner --- src/lxc/confile.c | 16 +++++++++------ src/lxc/confile.h | 2 +- src/lxc/lxccontainer.c | 44 +++++++++++++++++++++++++----------------- 3 files changed, 37 insertions(+), 25 deletions(-) diff --git a/src/lxc/confile.c b/src/lxc/confile.c index effff93dd..8c3eccccc 100644 --- a/src/lxc/confile.c +++ b/src/lxc/confile.c @@ -2417,17 +2417,21 @@ signed long lxc_config_parse_arch(const char *arch) } /* Write out a configuration file. */ -void write_config(FILE *fout, struct lxc_conf *c) +int write_config(int fd, const struct lxc_conf *conf) { int ret; - size_t len = c->unexpanded_len; + size_t len = conf->unexpanded_len; - if (!len) - return; + if (len == 0) + return 0; - ret = fwrite(c->unexpanded_config, 1, len, fout); - if (ret != len) + ret = lxc_write_nointr(fd, conf->unexpanded_config, len); + if (ret < 0) { SYSERROR("Failed to write configuration file"); + return -1; + } + + return 0; } bool do_append_unexp_config_line(struct lxc_conf *conf, const char *key, diff --git a/src/lxc/confile.h b/src/lxc/confile.h index c8929658e..0d877c898 100644 --- a/src/lxc/confile.h +++ b/src/lxc/confile.h @@ -93,7 +93,7 @@ extern signed long lxc_config_parse_arch(const char *arch); extern int lxc_clear_config_item(struct lxc_conf *c, const char *key); -extern void write_config(FILE *fout, struct lxc_conf *c); +extern int write_config(int fd, const struct lxc_conf *conf); extern bool do_append_unexp_config_line(struct lxc_conf *conf, const char *key, const char *v); diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index 4b056b6b0..1c9d2b169 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -2418,9 +2418,8 @@ WRAP_API_3(int, lxcapi_get_keys, const char *, char *, int) static bool do_lxcapi_save_config(struct lxc_container *c, const char *alt_file) { - FILE *fout; + int fd, lret; bool ret = false, need_disklock = false; - int lret; if (!alt_file) alt_file = c->configfile; @@ -2430,7 +2429,10 @@ static bool do_lxcapi_save_config(struct lxc_container *c, const char *alt_file) /* If we haven't yet loaded a config, load the stock config. */ if (!c->lxc_conf) { if (!do_lxcapi_load_config(c, lxc_global_config_value("lxc.default_config"))) { - ERROR("Error loading default configuration file %s while saving %s", lxc_global_config_value("lxc.default_config"), c->name); + ERROR("Error loading default configuration file %s " + "while saving %s", + lxc_global_config_value("lxc.default_config"), + c->name); return false; } } @@ -2453,18 +2455,24 @@ static bool do_lxcapi_save_config(struct lxc_container *c, const char *alt_file) if (lret) return false; - fout = fopen(alt_file, "w"); - if (!fout) - goto out; - write_config(fout, c->lxc_conf); - fclose(fout); + fd = open(alt_file, O_WRONLY | O_CREAT | O_CLOEXEC, + S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH); + if (fd < 0) + goto on_error; + + lret = write_config(fd, c->lxc_conf); + close(fd); + if (lret < 0) + goto on_error; + ret = true; -out: +on_error: if (need_disklock) container_disk_unlock(c); else container_mem_unlock(c); + return ret; } @@ -3515,10 +3523,9 @@ static struct lxc_container *do_lxcapi_clone(struct lxc_container *c, const char char **hookargs) { char newpath[MAXPATHLEN]; - int ret; + int fd, ret; struct clone_update_data data; size_t saved_unexp_len; - FILE *fout; pid_t pid; int storage_copied = 0; char *origroot = NULL, *saved_unexp_conf = NULL; @@ -3562,9 +3569,11 @@ static struct lxc_container *do_lxcapi_clone(struct lxc_container *c, const char origroot = c->lxc_conf->rootfs.path; c->lxc_conf->rootfs.path = NULL; } - fout = fopen(newpath, "w"); - if (!fout) { - SYSERROR("open %s", newpath); + + fd = open(newpath, O_WRONLY | O_CREAT | O_CLOEXEC, + S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH); + if (fd < 0) { + SYSERROR("Failed to open \"%s\"", newpath); goto out; } @@ -3572,14 +3581,13 @@ static struct lxc_container *do_lxcapi_clone(struct lxc_container *c, const char saved_unexp_len = c->lxc_conf->unexpanded_len; c->lxc_conf->unexpanded_config = strdup(saved_unexp_conf); if (!c->lxc_conf->unexpanded_config) { - ERROR("Out of memory"); - fclose(fout); + close(fd); goto out; } clear_unexp_config_line(c->lxc_conf, "lxc.rootfs.path", false); - write_config(fout, c->lxc_conf); - fclose(fout); + write_config(fd, c->lxc_conf); + close(fd); c->lxc_conf->rootfs.path = origroot; free(c->lxc_conf->unexpanded_config); c->lxc_conf->unexpanded_config = saved_unexp_conf;