From 3999f50bd2a86ad897fe267b99a676cc7a4a7fe8 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Sun, 23 Sep 2018 17:55:27 +0200 Subject: [PATCH 1/6] cgfsng: s/cgfsng_destroy/cgfsng_payload_destroy/g Signed-off-by: Christian Brauner --- src/lxc/cgroups/cgfsng.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 5475f7eff..9a0dba55c 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -1111,7 +1111,8 @@ static int cgroup_rmdir_wrapper(void *data) return cgroup_rmdir(arg->hierarchies, arg->container_cgroup); } -__cgfsng_ops static void cgfsng_destroy(struct cgroup_ops *ops, struct lxc_handler *handler) +__cgfsng_ops__ static void cgfsng_payload_destroy(struct cgroup_ops *ops, + struct lxc_handler *handler) { int ret; struct generic_userns_exec_data wrap; @@ -2681,7 +2682,7 @@ struct cgroup_ops *cgfsng_ops_init(struct lxc_conf *conf) } cgfsng_ops->data_init = cgfsng_data_init; - cgfsng_ops->destroy = cgfsng_destroy; + cgfsng_ops->destroy = cgfsng_payload_destroy; cgfsng_ops->monitor_create = cgfsng_monitor_create; cgfsng_ops->monitor_enter = cgfsng_monitor_enter; cgfsng_ops->payload_create = cgfsng_payload_create; From 434c8e15c9821bce65d3bc1eefae0ecfa843ef1e Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Sun, 23 Sep 2018 20:11:56 +0200 Subject: [PATCH 2/6] cgfsng: add cgfsng_monitor_destroy() Since we switched to the new cgroup scoping scheme that places the container payload into lxc.payload/ and lxc.monitor/ deletion becomes slightly more complicated. The monitor will be able to rm_rf(lxc.payload/) but will not be able to rm_rf(lxc.monitor/) since it will be located in that cgroup and it will thus be populated. My current solution to this is to create a lxc.pivot cgroup that only exists so that the monitor process on container stop can pivot into it, call rm_rf(lxc.monitor/) and can then exit. This group has not function whatsoever apart from this and can thus be shared by all monitor processes. Signed-off-by: Christian Brauner --- src/lxc/cgroups/cgfsng.c | 61 ++++++++++++++++++++++++++++++++++++++-- src/lxc/cgroups/cgroup.h | 3 +- src/lxc/start.c | 10 +++++-- src/lxc/start.h | 3 ++ 4 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 9a0dba55c..a2d2f1927 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -1111,8 +1111,8 @@ static int cgroup_rmdir_wrapper(void *data) return cgroup_rmdir(arg->hierarchies, arg->container_cgroup); } -__cgfsng_ops__ static void cgfsng_payload_destroy(struct cgroup_ops *ops, - struct lxc_handler *handler) +__cgfsng_ops static void cgfsng_payload_destroy(struct cgroup_ops *ops, + struct lxc_handler *handler) { int ret; struct generic_userns_exec_data wrap; @@ -1133,6 +1133,60 @@ __cgfsng_ops__ static void cgfsng_payload_destroy(struct cgroup_ops *ops, } } +__cgfsng_ops static void cgfsng_monitor_destroy(struct cgroup_ops *ops, + struct lxc_handler *handler) +{ + int len; + char *pivot_path; + struct lxc_conf *conf = handler->conf; + char pidstr[INTTYPE_TO_STRLEN(pid_t)]; + + if (!ops->hierarchies) + return; + + len = snprintf(pidstr, sizeof(pidstr), "%d", handler->monitor_pid); + if (len < 0 || (size_t)len >= sizeof(pidstr)) + return; + + for (int i = 0; ops->hierarchies[i]; i++) { + int ret; + struct hierarchy *h = ops->hierarchies[i]; + + if (!h->monitor_full_path) + continue; + + if (conf && conf->cgroup_meta.dir) + pivot_path = must_make_path(h->mountpoint, + h->container_base_path, + conf->cgroup_meta.dir, + "lxc.pivot", + "cgroup.procs", NULL); + else + pivot_path = must_make_path(h->mountpoint, + h->container_base_path, + "lxc.pivot", + "cgroup.procs", NULL); + + ret = mkdir_p(pivot_path, 0755); + if (ret < 0 && errno != EEXIST) + goto next; + + /* Move ourselves into the pivot cgroup to delete our own + * cgroup. + */ + ret = lxc_write_to_file(pivot_path, pidstr, len, false, 0666); + if (ret != 0) + goto next; + + ret = recursive_destroy(h->monitor_full_path); + if (ret < 0) + WARN("Failed to destroy \"%s\"", h->monitor_full_path); + + next: + free(pivot_path); + } +} + static bool cg_unified_create_cgroup(struct hierarchy *h, char *cgname) { size_t i, parts_len; @@ -2682,7 +2736,8 @@ struct cgroup_ops *cgfsng_ops_init(struct lxc_conf *conf) } cgfsng_ops->data_init = cgfsng_data_init; - cgfsng_ops->destroy = cgfsng_payload_destroy; + cgfsng_ops->payload_destroy = cgfsng_payload_destroy; + cgfsng_ops->monitor_destroy = cgfsng_monitor_destroy; cgfsng_ops->monitor_create = cgfsng_monitor_create; cgfsng_ops->monitor_enter = cgfsng_monitor_enter; cgfsng_ops->payload_create = cgfsng_payload_create; diff --git a/src/lxc/cgroups/cgroup.h b/src/lxc/cgroups/cgroup.h index 292fa1c92..e0a33563a 100644 --- a/src/lxc/cgroups/cgroup.h +++ b/src/lxc/cgroups/cgroup.h @@ -129,7 +129,8 @@ struct cgroup_ops { cgroup_layout_t cgroup_layout; bool (*data_init)(struct cgroup_ops *ops); - void (*destroy)(struct cgroup_ops *ops, struct lxc_handler *handler); + void (*payload_destroy)(struct cgroup_ops *ops, struct lxc_handler *handler); + void (*monitor_destroy)(struct cgroup_ops *ops, struct lxc_handler *handler); bool (*monitor_create)(struct cgroup_ops *ops, struct lxc_handler *handler); bool (*monitor_enter)(struct cgroup_ops *ops, pid_t pid); bool (*payload_create)(struct cgroup_ops *ops, struct lxc_handler *handler); diff --git a/src/lxc/start.c b/src/lxc/start.c index 0629e90c4..bbf31d68a 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -725,6 +725,8 @@ int lxc_init(const char *name, struct lxc_handler *handler) const char *loglevel; struct lxc_conf *conf = handler->conf; + handler->monitor_pid = lxc_raw_getpid(); + lsm_init(); TRACE("Initialized LSM"); @@ -857,7 +859,8 @@ int lxc_init(const char *name, struct lxc_handler *handler) return 0; out_destroy_cgroups: - handler->cgroup_ops->destroy(handler->cgroup_ops, handler); + handler->cgroup_ops->payload_destroy(handler->cgroup_ops, handler); + handler->cgroup_ops->monitor_destroy(handler->cgroup_ops, handler); out_delete_terminal: lxc_terminal_delete(&handler->conf->console); @@ -951,7 +954,8 @@ void lxc_fini(const char *name, struct lxc_handler *handler) lsm_process_cleanup(handler->conf, handler->lxcpath); - cgroup_ops->destroy(cgroup_ops, handler); + cgroup_ops->payload_destroy(cgroup_ops, handler); + cgroup_ops->monitor_destroy(cgroup_ops, handler); cgroup_exit(cgroup_ops); if (handler->conf->reboot == REBOOT_NONE) { @@ -1971,7 +1975,7 @@ int __lxc_start(const char *name, struct lxc_handler *handler, goto out_fini_nonet; } - if (!cgroup_ops->monitor_enter(cgroup_ops, lxc_raw_getpid())) { + if (!cgroup_ops->monitor_enter(cgroup_ops, handler->monitor_pid)) { ERROR("Failed to enter monitor cgroup"); goto out_fini_nonet; } diff --git a/src/lxc/start.h b/src/lxc/start.h index bcb852e8b..c3e68f3ae 100644 --- a/src/lxc/start.h +++ b/src/lxc/start.h @@ -103,6 +103,9 @@ struct lxc_handler { /* The child's pid. */ pid_t pid; + /* The monitor's pid. */ + pid_t monitor_pid; + /* Whether the child has already exited. */ bool init_died; From 625ad37b595868abb9c82fcf8665f2b4af522b5c Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 24 Sep 2018 00:14:22 +0200 Subject: [PATCH 3/6] cgroups: introduce helper macros Signed-off-by: Christian Brauner --- src/lxc/cgroups/cgfsng.c | 6 +++--- src/lxc/cgroups/cgroup.c | 1 - src/lxc/cgroups/cgroup.h | 6 ++++++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index a2d2f1927..22a559e12 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -1159,12 +1159,12 @@ __cgfsng_ops static void cgfsng_monitor_destroy(struct cgroup_ops *ops, pivot_path = must_make_path(h->mountpoint, h->container_base_path, conf->cgroup_meta.dir, - "lxc.pivot", + PIVOT_CGROUP, "cgroup.procs", NULL); else pivot_path = must_make_path(h->mountpoint, h->container_base_path, - "lxc.pivot", + PIVOT_CGROUP, "cgroup.procs", NULL); ret = mkdir_p(pivot_path, 0755); @@ -2714,7 +2714,7 @@ __cgfsng_ops static bool cgfsng_data_init(struct cgroup_ops *ops) return false; } ops->cgroup_pattern = must_copy_string(cgroup_pattern); - ops->monitor_pattern = must_copy_string("lxc.monitor"); + ops->monitor_pattern = MONITOR_CGROUP; return true; } diff --git a/src/lxc/cgroups/cgroup.c b/src/lxc/cgroups/cgroup.c index cf81f3edb..f223958da 100644 --- a/src/lxc/cgroups/cgroup.c +++ b/src/lxc/cgroups/cgroup.c @@ -76,7 +76,6 @@ void cgroup_exit(struct cgroup_ops *ops) free(ops->cgroup_pattern); free(ops->container_cgroup); - free(ops->monitor_pattern); for (it = ops->hierarchies; it && *it; it++) { char **ctrlr; diff --git a/src/lxc/cgroups/cgroup.h b/src/lxc/cgroups/cgroup.h index e0a33563a..976883a3c 100644 --- a/src/lxc/cgroups/cgroup.h +++ b/src/lxc/cgroups/cgroup.h @@ -28,6 +28,10 @@ #include #include +#define PAYLOAD_CGROUP "lxc.payload" +#define MONITOR_CGROUP "lxc.monitor" +#define PIVOT_CGROUP "lxc.pivot" + struct lxc_handler; struct lxc_conf; struct lxc_list; @@ -96,6 +100,8 @@ struct cgroup_ops { char **cgroup_use; char *cgroup_pattern; char *container_cgroup; + + /* Static memory, do not free.*/ char *monitor_pattern; /* @hierarchies From 5ce03bc048212feae5de5a3f41fe189bb7f9ee58 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 24 Sep 2018 11:04:04 +0200 Subject: [PATCH 4/6] cgfsng: ensure no-reuse in cgfsng_monitor_create() The same way we need to ensure that no existing cgroups are reused for the payload in cgfsng_payload_create() we need to ensure that no existing cgroups are reused for the monitor. Technially this is less of an issue since there currently is no logic for the monitor to apply limits to its cgroup but it is still the proper way to do it. Signed-off-by: Christian Brauner --- src/lxc/cgroups/cgfsng.c | 54 +++++++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 22a559e12..5d7effcba 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -1326,7 +1326,9 @@ static void remove_path_for_hierarchy(struct hierarchy *h, char *cgname, bool mo __cgfsng_ops static inline bool cgfsng_monitor_create(struct cgroup_ops *ops, struct lxc_handler *handler) { - char *monitor_cgroup; + char *monitor_cgroup, *offset, *tmp; + int idx = 0; + size_t len; bool bret = false; struct lxc_conf *conf = handler->conf; @@ -1334,24 +1336,46 @@ __cgfsng_ops static inline bool cgfsng_monitor_create(struct cgroup_ops *ops, return bret; if (conf->cgroup_meta.dir) - monitor_cgroup = lxc_string_join("/", (const char *[]){conf->cgroup_meta.dir, ops->monitor_pattern, handler->name, NULL}, false); + tmp = lxc_string_join("/", + (const char *[]){conf->cgroup_meta.dir, + ops->monitor_pattern, + handler->name, NULL}, + false); else - monitor_cgroup = must_make_path(ops->monitor_pattern, handler->name, NULL); - if (!monitor_cgroup) + tmp = must_make_path(ops->monitor_pattern, handler->name, NULL); + if (!tmp) return bret; - for (int i = 0; ops->hierarchies[i]; i++) { - if (!monitor_create_path_for_hierarchy(ops->hierarchies[i], monitor_cgroup)) { - ERROR("Failed to create cgroup \"%s\"", ops->hierarchies[i]->monitor_full_path); - free(ops->hierarchies[i]->container_full_path); - ops->hierarchies[i]->container_full_path = NULL; - for (int j = 0; j < i; j++) - remove_path_for_hierarchy(ops->hierarchies[j], monitor_cgroup, true); - goto on_error; - } - } + len = strlen(tmp) + 5; /* leave room for -NNN\0 */ + monitor_cgroup = must_alloc(len); + (void)strlcpy(monitor_cgroup, tmp, len); + free(tmp); + offset = monitor_cgroup + len - 5; - bret = true; + do { + if (idx) { + int ret = snprintf(offset, 5, "-%d", idx); + if (ret < 0 || (size_t)ret >= 5) + goto on_error; + } + + for (int i = 0; ops->hierarchies[i]; i++) { + if (!monitor_create_path_for_hierarchy(ops->hierarchies[i], monitor_cgroup)) { + ERROR("Failed to create cgroup \"%s\"", ops->hierarchies[i]->monitor_full_path); + free(ops->hierarchies[i]->container_full_path); + ops->hierarchies[i]->container_full_path = NULL; + + for (int j = 0; j < i; j++) + remove_path_for_hierarchy(ops->hierarchies[j], monitor_cgroup, true); + + idx++; + break; + } + } + } while (idx > 0 && idx < 1000); + + if (idx < 1000) + bret = true; on_error: free(monitor_cgroup); From a3650c0c4d54aa09ac8a18a768dbae37d4733801 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 26 Sep 2018 14:13:05 +0200 Subject: [PATCH 5/6] cgfsng: s/25/INTTYPE_TO_STRLEN(pid_t)/g Signed-off-by: Christian Brauner --- src/lxc/cgroups/cgfsng.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 5d7effcba..7388ad767 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -1465,10 +1465,10 @@ __cgfsng_ops static bool __do_cgroup_enter(struct cgroup_ops *ops, pid_t pid, bool monitor) { int len; - char pidstr[25]; + char pidstr[INTTYPE_TO_STRLEN(pid_t)]; - len = snprintf(pidstr, 25, "%d", pid); - if (len < 0 || len >= 25) + len = snprintf(pidstr, sizeof(pidstr), "%d", pid); + if (len < 0 || (size_t)len >= sizeof(pidstr)) return false; for (int i = 0; ops->hierarchies[i]; i++) { @@ -2097,10 +2097,10 @@ __cgfsng_ops static bool cgfsng_attach(struct cgroup_ops *ops, const char *name, const char *lxcpath, pid_t pid) { int i, len, ret; - char pidstr[25]; + char pidstr[INTTYPE_TO_STRLEN(pid_t)]; - len = snprintf(pidstr, 25, "%d", pid); - if (len < 0 || len >= 25) + len = snprintf(pidstr, sizeof(pidstr), "%d", pid); + if (len < 0 || (size_t)len >= sizeof(pidstr)) return false; for (i = 0; ops->hierarchies[i]; i++) { From ebc10afe213872d92c3d4641e34e6bd3c6d826be Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 26 Sep 2018 14:16:10 +0200 Subject: [PATCH 6/6] cgfsng: do not go into infinite loop Signed-off-by: Christian Brauner --- src/lxc/cgroups/cgfsng.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 7388ad767..e083ad6d6 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -1327,7 +1327,7 @@ __cgfsng_ops static inline bool cgfsng_monitor_create(struct cgroup_ops *ops, struct lxc_handler *handler) { char *monitor_cgroup, *offset, *tmp; - int idx = 0; + int i, idx = 0; size_t len; bool bret = false; struct lxc_conf *conf = handler->conf; @@ -1359,7 +1359,7 @@ __cgfsng_ops static inline bool cgfsng_monitor_create(struct cgroup_ops *ops, goto on_error; } - for (int i = 0; ops->hierarchies[i]; i++) { + for (i = 0; ops->hierarchies[i]; i++) { if (!monitor_create_path_for_hierarchy(ops->hierarchies[i], monitor_cgroup)) { ERROR("Failed to create cgroup \"%s\"", ops->hierarchies[i]->monitor_full_path); free(ops->hierarchies[i]->container_full_path); @@ -1372,7 +1372,7 @@ __cgfsng_ops static inline bool cgfsng_monitor_create(struct cgroup_ops *ops, break; } } - } while (idx > 0 && idx < 1000); + } while (ops->hierarchies[i] && idx > 0 && idx < 1000); if (idx < 1000) bret = true;