From: Daan De Meyer Date: Wed, 24 Jul 2024 08:41:24 +0000 (+0200) Subject: fs-util: Handle dangling symlinks in openat_report_new() X-Git-Tag: v257-rc1~817^2~7 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0dd82dab91eaac5e7b17bd5e9a1e07c6d2b78dca;p=thirdparty%2Fsystemd.git fs-util: Handle dangling symlinks in openat_report_new() openat() will always resolve symlinks, except if O_NOFOLLOW is passed or O_CREAT|O_EXCL is passed. This means that if a dangling symlink is passed to openat_report_new(), the first call to openat() will always fail with ENOENT and the second call to openat() will always fail with EEXIST. Let's catch this case explicitly and fallback to creating the file with just O_CREAT and assume we're the ones that created the file. We can't resolve the symlink with chase() because this function is itself called by chase() so we could end up in weird recursive calls if we'd try to do so. --- diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 92ceb3b352e..c3d2b6e052a 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -1114,8 +1114,27 @@ int openat_report_new(int dirfd, const char *pathname, int flags, mode_t mode, b if (errno != EEXIST) return -errno; - /* Hmm, so now we got EEXIST? So it apparently exists now? If so, let's try to open again - * without the two flags. But let's not spin forever, hence put a limit on things */ + /* Hmm, so now we got EEXIST? This can indicate two things. First, if the path points to a + * dangling symlink, the first openat() will fail with ENOENT because the symlink is resolved + * and the second openat() will fail with EEXIST because symlinks are not followed when + * O_CREAT|O_EXCL is specified. Let's check for this explicitly and fall back to opening with + * just O_CREAT and assume we're the ones that created the file. */ + + struct stat st; + if (fstatat(dirfd, pathname, &st, AT_SYMLINK_NOFOLLOW) < 0) + return -errno; + + if (S_ISLNK(st.st_mode)) { + fd = openat(dirfd, pathname, flags | O_CREAT, mode); + if (fd < 0) + return -errno; + + *ret_newly_created = true; + return fd; + } + + /* If we're not operating on a symlink, someone might have created the file between the first + * and second call to openat(). Let's try again but with a limit so we don't spin forever. */ if (--attempts == 0) /* Give up eventually, somebody is playing with us */ return -EEXIST; diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index f2fa51f55fa..8139af83ce6 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -667,6 +667,17 @@ TEST(openat_report_new) { ASSERT_OK(fd); fd = safe_close(fd); ASSERT_TRUE(b); + + ASSERT_OK_ERRNO(symlinkat("target", tfd, "link")); + fd = openat_report_new(tfd, "link", O_RDWR|O_CREAT, 0666, &b); + ASSERT_OK(fd); + fd = safe_close(fd); + ASSERT_TRUE(b); + + fd = openat_report_new(tfd, "link", O_RDWR|O_CREAT, 0666, &b); + ASSERT_OK(fd); + fd = safe_close(fd); + ASSERT_FALSE(b); } TEST(xopenat_full) {