Skip to content

Commit 440f57f

Browse files
tools/cgxget: free temporary conversions on failure paths
AddressSanitizer reported an 8-byte leak when convert_cgroups() aborted: Direct leak of 8 byte(s) in 1 object(s) allocated from: #0 0x... in __interceptor_malloc #1 convert_cgroups (<libcgroup-source>/src/tools/cgxget.c:822) libcgroup#2 main (<libcgroup-source>/src/tools/cgxget.c:893) The helper allocates a scratch cgroup list, but on failures or ECGNOVERSIONCONVERT it returned without releasing the partially built array. Worse, when the scratch malloc failed we still treated the conversion as “success” and left the old list pointing nowhere. Return an error if the allocation fails, zero-initialise the scratch list so we can free partially populated slots safely, free every temporary cgroup before returning on error, and only after a successful conversion free the original list (including the pointer array) before swapping in the new one. With these guards in place the ASan leak report is gone. Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
1 parent b119f71 commit 440f57f

File tree

1 file changed

+16
-12
lines changed

1 file changed

+16
-12
lines changed

src/tools/cgxget.c

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -826,8 +826,11 @@ static int convert_cgroups(struct cgroup **cgrp_list[], int cgrp_list_len,
826826
int i = 0, j, ret = 0;
827827

828828
cgrp_converted_list = malloc(cgrp_list_len * sizeof(struct cgroup *));
829-
if (cgrp_converted_list == NULL)
829+
if (cgrp_converted_list == NULL) {
830+
ret = ECGOTHER;
830831
goto out;
832+
}
833+
memset(cgrp_converted_list, 0, cgrp_list_len * sizeof(struct cgroup *));
831834

832835
for (i = 0; i < cgrp_list_len; i++) {
833836
cgrp_converted_list[i] = cgroup_new_cgroup((*cgrp_list)[i]->name);
@@ -843,20 +846,21 @@ static int convert_cgroups(struct cgroup **cgrp_list[], int cgrp_list_len,
843846
}
844847

845848
out:
846-
if (ret != 0 && ret != ECGNOVERSIONCONVERT) {
847-
/* The conversion failed */
848-
for (j = 0; j < i; j++)
849-
cgroup_free(&(cgrp_converted_list[j]));
850-
free(cgrp_converted_list);
851-
} else {
852-
/*
853-
* The conversion succeeded or was unmappable.
854-
* Free the old list.
855-
*/
849+
if (ret == 0 || ret == ECGNOVERSIONCONVERT) {
850+
struct cgroup **old_list = *cgrp_list;
851+
852+
/* Conversion succeeded: drop old list and publish the new one. */
856853
for (i = 0; i < cgrp_list_len; i++)
857-
cgroup_free(&(*cgrp_list)[i]);
854+
cgroup_free(&old_list[i]);
855+
free(old_list);
858856

859857
*cgrp_list = cgrp_converted_list;
858+
} else if (cgrp_converted_list) {
859+
for (j = 0; j < cgrp_list_len; j++) {
860+
if (cgrp_converted_list[j])
861+
cgroup_free(&(cgrp_converted_list[j]));
862+
}
863+
free(cgrp_converted_list);
860864
}
861865

862866
return ret;

0 commit comments

Comments
 (0)