summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/basic/fs-util.c46
-rw-r--r--src/basic/fs-util.h1
-rw-r--r--src/sysusers/sysusers.c45
-rw-r--r--src/test/test-fs-util.c45
4 files changed, 21 insertions, 116 deletions
diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index 13e7b1b163..c20a29332a 100644
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -280,52 +280,6 @@ int fchmod_and_chown(int fd, mode_t mode, uid_t uid, gid_t gid) {
return do_chown || do_chmod;
}
-int chmod_and_chown_unsafe(const char *path, mode_t mode, uid_t uid, gid_t gid) {
- bool do_chown, do_chmod;
- struct stat st;
-
- assert(path);
-
- /* Change ownership and access mode of the specified path, see description of fchmod_and_chown().
- * Should only be used on trusted paths. */
-
- if (lstat(path, &st) < 0)
- return -errno;
-
- do_chown =
- (uid != UID_INVALID && st.st_uid != uid) ||
- (gid != GID_INVALID && st.st_gid != gid);
-
- do_chmod =
- !S_ISLNK(st.st_mode) && /* chmod is not defined on symlinks */
- ((mode != MODE_INVALID && ((st.st_mode ^ mode) & 07777) != 0) ||
- do_chown); /* If we change ownership, make sure we reset the mode afterwards, since chown()
- * modifies the access mode too */
-
- if (mode == MODE_INVALID)
- mode = st.st_mode; /* If we only shall do a chown(), save original mode, since chown() might break it. */
- else if ((mode & S_IFMT) != 0 && ((mode ^ st.st_mode) & S_IFMT) != 0)
- return -EINVAL; /* insist on the right file type if it was specified */
-
- 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 (chmod(path, minimal & 07777) < 0)
- return -errno;
- }
-
- if (do_chown)
- if (lchown(path, uid, gid) < 0)
- return -errno;
-
- if (do_chmod)
- if (chmod(path, mode & 07777) < 0)
- return -errno;
-
- return do_chown || do_chmod;
-}
-
int fchmod_umask(int fd, mode_t m) {
mode_t u;
int r;
diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h
index b184570f9f..eb6e1eee4f 100644
--- a/src/basic/fs-util.h
+++ b/src/basic/fs-util.h
@@ -34,7 +34,6 @@ int readlink_and_make_absolute(const char *p, char **r);
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);
-int chmod_and_chown_unsafe(const char *path, mode_t mode, uid_t uid, gid_t gid);
int fchmod_umask(int fd, mode_t mode);
int fchmod_opath(int fd, mode_t m);
diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c
index cdfceb2533..7349e9fcb9 100644
--- a/src/sysusers/sysusers.c
+++ b/src/sysusers/sysusers.c
@@ -201,14 +201,16 @@ static int load_group_database(void) {
}
static int make_backup(const char *target, const char *x) {
- _cleanup_close_ int src = -1;
+ _cleanup_(unlink_and_freep) char *dst_tmp = NULL;
_cleanup_fclose_ FILE *dst = NULL;
- _cleanup_free_ char *dst_tmp = NULL;
- char *backup;
- struct timespec ts[2];
+ _cleanup_close_ int src = -1;
+ const char *backup;
struct stat st;
int r;
+ assert(target);
+ assert(x);
+
src = open(x, O_RDONLY|O_CLOEXEC|O_NOCTTY);
if (src < 0) {
if (errno == ENOENT) /* No backup necessary... */
@@ -220,43 +222,38 @@ static int make_backup(const char *target, const char *x) {
if (fstat(src, &st) < 0)
return -errno;
- r = fopen_temporary_label(target, x, &dst, &dst_tmp);
+ r = fopen_temporary_label(
+ target, /* The path for which to the lookup the label */
+ x, /* Where we want the file actually to end up */
+ &dst,
+ &dst_tmp /* The temporary file we write to */);
if (r < 0)
return r;
r = copy_bytes(src, fileno(dst), (uint64_t) -1, COPY_REFLINK);
if (r < 0)
- goto fail;
-
- /* Don't fail on chmod() or chown(). If it stays owned by us
- * and/or unreadable by others, then it isn't too bad... */
+ return r;
backup = strjoina(x, "-");
- /* Copy over the access mask */
- r = chmod_and_chown_unsafe(dst_tmp, st.st_mode & 07777, st.st_uid, st.st_gid);
+ /* Copy over the access mask. Don't fail on chmod() or chown(). If it stays owned by us and/or
+ * unreadable by others, then it isn't too bad... */
+ r = fchmod_and_chown(fileno(dst), st.st_mode & 07777, st.st_uid, st.st_gid);
if (r < 0)
log_warning_errno(r, "Failed to change access mode or ownership of %s: %m", backup);
- ts[0] = st.st_atim;
- ts[1] = st.st_mtim;
- if (futimens(fileno(dst), ts) < 0)
+ if (futimens(fileno(dst), (const struct timespec[2]) { st.st_atim, st.st_mtim }) < 0)
log_warning_errno(errno, "Failed to fix access and modification time of %s: %m", backup);
- r = fflush_sync_and_check(dst);
+ r = fsync_full(fileno(dst));
if (r < 0)
- goto fail;
+ return r;
- if (rename(dst_tmp, backup) < 0) {
- r = -errno;
- goto fail;
- }
+ if (rename(dst_tmp, backup) < 0)
+ return errno;
+ dst_tmp = mfree(dst_tmp); /* disable the unlink_and_freep() hook now that the file has been renamed*/
return 0;
-
-fail:
- (void) unlink(dst_tmp);
- return r;
}
static int putgrent_with_members(const struct group *gr, FILE *group) {
diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c
index f2df2e35e6..f63b1f5d5f 100644
--- a/src/test/test-fs-util.c
+++ b/src/test/test-fs-util.c
@@ -802,50 +802,6 @@ static void test_chmod_and_chown(void) {
assert_se(S_ISLNK(st.st_mode));
}
-static void test_chmod_and_chown_unsafe(void) {
- _cleanup_(rm_rf_physical_and_freep) char *d = NULL;
- _unused_ _cleanup_umask_ mode_t u = umask(0000);
- struct stat st;
- const char *p;
-
- if (geteuid() != 0)
- return;
-
- log_info("/* %s */", __func__);
-
- assert_se(mkdtemp_malloc(NULL, &d) >= 0);
-
- p = strjoina(d, "/reg");
- assert_se(mknod(p, S_IFREG | 0123, 0) >= 0);
-
- assert_se(chmod_and_chown_unsafe(p, S_IFREG | 0321, 1, 2) >= 0);
- assert_se(chmod_and_chown_unsafe(p, S_IFDIR | 0555, 3, 4) == -EINVAL);
-
- assert_se(lstat(p, &st) >= 0);
- assert_se(S_ISREG(st.st_mode));
- assert_se((st.st_mode & 07777) == 0321);
-
- p = strjoina(d, "/dir");
- assert_se(mkdir(p, 0123) >= 0);
-
- assert_se(chmod_and_chown_unsafe(p, S_IFDIR | 0321, 1, 2) >= 0);
- assert_se(chmod_and_chown_unsafe(p, S_IFREG | 0555, 3, 4) == -EINVAL);
-
- assert_se(lstat(p, &st) >= 0);
- assert_se(S_ISDIR(st.st_mode));
- assert_se((st.st_mode & 07777) == 0321);
-
- p = strjoina(d, "/lnk");
- assert_se(symlink("idontexist", p) >= 0);
-
- assert_se(chmod_and_chown_unsafe(p, S_IFLNK | 0321, 1, 2) >= 0);
- assert_se(chmod_and_chown_unsafe(p, S_IFREG | 0555, 3, 4) == -EINVAL);
- assert_se(chmod_and_chown_unsafe(p, S_IFDIR | 0555, 3, 4) == -EINVAL);
-
- assert_se(lstat(p, &st) >= 0);
- assert_se(S_ISLNK(st.st_mode));
-}
-
static void test_path_is_encrypted_one(const char *p, int expect) {
int r;
@@ -895,7 +851,6 @@ int main(int argc, char *argv[]) {
test_fsync_directory_of_file();
test_rename_noreplace();
test_chmod_and_chown();
- test_chmod_and_chown_unsafe();
test_path_is_encrypted();
return 0;