From: Christian Brauner Date: Tue, 10 Mar 2020 13:44:59 +0000 (+0100) Subject: cgroups: fix memory leak and simplify code X-Git-Tag: lxc-4.0.0~41^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bbba37f7b4cc64e24520d8390ec1ebc46a441714;p=thirdparty%2Flxc.git cgroups: fix memory leak and simplify code Closes #3252. Reported-by: LiFeng Signed-off-by: Christian Brauner --- diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 4c21a6f29..3c8f65319 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -54,19 +54,6 @@ lxc_log_define(cgfsng, cgroup); -static void free_string_list(char **clist) -{ - int i; - - if (!clist) - return; - - for (i = 0; clist[i]; i++) - free(clist[i]); - - free(clist); -} - /* Given a pointer to a null-terminated array of pointers, realloc to add one * entry, and point the new entry to NULL. Do not fail. Return the index to the * second-to-last entry - that is, the one which is now available for use @@ -2940,12 +2927,11 @@ static void cg_unified_delegate(char ***delegate) */ static int cg_hybrid_init(struct cgroup_ops *ops, bool relative, bool unprivileged) { - __do_free char *basecginfo = NULL; - __do_free char *line = NULL; + __do_free char *basecginfo = NULL, *line = NULL; + __do_free_string_list char **klist = NULL, **nlist = NULL; __do_fclose FILE *f = NULL; int ret; size_t len = 0; - char **klist = NULL, **nlist = NULL; /* Root spawned containers escape the current cgroup, so use init's * cgroups as our base in that case. @@ -2968,11 +2954,11 @@ static int cg_hybrid_init(struct cgroup_ops *ops, bool relative, bool unprivileg lxc_cgfsng_print_basecg_debuginfo(basecginfo, klist, nlist); while (getline(&line, &len, f) != -1) { + __do_free char *base_cgroup = NULL, *mountpoint = NULL; + __do_free_string_list char **controller_list = NULL; int type; bool writeable; struct hierarchy *new; - char *base_cgroup = NULL, *mountpoint = NULL; - char **controller_list = NULL; type = get_cgroup_version(line); if (type == 0) @@ -3000,18 +2986,18 @@ static int cg_hybrid_init(struct cgroup_ops *ops, bool relative, bool unprivileg if (type == CGROUP_SUPER_MAGIC) if (controller_list_is_dup(ops->hierarchies, controller_list)) - log_trace_errno(goto next, EEXIST, "Skipping duplicating controller"); + log_trace_errno(continue, EEXIST, "Skipping duplicating controller"); mountpoint = cg_hybrid_get_mountpoint(line); if (!mountpoint) - log_error_errno(goto next, EINVAL, "Failed parsing mountpoint from \"%s\"", line); + log_error_errno(continue, EINVAL, "Failed parsing mountpoint from \"%s\"", line); if (type == CGROUP_SUPER_MAGIC) base_cgroup = cg_hybrid_get_current_cgroup(basecginfo, controller_list[0], CGROUP_SUPER_MAGIC); else base_cgroup = cg_hybrid_get_current_cgroup(basecginfo, NULL, CGROUP2_SUPER_MAGIC); if (!base_cgroup) - log_error_errno(goto next, EINVAL, "Failed to find current cgroup"); + log_error_errno(continue, EINVAL, "Failed to find current cgroup"); trim(base_cgroup); prune_init_scope(base_cgroup); @@ -3020,7 +3006,7 @@ static int cg_hybrid_init(struct cgroup_ops *ops, bool relative, bool unprivileg else writeable = test_writeable_v1(mountpoint, base_cgroup); if (!writeable) - log_trace_errno(goto next, EROFS, "The %s group is not writeable", base_cgroup); + log_trace_errno(continue, EROFS, "The %s group is not writeable", base_cgroup); if (type == CGROUP2_SUPER_MAGIC) { char *cgv2_ctrl_path; @@ -3040,26 +3026,16 @@ static int cg_hybrid_init(struct cgroup_ops *ops, bool relative, bool unprivileg /* Exclude all controllers that cgroup use does not want. */ if (!cgroup_use_wants_controllers(ops, controller_list)) - log_trace_errno(goto next, EINVAL, "Skipping controller"); + log_trace_errno(continue, EINVAL, "Skipping controller"); - new = add_hierarchy(&ops->hierarchies, controller_list, mountpoint, base_cgroup, type); + new = add_hierarchy(&ops->hierarchies, move_ptr(controller_list), move_ptr(mountpoint), move_ptr(base_cgroup), type); if (type == CGROUP2_SUPER_MAGIC && !ops->unified) { if (unprivileged) cg_unified_delegate(&new->cgroup2_chown); ops->unified = new; } - - continue; - - next: - free_string_list(controller_list); - free(mountpoint); - free(base_cgroup); } - free_string_list(klist); - free_string_list(nlist); - TRACE("Writable cgroup hierarchies:"); lxc_cgfsng_print_hierarchies(ops); diff --git a/src/lxc/memory_utils.h b/src/lxc/memory_utils.h index 60716b288..b3c25827a 100644 --- a/src/lxc/memory_utils.h +++ b/src/lxc/memory_utils.h @@ -13,11 +13,35 @@ #include "macro.h" +#define define_cleanup_attribute(type, func) \ + static inline void func##_ptr(type *ptr) \ + { \ + if (*ptr) \ + func(*ptr); \ + } + +#define free_disarm(ptr) \ + ({ \ + free(ptr); \ + move_ptr(ptr); \ + }) + static inline void __auto_free__(void *p) { free(*(void **)p); } +static inline void free_string_list(char **list) +{ + if (list) { + for (int i = 0; list[i]; i++) + free(list[i]); + free_disarm(list); + } +} +define_cleanup_attribute(char **, free_string_list); +#define __do_free_string_list __attribute__((__cleanup__(free_string_list_ptr))) + static inline void __auto_fclose__(FILE **f) { if (*f) @@ -38,12 +62,6 @@ static inline void __auto_closedir__(DIR **d) fd = -EBADF; \ } -#define free_disarm(ptr) \ - ({ \ - free(ptr); \ - move_ptr(ptr); \ - }) - static inline void __auto_close__(int *fd) { close_prot_errno_disarm(*fd); diff --git a/src/lxc/pam/pam_cgfs.c b/src/lxc/pam/pam_cgfs.c index 0ea6b2406..74d10a760 100644 --- a/src/lxc/pam/pam_cgfs.c +++ b/src/lxc/pam/pam_cgfs.c @@ -77,7 +77,6 @@ static inline void clear_bit(unsigned bit, uint32_t *bitarr) bitarr[bit / NBITS] &= ~(1 << (bit % NBITS)); } static char *copy_to_eol(char *s); -static void free_string_list(char **list); static char *get_mountpoint(char *line); static bool get_uid_gid(const char *user, uid_t *uid, gid_t *gid); static int handle_login(const char *user, uid_t uid, gid_t gid); @@ -454,16 +453,6 @@ static size_t string_list_length(char **list) return len; } -/* Free null-terminated array of strings. */ -static void free_string_list(char **list) -{ - char **it; - - for (it = list; it && *it; it++) - free(*it); - free(list); -} - /* Write single integer to file. */ static bool write_int(char *path, int v) {