From: Zbigniew Jędrzejewski-Szmek Date: Thu, 22 Mar 2018 12:03:41 +0000 (+0100) Subject: tree-wide: warn when a directory path already exists but has bad mode/owner/type X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=13102410a7e1867affc236922b26e71269817ab8;p=elogind.git tree-wide: warn when a directory path already exists but has bad mode/owner/type When we are attempting to create directory somewhere in the bowels of /var/lib and get an error that it already exists, it can be quite hard to diagnose what is wrong (especially for a user who is not aware that the directory must have the specified owner, and permissions not looser than what was requested). Let's print a warning in most cases. A warning is appropriate, because such state is usually a sign of borked installation and needs to be resolved by the adminstrator. $ build/test-fs-util Path "/tmp/test-readlink_and_make_absolute" already exists and is not a directory, refusing. (or) Directory "/tmp/test-readlink_and_make_absolute" already exists, but has mode 0775 that is too permissive (0755 was requested), refusing. (or) Directory "/tmp/test-readlink_and_make_absolute" already exists, but is owned by 1001:1000 (1000:1000 was requested), refusing. Assertion 'mkdir_safe(tempdir, 0755, getuid(), getgid(), MKDIR_WARN_MODE) >= 0' failed at ../src/test/test-fs-util.c:320, function test_readlink_and_make_absolute(). Aborting. No functional change except for the new log lines. --- diff --git a/src/basic/mkdir.c b/src/basic/mkdir.c index 139318523..ac127b9dc 100644 --- a/src/basic/mkdir.c +++ b/src/basic/mkdir.c @@ -29,6 +29,7 @@ #include "mkdir.h" #include "path-util.h" #include "stat-util.h" +//#include "stdio-util.h" #include "user-util.h" /// Additional includes needed by elogind @@ -65,13 +66,32 @@ int mkdir_safe_internal(const char *path, mode_t mode, uid_t uid, gid_t gid, Mkd return -errno; } + if (!S_ISDIR(st.st_mode)) { + log_full(flags & MKDIR_WARN_MODE ? LOG_WARNING : LOG_DEBUG, + "Path \"%s\" already exists and is not a directory, refusing.", path); + return -ENOTDIR; + } if ((st.st_mode & 0007) > (mode & 0007) || (st.st_mode & 0070) > (mode & 0070) || - (st.st_mode & 0700) > (mode & 0700) || - (uid != UID_INVALID && st.st_uid != uid) || - (gid != GID_INVALID && st.st_gid != gid) || - !S_ISDIR(st.st_mode)) + (st.st_mode & 0700) > (mode & 0700)) { + log_full(flags & MKDIR_WARN_MODE ? LOG_WARNING : LOG_DEBUG, + "Directory \"%s\" already exists, but has mode %04o that is too permissive (%04o was requested), refusing.", + path, st.st_mode & 0777, mode); return -EEXIST; + } + if ((uid != UID_INVALID && st.st_uid != uid) || + (gid != GID_INVALID && st.st_gid != gid)) { + char u[DECIMAL_STR_MAX(uid_t)] = "-", g[DECIMAL_STR_MAX(gid_t)] = "-"; + + if (uid != UID_INVALID) + xsprintf(u, UID_FMT, uid); + if (gid != UID_INVALID) + xsprintf(g, GID_FMT, gid); + log_full(flags & MKDIR_WARN_MODE ? LOG_WARNING : LOG_DEBUG, + "Directory \"%s\" already exists, but is owned by "UID_FMT":"GID_FMT" (%s:%s was requested), refusing.", + path, st.st_uid, st.st_gid, u, g); + return -EEXIST; + } return 0; } diff --git a/src/basic/mkdir.h b/src/basic/mkdir.h index 0f089bc72..07d271980 100644 --- a/src/basic/mkdir.h +++ b/src/basic/mkdir.h @@ -25,6 +25,7 @@ typedef enum MkdirFlags { MKDIR_FOLLOW_SYMLINK = 1 << 0, + MKDIR_WARN_MODE = 1 << 1, } MkdirFlags; int mkdir_errno_wrapper(const char *pathname, mode_t mode); diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 3da51259e..165274461 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -1239,7 +1239,7 @@ static int method_set_user_linger(sd_bus_message *message, void *userdata, sd_bu mkdir_p_label("/var/lib/elogind", 0755); - r = mkdir_safe_label("/var/lib/elogind/linger", 0755, 0, 0, 0); + r = mkdir_safe_label("/var/lib/elogind/linger", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) return r; @@ -2084,7 +2084,7 @@ static int update_schedule_file(Manager *m) { assert(m); - r = mkdir_safe_label("/run/systemd/shutdown", 0755, 0, 0, 0); + r = mkdir_safe_label("/run/systemd/shutdown", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) return log_error_errno(r, "Failed to create shutdown subdirectory: %m"); diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c index 0c84e5ed9..f814376e8 100644 --- a/src/login/logind-inhibit.c +++ b/src/login/logind-inhibit.c @@ -87,7 +87,7 @@ int inhibitor_save(Inhibitor *i) { assert(i); - r = mkdir_safe_label("/run/systemd/inhibit", 0755, 0, 0, 0); + r = mkdir_safe_label("/run/systemd/inhibit", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) goto fail; @@ -291,7 +291,7 @@ int inhibitor_create_fifo(Inhibitor *i) { /* Create FIFO */ if (!i->fifo_path) { - r = mkdir_safe_label("/run/systemd/inhibit", 0755, 0, 0, 0); + r = mkdir_safe_label("/run/systemd/inhibit", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) return r; diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 5cd9126b9..0b7a1adab 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -95,7 +95,7 @@ int seat_save(Seat *s) { if (!s->started) return 0; - r = mkdir_safe_label("/run/systemd/seats", 0755, 0, 0, 0); + r = mkdir_safe_label("/run/systemd/seats", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) goto fail; diff --git a/src/login/logind-session.c b/src/login/logind-session.c index fdf22f315..612feaefa 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -185,7 +185,7 @@ int session_save(Session *s) { if (!s->started) return 0; - r = mkdir_safe_label("/run/systemd/sessions", 0755, 0, 0, 0); + r = mkdir_safe_label("/run/systemd/sessions", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) goto fail; @@ -993,6 +993,7 @@ int session_create_fifo(Session *s) { if (!s->fifo_path) { r = mkdir_safe_label("/run/systemd/sessions", 0755, 0, 0, false); r = mkdir_safe_label("/run/systemd/sessions", 0755, 0, 0, 0); + r = mkdir_safe_label("/run/systemd/sessions", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) return r; diff --git a/src/login/logind-user.c b/src/login/logind-user.c index daa4ab6a7..155ec6a5a 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -145,7 +145,7 @@ static int user_save_internal(User *u) { assert(u); assert(u->state_file); - r = mkdir_safe_label("/run/systemd/users", 0755, 0, 0, 0); + r = mkdir_safe_label("/run/systemd/users", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) goto fail; @@ -343,7 +343,7 @@ static int user_mkdir_runtime_path(User *u) { assert(u); - r = mkdir_safe_label("/run/user", 0755, 0, 0, 0); + r = mkdir_safe_label("/run/user", 0755, 0, 0, MKDIR_WARN_MODE); if (r < 0) return log_error_errno(r, "Failed to create /run/user: %m"); diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index 6fd756752..ba01eea4e 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -317,7 +317,7 @@ static void test_readlink_and_make_absolute(void) { char *r = NULL; _cleanup_free_ char *pwd = NULL; - assert_se(mkdir_safe(tempdir, 0755, getuid(), getgid(), 0) >= 0); + assert_se(mkdir_safe(tempdir, 0755, getuid(), getgid(), MKDIR_WARN_MODE) >= 0); assert_se(touch(name) >= 0); assert_se(symlink(name, name_alias) >= 0);