From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 22 Mar 2022 12:32:38 +0000 (+0100)
Subject: fs-util: make sure openat_report_new() initializes return param also on shortcut
X-Git-Tag: v251-rc1~69
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4d5dacbef36e9e90d24f3d1b10eeda44cee6d3fe;p=thirdparty%2Fsystemd.git

fs-util: make sure openat_report_new() initializes return param also on shortcut

Our coding style dictates that return parameters should be initialized
always on success, hence do so here also in the shortcut codepath.

Issue discovered by @fbuihuu:

https://github.com/systemd/systemd/pull/22808/files/ca8503f168d0632c606110da909aba3057777395#r831911069
---

diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index 9d9f265b0f1..e7e8ee236bc 100644
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -1086,17 +1086,25 @@ int open_mkdir_at(int dirfd, const char *path, int flags, mode_t mode) {
 
 int openat_report_new(int dirfd, const char *pathname, int flags, mode_t mode, bool *ret_newly_created) {
         unsigned attempts = 7;
+        int fd;
 
         /* Just like openat(), but adds one thing: optionally returns whether we created the file anew or if
          * it already existed before. This is only relevant if O_CREAT is set without O_EXCL, and thus will
          * shortcut to openat() otherwise */
 
-        if (!FLAGS_SET(flags, O_CREAT) || FLAGS_SET(flags, O_EXCL) || !ret_newly_created)
+        if (!ret_newly_created)
                 return RET_NERRNO(openat(dirfd, pathname, flags, mode));
 
-        for (;;) {
-                int fd;
+        if (!FLAGS_SET(flags, O_CREAT) || FLAGS_SET(flags, O_EXCL)) {
+                fd = openat(dirfd, pathname, flags, mode);
+                if (fd < 0)
+                        return -errno;
 
+                *ret_newly_created = FLAGS_SET(flags, O_CREAT);
+                return fd;
+        }
+
+        for (;;) {
                 /* First, attempt to open without O_CREAT/O_EXCL, i.e. open existing file */
                 fd = openat(dirfd, pathname, flags & ~(O_CREAT | O_EXCL), mode);
                 if (fd >= 0) {
diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c
index c93723ad293..f24e97b4ccc 100644
--- a/src/test/test-fs-util.c
+++ b/src/test/test-fs-util.c
@@ -1018,6 +1018,24 @@ TEST(openat_report_new) {
         assert_se(fd >= 0);
         fd = safe_close(fd);
         assert_se(!b);
+
+        fd = openat_report_new(AT_FDCWD, j, O_RDWR, 0666, &b);
+        assert_se(fd >= 0);
+        fd = safe_close(fd);
+        assert_se(!b);
+
+        fd = openat_report_new(AT_FDCWD, j, O_RDWR|O_CREAT|O_EXCL, 0666, &b);
+        assert_se(fd == -EEXIST);
+
+        assert_se(unlink(j) >= 0);
+
+        fd = openat_report_new(AT_FDCWD, j, O_RDWR, 0666, &b);
+        assert_se(fd == -ENOENT);
+
+        fd = openat_report_new(AT_FDCWD, j, O_RDWR|O_CREAT|O_EXCL, 0666, &b);
+        assert_se(fd >= 0);
+        fd = safe_close(fd);
+        assert_se(b);
 }
 
 static int intro(void) {