There are fd leaks in lxc_cgroup_load_meta2() in particular in the success
case. This change attempts to ensure resources are free'd/close'd, but it
is possible there are still some error cases where leaks occur.

Signed-off-by: Dwight Engen <dwight.en...@oracle.com>
---
Hi Christian,

These changes fix problems I was seeing with valgrind, but they may not be
how you would do it, so feel free to rework them if I've messed up your
code too bad :) The command I was using:
 valgrind --leak-check=full --track-fds=yes ./lxc-test-concurrent -i 2 -j 1

 src/lxc/cgroup.c | 83 ++++++++++++++++++++++++++++++++------------------------
 1 file changed, 48 insertions(+), 35 deletions(-)

diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index 9417e77..101998b 100644
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -124,7 +124,7 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char 
**subsystem_whitelist)
        size_t token_capacity = 0;
        char *line = NULL;
        size_t sz = 0;
-       int r, saved_errno = 0;
+       int r, ret = ENOMEM;
 
        /* if the subsystem whitelist is not specified, include all
         * hierarchies that contain kernel subsystems by default but
@@ -149,8 +149,10 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char 
**subsystem_whitelist)
 
        /* Step 1: determine all kernel subsystems */
        proc_cgroups = fopen_cloexec("/proc/cgroups", "r");
-       if (!proc_cgroups)
-               goto out_error;
+       if (!proc_cgroups) {
+               ret = errno;
+               goto out1;
+       }
 
        while (getline(&line, &sz, proc_cgroups) != -1) {
                char *tab1;
@@ -179,10 +181,10 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char 
**subsystem_whitelist)
 
                r = lxc_grow_array((void ***)&kernel_subsystems, 
&kernel_subsystems_capacity, kernel_subsystems_count + 1, 12);
                if (r < 0)
-                       goto out_error;
+                       goto out2;
                kernel_subsystems[kernel_subsystems_count] = strdup(line);
                if (!kernel_subsystems[kernel_subsystems_count])
-                       goto out_error;
+                       goto out2;
                kernel_subsystems_count++;
        }
 
@@ -198,8 +200,10 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char 
**subsystem_whitelist)
         * /proc/self is not valid, we try /proc/1/cgroup... */
        if (!proc_self_cgroup)
                proc_self_cgroup = fopen_cloexec("/proc/1/cgroup", "r");
-       if (!proc_self_cgroup)
-               goto out_error;
+       if (!proc_self_cgroup) {
+               ret = errno;
+               goto out2;
+       }
 
        while (getline(&line, &sz, proc_self_cgroup) != -1) {
                /* file format: hierarchy:subsystems:group,
@@ -234,25 +238,25 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char 
**subsystem_whitelist)
                        */
                        r = lxc_grow_array((void ***)&meta_data->hierarchies, 
&hierarchy_capacity, hierarchy_number + 1, 12);
                        if (r < 0)
-                               goto out_error;
+                               goto out3;
 
                        meta_data->maximum_hierarchy = hierarchy_number;
                }
 
                /* this shouldn't happen, we had this already */
                if (meta_data->hierarchies[hierarchy_number])
-                       goto out_error;
+                       goto out3;
 
                h = calloc(1, sizeof(struct cgroup_hierarchy));
                if (!h)
-                       goto out_error;
+                       goto out3;
 
                meta_data->hierarchies[hierarchy_number] = h;
 
                h->index = hierarchy_number;
                h->subsystems = lxc_string_split_and_trim(colon1, ',');
                if (!h->subsystems)
