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