[PATCH iproute2 v2 0/2] Some fixes to lib/fs.c

[PATCH iproute2 v2 0/2] Some fixes to lib/fs.c

[Andrea Claudi]

This series contains a couple of fixes and improvements on lib/fs.c
- in functions get_cgroup2_id() and get_cgroup2_path(), fixes cleanup on
  single return point;
- in function make_path(), avoid to call mkdir() two times in a row.

v1->v2:
- on 1/2, no code changes, add fixes tag suggested by Phil Sutter and
  his Acked-by;
- on 2/2, simplify changes using conditional close() calls, as suggested
  by Phil Sutter.

Andrea Claudi (2):
  lib/fs: avoid double call to mkdir on make_path()
  lib/fs: Fix single return points for get_cgroup2_*

 lib/fs.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

-- 
2.29.2

[PATCH iproute2 v2 1/2] lib/fs: avoid double call to mkdir on make_path()

[Andrea Claudi]

make_path() function calls mkdir two times in a row. The first one it
stores mkdir return code, and then it calls it again to check for errno.

This seems unnecessary, as we can use the return code from the first
call and check for errno if not 0.

Fixes: ac3415f5c1b1d ("lib/fs: Fix and simplify make_path()")
Acked-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
 lib/fs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/fs.c b/lib/fs.c
index 4b90a704..2ae506ec 100644
--- a/lib/fs.c
+++ b/lib/fs.c
@@ -253,7 +253,7 @@ int make_path(const char *path, mode_t mode)
 			*delim = '\0';
 
 		rc = mkdir(dir, mode);
-		if (mkdir(dir, mode) != 0 && errno != EEXIST) {
+		if (rc && errno != EEXIST) {
 			fprintf(stderr, "mkdir failed for %s: %s\n",
 				dir, strerror(errno));
 			goto out;
-- 
2.29.2

[PATCH iproute2 v2 2/2] lib/fs: Fix single return points for get_cgroup2_*

[Andrea Claudi]

Functions get_cgroup2_id() and get_cgroup2_path() may call close() with
a negative argument.
Avoid that making the calls conditional on the file descriptors.

get_cgroup2_path() may also return NULL leaking a file descriptor.
Ensure this does not happen using a single return point.

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@redhat.com>
---
 lib/fs.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/lib/fs.c b/lib/fs.c
index 2ae506ec..ee0b130b 100644
--- a/lib/fs.c
+++ b/lib/fs.c
@@ -157,7 +157,8 @@ __u64 get_cgroup2_id(const char *path)
 	memcpy(cg_id.bytes, fhp->f_handle, sizeof(__u64));
 
 out:
-	close(mnt_fd);
+	if (mnt_fd >= 0)
+		close(mnt_fd);
 	free(mnt);
 
 	return cg_id.id;
@@ -179,16 +180,16 @@ char *get_cgroup2_path(__u64 id, bool full)
 	char *path = NULL;
 	char fd_path[64];
 	int link_len;
-	char *mnt;
+	char *mnt = NULL;
 
 	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) {
@@ -225,8 +226,10 @@ char *get_cgroup2_path(__u64 id, bool full)
 			"Failed to allocate memory for cgroup2 path\n");
 
 out:
-	close(fd);
-	close(mnt_fd);
+	if (fd >= 0)
+		close(fd);
+	if (mnt_fd >= 0)
+		close(mnt_fd);
 	free(mnt);
 
 	return path;
-- 
2.29.2