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 139318523f382bd9d0e77afe6ff44fd28dc7dbcb..ac127b9dc080d92d0f019ec4986ba16ccb3a9ac6 100644 (file)
--- 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 0f089bc725819acab7c4a0971710747c05e5a70d..07d271980e1e1de58248865c8e3806c4f8472428 100644 (file)
--- 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 3da51259eeee60a6563c6a3418c9c32ab2b58930..165274461d069235f5c585927b0c94c04c3b1cd8 100644 (file)
--- 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 0c84e5ed9d5ddd8f52c58222d08e1ab0ba71a875..f814376e8034ec622e922a63e5d594943ef4aad1 100644 (file)
--- 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 5cd9126b961ed005fd7f540d6044a89016014e69..0b7a1adabceeb3a823c7907573b65c974c93f520 100644 (file)
--- 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 fdf22f3150cdeb2bd66826de8ad0eca4656ab2d6..612feaefadebb0235232a9c6f564d80352f6fe89 100644 (file)
--- 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 daa4ab6a7c564e200d18125dfdb3b52fe84f53f8..155ec6a5ae0392e508d1efe299bde5b03578711f 100644 (file)
--- 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 6fd756752d35a5a08d0f4793bf8b76837caa9d3c..ba01eea4e3bc156596760deb9abf9f9cbf8b15ef 100644 (file)
--- 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);