Merge pull request #13385 from yuwata/core-remove-private-directories-13355

core: also remove private directories by systemctl clean

8 files changed, 142 insertions, 24 deletions

diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index eae4bd93f2..2be3db26a6 100644
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -815,6 +815,7 @@ const sd_bus_vtable bus_exec_vtable[] = {
         SD_BUS_PROPERTY("LogsDirectory", "as", NULL, offsetof(ExecContext, directories[EXEC_DIRECTORY_LOGS].paths), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("ConfigurationDirectoryMode", "u", bus_property_get_mode, offsetof(ExecContext, directories[EXEC_DIRECTORY_CONFIGURATION].mode), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("ConfigurationDirectory", "as", NULL, offsetof(ExecContext, directories[EXEC_DIRECTORY_CONFIGURATION].paths), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("TimeoutCleanUSec", "t", bus_property_get_usec, offsetof(ExecContext, timeout_clean_usec), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("MemoryDenyWriteExecute", "b", bus_property_get_bool, offsetof(ExecContext, memory_deny_write_execute), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("RestrictRealtime", "b", bus_property_get_bool, offsetof(ExecContext, restrict_realtime), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("RestrictSUIDSGID", "b", bus_property_get_bool, offsetof(ExecContext, restrict_suid_sgid), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index fbda8d8a4c..ad7471c729 100644
--- a/src/core/dbus-service.c
+++ b/src/core/dbus-service.c
@@ -101,7 +101,6 @@ const sd_bus_vtable bus_service_vtable[] = {
         SD_BUS_PROPERTY("TimeoutStartUSec", "t", bus_property_get_usec, offsetof(Service, timeout_start_usec), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("TimeoutStopUSec", "t", bus_property_get_usec, offsetof(Service, timeout_stop_usec), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("TimeoutAbortUSec", "t", property_get_timeout_abort_usec, 0, 0),
-        SD_BUS_PROPERTY("TimeoutCleanUSec", "t", bus_property_get_usec, offsetof(Service, timeout_clean_usec), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("RuntimeMaxUSec", "t", bus_property_get_usec, offsetof(Service, runtime_max_usec), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("WatchdogUSec", "t", bus_property_get_usec, offsetof(Service, watchdog_usec), SD_BUS_VTABLE_PROPERTY_CONST),
         BUS_PROPERTY_DUAL_TIMESTAMP("WatchdogTimestamp", offsetof(Service, watchdog_timestamp), 0),
diff --git a/src/core/execute.c b/src/core/execute.c
index 21127d4f70..4c90007778 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -2045,6 +2045,19 @@ static int setup_private_users(uid_t uid, gid_t gid) {
         return 0;
 }
 
+static bool exec_directory_is_private(const ExecContext *context, ExecDirectoryType type) {
+        if (!context->dynamic_user)
+                return false;
+
+        if (type == EXEC_DIRECTORY_CONFIGURATION)
+                return false;
+
+        if (type == EXEC_DIRECTORY_RUNTIME && context->runtime_directory_preserve_mode == EXEC_PRESERVE_NO)
+                return false;
+
+        return true;
+}
+
 static int setup_exec_directory(
                 const ExecContext *context,
                 const ExecParameters *params,
@@ -2091,9 +2104,7 @@ static int setup_exec_directory(
                 if (r < 0)
                         goto fail;
 
-                if (context->dynamic_user &&
-                    (!IN_SET(type, EXEC_DIRECTORY_RUNTIME, EXEC_DIRECTORY_CONFIGURATION) ||
-                     (type == EXEC_DIRECTORY_RUNTIME && context->runtime_directory_preserve_mode != EXEC_PRESERVE_NO))) {
+                if (exec_directory_is_private(context, type)) {
                         _cleanup_free_ char *private_root = NULL;
 
                         /* So, here's one extra complication when dealing with DynamicUser=1 units. In that
@@ -2369,8 +2380,7 @@ static int compile_bind_mounts(
                 if (strv_isempty(context->directories[t].paths))
                         continue;
 
-                if (context->dynamic_user &&
-                    !IN_SET(t, EXEC_DIRECTORY_RUNTIME, EXEC_DIRECTORY_CONFIGURATION) &&
+                if (exec_directory_is_private(context, t) &&
                     !(context->root_directory || context->root_image)) {
                         char *private_root;
 
@@ -2392,8 +2402,7 @@ static int compile_bind_mounts(
                 STRV_FOREACH(suffix, context->directories[t].paths) {
                         char *s, *d;
 
-                        if (context->dynamic_user &&
-                            !IN_SET(t, EXEC_DIRECTORY_RUNTIME, EXEC_DIRECTORY_CONFIGURATION))
+                        if (exec_directory_is_private(context, t))
                                 s = path_join(params->prefix[t], "private", *suffix);
                         else
                                 s = path_join(params->prefix[t], *suffix);
@@ -2402,8 +2411,7 @@ static int compile_bind_mounts(
                                 goto finish;
                         }
 
-                        if (context->dynamic_user &&
-                            !IN_SET(t, EXEC_DIRECTORY_RUNTIME, EXEC_DIRECTORY_CONFIGURATION) &&
+                        if (exec_directory_is_private(context, t) &&
                             (context->root_directory || context->root_image))
                                 /* When RootDirectory= or RootImage= are set, then the symbolic link to the private
                                  * directory is not created on the root directory. So, let's bind-mount the directory
@@ -2854,10 +2862,10 @@ static int compile_suggested_paths(const ExecContext *c, const ExecParameters *p
                 STRV_FOREACH(i, c->directories[t].paths) {
                         char *e;
 
-                        if (t == EXEC_DIRECTORY_RUNTIME)
-                                e = path_join(p->prefix[t], *i);
-                        else
+                        if (exec_directory_is_private(c, t))
                                 e = path_join(p->prefix[t], "private", *i);
+                        else
+                                e = path_join(p->prefix[t], *i);
                         if (!e)
                                 return -ENOMEM;
 
@@ -3924,6 +3932,7 @@ void exec_context_init(ExecContext *c) {
         c->personality = PERSONALITY_INVALID;
         for (i = 0; i < _EXEC_DIRECTORY_TYPE_MAX; i++)
                 c->directories[i].mode = 0755;
+        c->timeout_clean_usec = USEC_INFINITY;
         c->capability_bounding_set = CAP_ALL;
         assert_cc(NAMESPACE_FLAGS_INITIAL != NAMESPACE_FLAGS_ALL);
         c->restrict_namespaces = NAMESPACE_FLAGS_INITIAL;
@@ -4011,7 +4020,10 @@ int exec_context_destroy_runtime_directory(const ExecContext *c, const char *run
         STRV_FOREACH(i, c->directories[EXEC_DIRECTORY_RUNTIME].paths) {
                 _cleanup_free_ char *p;
 
-                p = path_join(runtime_prefix, *i);
+                if (exec_directory_is_private(c, EXEC_DIRECTORY_RUNTIME))
+                        p = path_join(runtime_prefix, "private", *i);
+                else
+                        p = path_join(runtime_prefix, *i);
                 if (!p)
                         return -ENOMEM;
 
@@ -4292,8 +4304,8 @@ static void strv_fprintf(FILE *f, char **l) {
 }
 
 void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) {
+        char **e, **d, buf_clean[FORMAT_TIMESPAN_MAX];
         ExecDirectoryType dt;
-        char **e, **d;
         unsigned i;
         int r;
 
@@ -4368,6 +4380,10 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) {
                         fprintf(f, "%s%s: %s\n", prefix, exec_directory_type_to_string(dt), *d);
         }
 
+        fprintf(f,
+                "%sTimeoutCleanSec: %s\n",
+                prefix, format_timespan(buf_clean, sizeof(buf_clean), c->timeout_clean_usec, USEC_PER_SEC));
+
         if (c->nice_set)
                 fprintf(f,
                         "%sNice: %i\n",
@@ -4811,6 +4827,17 @@ int exec_context_get_clean_directories(
                         r = strv_consume(&l, j);
                         if (r < 0)
                                 return r;
+
+                        /* Also remove private directories unconditionally. */
+                        if (t != EXEC_DIRECTORY_CONFIGURATION) {
+                                j = path_join(prefix[t], "private", *i);
+                                if (!j)
+                                        return -ENOMEM;
+
+                                r = strv_consume(&l, j);
+                                if (r < 0)
+                                        return r;
+                        }
                 }
         }
 
diff --git a/src/core/execute.h b/src/core/execute.h
index 0d5398acfa..cddfafa0d6 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -287,6 +287,7 @@ struct ExecContext {
 
         ExecDirectory directories[_EXEC_DIRECTORY_TYPE_MAX];
         ExecPreserveMode runtime_directory_preserve_mode;
+        usec_t timeout_clean_usec;
 };
 
 static inline bool exec_context_restrict_namespaces_set(const ExecContext *c) {
diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
index 58a28e3241..5490ae32fb 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -138,6 +138,7 @@ $1.LogsDirectoryMode,            config_parse_mode,                  0,
 $1.LogsDirectory,                config_parse_exec_directories,      0,                             offsetof($1, exec_context.directories[EXEC_DIRECTORY_LOGS].paths)
 $1.ConfigurationDirectoryMode,   config_parse_mode,                  0,                             offsetof($1, exec_context.directories[EXEC_DIRECTORY_CONFIGURATION].mode)
 $1.ConfigurationDirectory,       config_parse_exec_directories,      0,                             offsetof($1, exec_context.directories[EXEC_DIRECTORY_CONFIGURATION].paths)
+$1.TimeoutCleanSec,              config_parse_sec,                   0,                             offsetof($1, exec_context.timeout_clean_usec)
 $1.ProtectHostname,              config_parse_bool,                  0,                             offsetof($1, exec_context.protect_hostname)
 m4_ifdef(`HAVE_PAM',
 `$1.PAMName,                     config_parse_unit_string_printf,    0,                             offsetof($1, exec_context.pam_name)',
@@ -316,7 +317,6 @@ Service.TimeoutSec,              config_parse_service_timeout,       0,
 Service.TimeoutStartSec,         config_parse_service_timeout,       0,                             0
 Service.TimeoutStopSec,          config_parse_sec_fix_0,             0,                             offsetof(Service, timeout_stop_usec)
 Service.TimeoutAbortSec,         config_parse_service_timeout_abort, 0,                             0
-Service.TimeoutCleanSec,         config_parse_sec,                   0,                             offsetof(Service, timeout_clean_usec)
 Service.RuntimeMaxSec,           config_parse_sec,                   0,                             offsetof(Service, runtime_max_usec)
 Service.WatchdogSec,             config_parse_sec,                   0,                             offsetof(Service, watchdog_usec)
 m4_dnl The following five only exist for compatibility, they moved into Unit, see above
diff --git a/src/core/service.c b/src/core/service.c
index 61e18de5fb..73b3c9c316 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -108,7 +108,6 @@ static void service_init(Unit *u) {
         s->timeout_abort_set = u->manager->default_timeout_abort_set;
         s->restart_usec = u->manager->default_restart_usec;
         s->runtime_max_usec = USEC_INFINITY;
-        s->timeout_clean_usec = USEC_INFINITY;
         s->type = _SERVICE_TYPE_INVALID;
         s->socket_fd = -1;
         s->stdin_fd = s->stdout_fd = s->stderr_fd = -1;
@@ -794,8 +793,7 @@ static int service_load(Unit *u) {
 
 static void service_dump(Unit *u, FILE *f, const char *prefix) {
         char buf_restart[FORMAT_TIMESPAN_MAX], buf_start[FORMAT_TIMESPAN_MAX], buf_stop[FORMAT_TIMESPAN_MAX],
-                buf_runtime[FORMAT_TIMESPAN_MAX], buf_watchdog[FORMAT_TIMESPAN_MAX], buf_abort[FORMAT_TIMESPAN_MAX],
-                buf_clean[FORMAT_TIMESPAN_MAX];
+                buf_runtime[FORMAT_TIMESPAN_MAX], buf_watchdog[FORMAT_TIMESPAN_MAX], buf_abort[FORMAT_TIMESPAN_MAX];
         ServiceExecCommand c;
         Service *s = SERVICE(u);
         const char *prefix2;
@@ -878,10 +876,8 @@ static void service_dump(Unit *u, FILE *f, const char *prefix) {
                         prefix, format_timespan(buf_abort, sizeof(buf_abort), s->timeout_abort_usec, USEC_PER_SEC));
 
         fprintf(f,
-                "%sTimeoutCleanSec: %s\n"
                 "%sRuntimeMaxSec: %s\n"
                 "%sWatchdogSec: %s\n",
-                prefix, format_timespan(buf_clean, sizeof(buf_clean), s->timeout_clean_usec, USEC_PER_SEC),
                 prefix, format_timespan(buf_runtime, sizeof(buf_runtime), s->runtime_max_usec, USEC_PER_SEC),
                 prefix, format_timespan(buf_watchdog, sizeof(buf_watchdog), s->watchdog_usec, USEC_PER_SEC));
 
@@ -1167,7 +1163,7 @@ static usec_t service_coldplug_timeout(Service *s) {
                 return usec_add(UNIT(s)->inactive_enter_timestamp.monotonic, s->restart_usec);
 
         case SERVICE_CLEANING:
-                return usec_add(UNIT(s)->state_change_timestamp.monotonic, s->timeout_clean_usec);
+                return usec_add(UNIT(s)->state_change_timestamp.monotonic, s->exec_context.timeout_clean_usec);
 
         default:
                 return USEC_INFINITY;
@@ -4271,7 +4267,7 @@ static int service_clean(Unit *u, ExecCleanMask mask) {
         s->control_command = NULL;
         s->control_command_id = _SERVICE_EXEC_COMMAND_INVALID;
 
-        r = service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_clean_usec));
+        r = service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->exec_context.timeout_clean_usec));
         if (r < 0)
                 goto fail;
 
diff --git a/src/core/service.h b/src/core/service.h
index adc05d3218..11e861a3d4 100644
--- a/src/core/service.h
+++ b/src/core/service.h
@@ -101,7 +101,6 @@ struct Service {
         usec_t timeout_stop_usec;
         usec_t timeout_abort_usec;
         bool timeout_abort_set;
-        usec_t timeout_clean_usec;
         usec_t runtime_max_usec;
 
         dual_timestamp watchdog_timestamp;
diff --git a/test/TEST-33-CLEAN-UNIT/testsuite.sh b/test/TEST-33-CLEAN-UNIT/testsuite.sh
index 15aa687238..26754f01ba 100755
--- a/test/TEST-33-CLEAN-UNIT/testsuite.sh
+++ b/test/TEST-33-CLEAN-UNIT/testsuite.sh
@@ -74,6 +74,101 @@ systemctl clean testservice --what=all
 ! test -e /var/cache/testservice
 ! test -e /var/log/testservice
 
+cat > /etc/systemd/system/testservice.service <<EOF
+[Service]
+DynamicUser=yes
+ConfigurationDirectory=testservice
+RuntimeDirectory=testservice
+StateDirectory=testservice
+CacheDirectory=testservice
+LogsDirectory=testservice
+RuntimeDirectoryPreserve=yes
+ExecStart=/bin/sleep infinity
+Type=exec
+EOF
+
+systemctl daemon-reload
+
+! test -e /etc/testservice
+! test -e /run/testservice
+! test -e /var/lib/testservice
+! test -e /var/cache/testservice
+! test -e /var/log/testservice
+
+systemctl restart testservice
+
+test -d /etc/testservice
+test -d /run/private/testservice
+test -d /var/lib/private/testservice
+test -d /var/cache/private/testservice
+test -d /var/log/private/testservice
+test -L /run/testservice
+test -L /var/lib/testservice
+test -L /var/cache/testservice
+test -L /var/log/testservice
+
+! systemctl clean testservice
+
+systemctl stop testservice
+
+test -d /etc/testservice
+test -d /run/private/testservice
+test -d /var/lib/private/testservice
+test -d /var/cache/private/testservice
+test -d /var/log/private/testservice
+test -L /run/testservice
+test -L /var/lib/testservice
+test -L /var/cache/testservice
+test -L /var/log/testservice
+
+systemctl clean testservice --what=configuration
+
+! test -d /etc/testservice
+test -d /run/private/testservice
+test -d /var/lib/private/testservice
+test -d /var/cache/private/testservice
+test -d /var/log/private/testservice
+test -L /run/testservice
+test -L /var/lib/testservice
+test -L /var/cache/testservice
+test -L /var/log/testservice
+
+systemctl clean testservice
+
+! test -d /etc/testservice
+! test -d /run/private/testservice
+test -d /var/lib/private/testservice
+! test -d /var/cache/private/testservice
+test -d /var/log/private/testservice
+! test -L /run/testservice
+test -L /var/lib/testservice
+! test -L /var/cache/testservice
+test -L /var/log/testservice
+
+systemctl clean testservice --what=logs
+
+! test -d /etc/testservice
+! test -d /run/private/testservice
+test -d /var/lib/private/testservice
+! test -d /var/cache/private/testservice
+! test -d /var/log/private/testservice
+! test -L /run/testservice
+test -L /var/lib/testservice
+! test -L /var/cache/testservice
+! test -L /var/log/testservice
+
+systemctl clean testservice --what=all
+
+! test -d /etc/testservice
+! test -d /run/private/testservice
+! test -d /var/lib/private/testservice
+! test -d /var/cache/private/testservice
+! test -d /var/log/private/testservice
+! test -L /run/testservice
+! test -L /var/lib/testservice
+! test -L /var/cache/testservice
+! test -L /var/log/testservice
+
 echo OK > /testok
 
 exit 0