cgroups: fix memory leak and simplify code

Closes #3252.
Reported-by: LiFeng <lifeng68@huawei.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 4c21a6f29eeb83a42b95b8fe0933842543b7a084..3c8f653193e12465eb899bed23f2f2c977dcb093 100644 (file)
--- 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 60716b288f2c80e535f26c193a4f2f39b778263c..b3c25827a245ee82bd12a76dd0f2693dfd235843 100644 (file)
--- 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 0ea6b24066feeabf192ea973e1b8f835c5502da9..74d10a760410b7959e30798ed743cc4e2f4447f6 100644 (file)
--- 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)
 {