-                       goto out_error;
+                       goto out3;
                /* see if this hierarchy should be considered */
                if (!all_kernel_subsystems || !all_named_subsystems) {
                        for (p = h->subsystems; *p; p++) {
@@ -283,8 +287,10 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char 
**subsystem_whitelist)
         * /proc/self is not valid, we try /proc/1/cgroup... */
        if (!proc_self_mountinfo)
                proc_self_mountinfo = fopen_cloexec("/proc/1/mountinfo", "r");
-       if (!proc_self_mountinfo)
-               goto out_error;
+       if (!proc_self_mountinfo) {
+               ret = errno;
+               goto out3;
+       }
 
        while (getline(&line, &sz, proc_self_mountinfo) != -1) {
                char *token, *saveptr = NULL;
@@ -299,7 +305,7 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char 
**subsystem_whitelist)
                for (i = 0; (token = strtok_r(line, " ", &saveptr)); line = 
NULL) {
                        r = lxc_grow_array((void ***)&tokens, &token_capacity, 
i + 1, 64);
                        if (r < 0)
-                               goto out_error;
+                               goto out4;
                        tokens[i++] = token;
                }
 
@@ -335,7 +341,7 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char 
**subsystem_whitelist)
 
                subsystems = subsystems_from_mount_options(tokens[j + 3], 
kernel_subsystems);
                if (!subsystems)
-                       goto out_error;
+                       goto out4;
 
                h = NULL;
                for (k = 1; k <= meta_data->maximum_hierarchy; k++) {
@@ -352,12 +358,12 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char 
**subsystem_whitelist)
 
                r = lxc_grow_array((void ***)&meta_data->mount_points, 
&mount_point_capacity, mount_point_count + 1, 12);
                if (r < 0)
-                       goto out_error;
+                       goto out4;
 
                /* create mount point object */
                mount_point = calloc(1, sizeof(*mount_point));
                if (!mount_point)
-                       goto out_error;
+                       goto out4;
 
                meta_data->mount_points[mount_point_count++] = mount_point;
 
@@ -365,7 +371,7 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char 
**subsystem_whitelist)
                mount_point->mount_point = strdup(tokens[4]);
                mount_point->mount_prefix = strdup(tokens[3]);
                if (!mount_point->mount_point || !mount_point->mount_prefix)
-                       goto out_error;
+                       goto out4;
                mount_point->read_only = !lxc_string_in_list("rw", tokens[5], 
',');
 
                if (!strcmp(mount_point->mount_prefix, "/")) {
@@ -381,32 +387,39 @@ struct cgroup_meta_data *lxc_cgroup_load_meta2(const char 
**subsystem_whitelist)
                k = lxc_array_len((void **)h->all_mount_points);
                r = lxc_grow_array((void ***)&h->all_mount_points, 
&h->all_mount_point_capacity, k + 1, 4);
                if (r < 0)
-                       goto out_error;
+                       goto out4;
                h->all_mount_points[k] = mount_point;
        }
 
        /* oops, we couldn't find anything */
        if (!meta_data->hierarchies || !meta_data->mount_points) {
-               errno = EINVAL;
-               goto out_error;
+               ret = EINVAL;
+               goto out4;
        }
 
-       return meta_data;
-
-out_error:
-       saved_errno = errno;
-       if (proc_cgroups)
-               fclose(proc_cgroups);
-       if (proc_self_cgroup)
-               fclose(proc_self_cgroup);
+       ret = 0;
+out4:
        if (proc_self_mountinfo)
                fclose(proc_self_mountinfo);
-       free(line);
-       free(tokens);
-       lxc_free_array((void **)kernel_subsystems, free);
-       lxc_cgroup_put_meta(meta_data);
-       errno = saved_errno;
-       return NULL;
+out3:
+       if (proc_self_cgroup)
+               fclose(proc_self_cgroup);
+out2:
+       if (proc_cgroups)
+               fclose(proc_cgroups);
+       if (line)
+               free(line);
+       if (tokens)
+               free(tokens);
+out1:
+       if (ret != 0) {
+               lxc_free_array((void **)kernel_subsystems, free);
+               lxc_cgroup_put_meta(meta_data);
+               free(meta_data);
+               meta_data = NULL;
+       }
+       errno = ret;
+       return meta_data;
 }
 
 struct cgroup_meta_data *lxc_cgroup_get_meta(struct cgroup_meta_data 
*meta_data)
-- 
1.8.1.4


------------------------------------------------------------------------------
LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13. 
http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel

Reply via email to