From dee00c1939c6194404c15a80650d0c04bb01b0db Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Sep 2020 20:35:33 +0200 Subject: [PATCH] fs-util,tmpfiles: fix error handling of fchmod_opath() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When 4dfaa528d45 was first commited its callers relied on `errno` instead of the return value for error reporting. Which worked fine, since internally under all conditions base were set — even if ugly and not inline with our coding style. Things then got broken in f8606626ed3c2582e06543550d58fe9886cdca5f where suddenly additional syscalls might end up being done in the function, thus corrupting `errno`. --- src/basic/fs-util.c | 17 +++++++++++------ src/tmpfiles/tmpfiles.c | 11 +++++++---- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 030789d716..7f8b8b22b3 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -229,6 +229,7 @@ int chmod_and_chown(const char *path, mode_t mode, uid_t uid, gid_t gid) { int fchmod_and_chown(int fd, mode_t mode, uid_t uid, gid_t gid) { bool do_chown, do_chmod; struct stat st; + int r; /* Change ownership and access mode of the specified fd. Tries to do so safely, ensuring that at no * point in time the access mode is above the old access mode under the old ownership or the new @@ -259,18 +260,22 @@ int fchmod_and_chown(int fd, mode_t mode, uid_t uid, gid_t gid) { if (do_chown && do_chmod) { mode_t minimal = st.st_mode & mode; /* the subset of the old and the new mask */ - if (((minimal ^ st.st_mode) & 07777) != 0) - if (fchmod_opath(fd, minimal & 07777) < 0) - return -errno; + if (((minimal ^ st.st_mode) & 07777) != 0) { + r = fchmod_opath(fd, minimal & 07777); + if (r < 0) + return r; + } } if (do_chown) if (fchownat(fd, "", uid, gid, AT_EMPTY_PATH) < 0) return -errno; - if (do_chmod) - if (fchmod_opath(fd, mode & 07777) < 0) - return -errno; + if (do_chmod) { + r = fchmod_opath(fd, mode & 07777); + if (r < 0) + return r; + } return do_chown || do_chmod; } diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 5a0a5a5867..61571a45ee 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -836,6 +836,7 @@ static int fd_set_perms(Item *i, int fd, const char *path, const struct stat *st struct stat stbuf; mode_t new_mode; bool do_chown; + int r; assert(i); assert(fd); @@ -881,8 +882,9 @@ static int fd_set_perms(Item *i, int fd, const char *path, const struct stat *st log_debug("\"%s\" matches temporary mode %o already.", path, m); else { log_debug("Temporarily changing \"%s\" to mode %o.", path, m); - if (fchmod_opath(fd, m) < 0) - return log_error_errno(errno, "fchmod() of %s failed: %m", path); + r = fchmod_opath(fd, m); + if (r < 0) + return log_error_errno(r, "fchmod() of %s failed: %m", path); } } } @@ -913,8 +915,9 @@ static int fd_set_perms(Item *i, int fd, const char *path, const struct stat *st log_debug("\"%s\" matches mode %o already.", path, new_mode); else { log_debug("Changing \"%s\" to mode %o.", path, new_mode); - if (fchmod_opath(fd, new_mode) < 0) - return log_error_errno(errno, "fchmod() of %s failed: %m", path); + r = fchmod_opath(fd, new_mode); + if (r < 0) + return log_error_errno(r, "fchmod() of %s failed: %m", path); } } }