Functions get_cgroup2_id() and get_cgroup2_path() uncorrectly performs cleanup on the single return point. Both of them may get to use close() with a negative argument, if open() fails.
Fix this adding proper labels and gotos to make sure we clean up only resources we are effectively used before. Fixes: d5e6ee0dac64 ("ss: introduce cgroup2 cache and helper functions") Fixes: 8f1cd119b377 ("lib: fix checking of returned file handle size for cgroup") Signed-off-by: Andrea Claudi <acla...@redhat.com> --- lib/fs.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/fs.c b/lib/fs.c index 2ae506ec..77414e99 100644 --- a/lib/fs.c +++ b/lib/fs.c @@ -139,27 +139,31 @@ __u64 get_cgroup2_id(const char *path) mnt_fd = open(mnt, O_RDONLY); if (mnt_fd < 0) { fprintf(stderr, "Failed to open cgroup2 mount\n"); - goto out; + goto out_clean_mnt; } fhp->handle_bytes = sizeof(__u64); if (name_to_handle_at(mnt_fd, path, fhp, &mnt_id, 0) < 0) { fprintf(stderr, "Failed to get cgroup2 ID: %s\n", strerror(errno)); - goto out; + goto out_clean_all; } } if (fhp->handle_bytes != sizeof(__u64)) { fprintf(stderr, "Invalid size of cgroup2 ID\n"); - goto out; + if (mnt_fd < 0) + goto out; + goto out_clean_all; } memcpy(cg_id.bytes, fhp->f_handle, sizeof(__u64)); -out: +out_clean_all: close(mnt_fd); +out_clean_mnt: free(mnt); +out: return cg_id.id; } @@ -183,17 +187,17 @@ char *get_cgroup2_path(__u64 id, bool full) if (!id) { fprintf(stderr, "Invalid cgroup2 ID\n"); - return NULL; + goto out; } mnt = find_cgroup2_mount(false); if (!mnt) - return NULL; + goto out; mnt_fd = open(mnt, O_RDONLY); if (mnt_fd < 0) { fprintf(stderr, "Failed to open cgroup2 mount\n"); - goto out; + goto out_clean_mnt; } fhp->handle_bytes = sizeof(__u64); @@ -203,7 +207,7 @@ char *get_cgroup2_path(__u64 id, bool full) fd = open_by_handle_at(mnt_fd, fhp, 0); if (fd < 0) { fprintf(stderr, "Failed to open cgroup2 by ID\n"); - goto out; + goto out_clean_mntfd; } snprintf(fd_path, sizeof(fd_path), "/proc/self/fd/%d", fd); @@ -212,7 +216,7 @@ char *get_cgroup2_path(__u64 id, bool full) fprintf(stderr, "Failed to read value of symbolic link %s\n", fd_path); - goto out; + goto out_clean_all; } link_buf[link_len] = '\0'; @@ -224,11 +228,14 @@ char *get_cgroup2_path(__u64 id, bool full) fprintf(stderr, "Failed to allocate memory for cgroup2 path\n"); -out: +out_clean_all: close(fd); +out_clean_mntfd: close(mnt_fd); +out_clean_mnt: free(mnt); +out: return path; } -- 2.29.2