netdev - [PATCH iproute2 2/2] lib/fs: Fix single return points for get_cgroup2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <9b07b59c4c422b29d6c8297f7f7ec0f2dcc7fb3f.1608315719.git.aclaudi@redhat.com>
Date:   Fri, 18 Dec 2020 20:09:23 +0100
From:   Andrea Claudi <aclaudi@...hat.com>
To:     netdev@...r.kernel.org
Cc:     stephen@...workplumber.org, dsahern@...il.com
Subject: [PATCH iproute2 2/2] lib/fs: Fix single return points for get_cgroup2_*

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 <aclaudi@...hat.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