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

Reply via email to