Merge pull request #24784 from yuwata/core-exec-directory

core: do not create symlink to private directory if parent already exists
2026-04-14 08:25:20 +09:00 · 2022-10-12 09:37:16 +02:00
parent 9a72e98f02 f01f70a9a3
commit 9d4cfc7579
7 changed files with 247 additions and 64 deletions
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -3320,10 +3320,11 @@ int bus_exec_context_set_transient_property(
                                _cleanup_free_ char *joined = NULL;

                                STRV_FOREACH(source, l) {
-                                        r = exec_directory_add(&d->items, &d->n_items, *source, NULL);
+                                        r = exec_directory_add(d, *source, NULL);
                                        if (r < 0)
                                                return log_oom();
                                }
+                                exec_directory_sort(d);

                                joined = unit_concat_strv(l, UNIT_ESCAPE_SPECIFIERS);
                                if (!joined)
@@ -3765,21 +3766,8 @@ int bus_exec_context_set_transient_property(

                        if (!UNIT_WRITE_FLAGS_NOOP(flags)) {
                                _cleanup_free_ char *destination_escaped = NULL, *source_escaped = NULL;
-                                ExecDirectoryItem *item = NULL;

-                                /* Adding new directories is supported from both *DirectorySymlink methods and the
-                                 * older ones, so try to find an existing configuration first and create it if it's
-                                 * not there yet. */
-                                for (size_t j = 0; j < directory->n_items; ++j)
-                                        if (path_equal(source, directory->items[j].path)) {
-                                                item = &directory->items[j];
-                                                break;
-                                        }
-
-                                if (item)
-                                        r = strv_extend(&item->symlinks, destination);
-                                else
-                                        r = exec_directory_add(&directory->items, &directory->n_items, source, STRV_MAKE(destination));
+                                r = exec_directory_add(directory, source, destination);
                                if (r < 0)
                                        return r;

@@ -3800,6 +3788,8 @@ int bus_exec_context_set_transient_property(
                if (r < 0)
                        return r;

+                exec_directory_sort(directory);
+
                r = sd_bus_message_exit_container(message);
                if (r < 0)
                        return r;
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -91,6 +91,7 @@
 #include "signal-util.h"
 #include "smack-util.h"
 #include "socket-util.h"
+#include "sort-util.h"
 #include "special.h"
 #include "stat-util.h"
 #include "string-table.h"
@@ -2412,12 +2413,24 @@ static int setup_exec_directory(
                                        goto fail;
                        }

-                        /* And link it up from the original place. Note that if a mount namespace is going to be
-                         * used, then this symlink remains on the host, and a new one for the child namespace will
-                         * be created later. */
-                        r = symlink_idempotent(pp, p, true);
-                        if (r < 0)
-                                goto fail;
+                        if (!context->directories[type].items[i].only_create) {
+                                /* And link it up from the original place.
+                                 * Notes
+                                 * 1) If a mount namespace is going to be used, then this symlink remains on
+                                 *    the host, and a new one for the child namespace will be created later.
+                                 * 2) It is not necessary to create this symlink when one of its parent
+                                 *    directories is specified and already created. E.g.
+                                 *        StateDirectory=foo foo/bar
+                                 *    In that case, the inode points to pp and p for "foo/bar" are the same:
+                                 *        pp = "/var/lib/private/foo/bar"
+                                 *        p = "/var/lib/foo/bar"
+                                 *    and, /var/lib/foo is a symlink to /var/lib/private/foo. So, not only
+                                 *    we do not need to create the symlink, but we cannot create the symlink.
+                                 *    See issue #24783. */
+                                r = symlink_idempotent(pp, p, true);
+                                if (r < 0)
+                                        goto fail;
+                        }

                } else {
                        _cleanup_free_ char *target = NULL;
@@ -3289,7 +3302,8 @@ static int compile_bind_mounts(
                if (!params->prefix[t])
                        continue;

-                n += context->directories[t].n_items;
+                for (size_t i = 0; i < context->directories[t].n_items; i++)
+                        n += !context->directories[t].items[i].only_create;
        }

        if (n <= 0) {
@@ -3358,6 +3372,11 @@ static int compile_bind_mounts(
                for (size_t i = 0; i < context->directories[t].n_items; i++) {
                        char *s, *d;

+                        /* When one of the parent directories is in the list, we cannot create the symlink
+                         * for the child directory. See also the comments in setup_exec_directory(). */
+                        if (context->directories[t].items[i].only_create)
+                                continue;
+
                        if (exec_directory_is_private(context, t))
                                s = path_join(params->prefix[t], "private", context->directories[t].items[i].path);
                        else
@@ -3437,7 +3456,9 @@ static int compile_symlinks(
                                        return r;
                        }

-                        if (!exec_directory_is_private(context, dt) || exec_context_with_rootfs(context))
+                        if (!exec_directory_is_private(context, dt) ||
+                            exec_context_with_rootfs(context) ||
+                            context->directories[dt].items[i].only_create)
                                continue;

                        private_path = path_join(params->prefix[dt], "private", context->directories[dt].items[i].path);
@@ -7041,33 +7062,82 @@ void exec_directory_done(ExecDirectory *d) {
        d->mode = 0755;
 }

-int exec_directory_add(ExecDirectoryItem **d, size_t *n, const char *path, char **symlinks) {
+static ExecDirectoryItem *exec_directory_find(ExecDirectory *d, const char *path) {
+        assert(d);
+        assert(path);
+
+        for (size_t i = 0; i < d->n_items; i++)
+                if (path_equal(d->items[i].path, path))
+                        return &d->items[i];
+
+        return NULL;
+}
+
+int exec_directory_add(ExecDirectory *d, const char *path, const char *symlink) {
        _cleanup_strv_free_ char **s = NULL;
        _cleanup_free_ char *p = NULL;
+        ExecDirectoryItem *existing;
+        int r;

        assert(d);
-        assert(n);
        assert(path);

+        existing = exec_directory_find(d, path);
+        if (existing) {
+                r = strv_extend(&existing->symlinks, symlink);
+                if (r < 0)
+                        return r;
+
+                return 0; /* existing item is updated */
+        }
+
        p = strdup(path);
        if (!p)
                return -ENOMEM;

-        if (symlinks) {
-                s = strv_copy(symlinks);
+        if (symlink) {
+                s = strv_new(symlink);
                if (!s)
                        return -ENOMEM;
        }

-        if (!GREEDY_REALLOC(*d, *n + 1))
+        if (!GREEDY_REALLOC(d->items, d->n_items + 1))
                return -ENOMEM;

-        (*d)[(*n) ++] = (ExecDirectoryItem) {
+        d->items[d->n_items++] = (ExecDirectoryItem) {
                .path = TAKE_PTR(p),
                .symlinks = TAKE_PTR(s),
        };

-        return 0;
+        return 1; /* new item is added */
+}
+
+static int exec_directory_item_compare_func(const ExecDirectoryItem *a, const ExecDirectoryItem *b) {
+        assert(a);
+        assert(b);
+
+        return path_compare(a->path, b->path);
+}
+
+void exec_directory_sort(ExecDirectory *d) {
+        assert(d);
+
+        /* Sort the exec directories to make always parent directories processed at first in
+         * setup_exec_directory(), e.g., even if StateDirectory=foo/bar foo, we need to create foo at first,
+         * then foo/bar. Also, set .only_create flag if one of the parent directories is contained in the
+         * list. See also comments in setup_exec_directory() and issue #24783. */
+
+        if (d->n_items <= 1)
+                return;
+
+        typesafe_qsort(d->items, d->n_items, exec_directory_item_compare_func);
+
+        for (size_t i = 1; i < d->n_items; i++)
+                for (size_t j = 0; j < i; j++)
+                        if (path_startswith(d->items[i].path, d->items[j].path)) {
+                                d->items[i].only_create = true;
+                                break;
+                        }
 }

 DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(exec_set_credential_hash_ops, char, string_hash_func, string_compare_func, ExecSetCredential, exec_set_credential_free);
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -135,6 +135,7 @@ typedef enum ExecDirectoryType {
 typedef struct ExecDirectoryItem {
        char *path;
        char **symlinks;
+        bool only_create;
 } ExecDirectoryItem;

 typedef struct ExecDirectory {
@@ -492,7 +493,8 @@ ExecLoadCredential *exec_load_credential_free(ExecLoadCredential *lc);
 DEFINE_TRIVIAL_CLEANUP_FUNC(ExecLoadCredential*, exec_load_credential_free);

 void exec_directory_done(ExecDirectory *d);
-int exec_directory_add(ExecDirectoryItem **d, size_t *n, const char *path, char **symlinks);
+int exec_directory_add(ExecDirectory *d, const char *path, const char *symlink);
+void exec_directory_sort(ExecDirectory *d);

 extern const struct hash_ops exec_set_credential_hash_ops;
 extern const struct hash_ops exec_load_credential_hash_ops;
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -4629,10 +4629,8 @@ int config_parse_exec_directories(

                /* For State and Runtime directories we support an optional destination parameter, which
                 * will be used to create a symlink to the source. */
-                _cleanup_strv_free_ char **symlinks = NULL;
+                _cleanup_free_ char *dresolved = NULL;
                if (!isempty(dest)) {
-                        _cleanup_free_ char *dresolved = NULL;
-
                        if (streq(lvalue, "ConfigurationDirectory")) {
                                log_syntax(unit, LOG_WARNING, filename, line, 0,
                                           "Destination parameter is not supported for ConfigurationDirectory, ignoring: %s", tuple);
@@ -4649,13 +4647,9 @@ int config_parse_exec_directories(
                        r = path_simplify_and_warn(dresolved, PATH_CHECK_RELATIVE, unit, filename, line, lvalue);
                        if (r < 0)
                                continue;
-
-                        r = strv_consume(&symlinks, TAKE_PTR(dresolved));
-                        if (r < 0)
-                                return log_oom();
                }

-                r = exec_directory_add(&ed->items, &ed->n_items, sresolved, symlinks);
+                r = exec_directory_add(ed, sresolved, dresolved);
                if (r < 0)
                        return log_oom();
        }
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -4131,6 +4131,9 @@ int unit_patch_contexts(Unit *u) {
                        ec->no_new_privileges = true;
                        ec->restrict_suid_sgid = true;
                }
+
+                for (ExecDirectoryType dt = 0; dt < _EXEC_DIRECTORY_TYPE_MAX; dt++)
+                        exec_directory_sort(ec->directories + dt);
        }

        cc = unit_get_cgroup_context(u);
--- a/test/test-execute/exec-dynamicuser-statedir.service
+++ b/test/test-execute/exec-dynamicuser-statedir.service
@@ -5,17 +5,72 @@ Description=Test DynamicUser= with StateDirectory=
 [Service]
 ExecStart=test -w /var/lib/waldo
 ExecStart=test -w /var/lib/quux/pief
-ExecStart=touch /var/lib/waldo/yay
-ExecStart=touch /var/lib/quux/pief/yayyay
-ExecStart=test -f /var/lib/waldo/yay
-ExecStart=test -f /var/lib/quux/pief/yayyay
-ExecStart=test -f /var/lib/private/waldo/yay
-ExecStart=test -f /var/lib/private/quux/pief/yayyay
-ExecStart=sh -x -c 'test "$$STATE_DIRECTORY" = "%S/waldo:%S/quux/pief"'
+ExecStart=test -w /var/lib/aaa
+ExecStart=test -w /var/lib/aaa/bbb
+ExecStart=test -w /var/lib/aaa/ccc
+ExecStart=test -w /var/lib/xxx
+ExecStart=test -w /var/lib/xxx/yyy
+ExecStart=test -w /var/lib/xxx/zzz
+ExecStart=test -w /var/lib/aaa/111
+ExecStart=test -w /var/lib/aaa/222
+ExecStart=test -w /var/lib/aaa/333

-# Make sure that /var/lib/private/waldo is really the only writable directory besides the obvious candidates
-ExecStart=sh -x -c 'test $$(find / \\( -path /var/tmp -o -path /tmp -o -path /proc -o -path /dev/mqueue -o -path /dev/shm -o -path /sys/fs/bpf -o -path /dev/.lxc -o -path /sys/devices/system/cpu \\) -prune -o -type d -writable -print 2>/dev/null | sort -u | tr -d "\\\\n") = /var/lib/private/quux/pief/var/lib/private/waldo'
+ExecStart=test -d /var/lib/waldo
+ExecStart=test -d /var/lib/quux/pief
+ExecStart=test -d /var/lib/aaa
+ExecStart=test -d /var/lib/aaa/bbb
+ExecStart=test -d /var/lib/aaa/ccc
+ExecStart=test -d /var/lib/xxx
+ExecStart=test -d /var/lib/xxx/yyy
+ExecStart=test -d /var/lib/xxx/zzz
+ExecStart=test -L /var/lib/aaa/111
+ExecStart=test -L /var/lib/aaa/222
+ExecStart=test -L /var/lib/aaa/333
+
+ExecStart=touch /var/lib/waldo/hoge
+ExecStart=touch /var/lib/quux/pief/hoge
+ExecStart=touch /var/lib/aaa/hoge
+ExecStart=touch /var/lib/aaa/bbb/hoge
+ExecStart=touch /var/lib/aaa/ccc/hoge
+ExecStart=touch /var/lib/xxx/hoge
+ExecStart=touch /var/lib/xxx/yyy/hoge
+ExecStart=touch /var/lib/xxx/zzz/hoge
+ExecStart=touch /var/lib/aaa/111/foo
+ExecStart=touch /var/lib/aaa/222/foo
+ExecStart=touch /var/lib/aaa/333/foo
+
+ExecStart=test -f /var/lib/waldo/hoge
+ExecStart=test -f /var/lib/quux/pief/hoge
+ExecStart=test -f /var/lib/aaa/hoge
+ExecStart=test -f /var/lib/aaa/bbb/hoge
+ExecStart=test -f /var/lib/aaa/ccc/hoge
+ExecStart=test -f /var/lib/xxx/hoge
+ExecStart=test -f /var/lib/xxx/yyy/hoge
+ExecStart=test -f /var/lib/xxx/zzz/hoge
+ExecStart=test -f /var/lib/aaa/111/foo
+ExecStart=test -f /var/lib/aaa/222/foo
+ExecStart=test -f /var/lib/aaa/333/foo
+ExecStart=test -f /var/lib/xxx/foo
+ExecStart=test -f /var/lib/xxx/yyy/foo
+ExecStart=test -f /var/lib/xxx/zzz/foo
+
+ExecStart=test -f /var/lib/private/waldo/hoge
+ExecStart=test -f /var/lib/private/quux/pief/hoge
+ExecStart=test -f /var/lib/private/aaa/hoge
+ExecStart=test -f /var/lib/private/aaa/bbb/hoge
+ExecStart=test -f /var/lib/private/aaa/ccc/hoge
+ExecStart=test -f /var/lib/private/xxx/hoge
+ExecStart=test -f /var/lib/private/xxx/yyy/hoge
+ExecStart=test -f /var/lib/private/xxx/zzz/hoge
+ExecStart=test -f /var/lib/private/aaa/111/foo
+ExecStart=test -f /var/lib/private/aaa/222/foo
+ExecStart=test -f /var/lib/private/aaa/333/foo
+ExecStart=test -f /var/lib/private/xxx/foo
+ExecStart=test -f /var/lib/private/xxx/yyy/foo
+ExecStart=test -f /var/lib/private/xxx/zzz/foo
+
+ExecStart=sh -x -c 'test "$$STATE_DIRECTORY" = "%S/aaa:%S/aaa/bbb:%S/aaa/ccc:%S/quux/pief:%S/waldo:%S/xxx:%S/xxx/yyy:%S/xxx/zzz"'

 Type=oneshot
 DynamicUser=yes
-StateDirectory=waldo quux/pief
+StateDirectory=waldo quux/pief aaa/bbb aaa aaa/ccc xxx/yyy:aaa/111 xxx:aaa/222 xxx/zzz:aaa/333
--- a/test/units/testsuite-34.sh
+++ b/test/units/testsuite-34.sh
@@ -5,17 +5,22 @@ set -o pipefail

 systemd-analyze log-level debug

-function test_directory() {
+test_directory() {
    local directory="$1"
    local path="$2"

+    # cleanup for previous invocation
+    for i in xxx xxx2 yyy zzz x:yz x:yz2; do
+        rm -rf "${path:?}/${i}" "${path:?}/private/${i}"
+    done
+
    # Set everything up without DynamicUser=1

    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=0 -p "${directory}"=zzz touch "${path}"/zzz/test
    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=0 -p "${directory}"=zzz test -f "${path}"/zzz/test
    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=0 -p "${directory}"=zzz -p TemporaryFileSystem="${path}" test -f "${path}"/zzz/test
    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=0 -p "${directory}"=zzz:yyy test -f "${path}"/yyy/test
-    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=0 -p "${directory}"=zzz:xxx -p TemporaryFileSystem="${path}" test -f "${path}"/xxx/test
+    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=0 -p "${directory}=zzz:xxx zzz:xxx2" -p TemporaryFileSystem="${path}" bash -c "test -f ${path}/xxx/test && test -f ${path}/xxx2/test"
    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=0 -p "${directory}"=zzz:xxx -p TemporaryFileSystem="${path}":ro test -f "${path}"/xxx/test
    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=0 -p "${directory}"=zzz test -f "${path}"/zzz/test-missing \
        && { echo 'unexpected success'; exit 1; }
@@ -23,27 +28,40 @@ function test_directory() {
    test -d "${path}"/zzz
    test ! -L "${path}"/zzz
    test ! -e "${path}"/private/zzz
+
+    test ! -e "${path}"/xxx
+    test ! -e "${path}"/private/xxx
+    test ! -e "${path}"/xxx2
+    test ! -e "${path}"/private/xxx2
+    test -L "${path}"/yyy
+    test ! -e "${path}"/private/yyy
+
    test -f "${path}"/zzz/test
-    test ! -f "${path}"/zzz/test-missing
+    test ! -e "${path}"/zzz/test-missing

    # Convert to DynamicUser=1

    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=1 -p "${directory}"=zzz test -f "${path}"/zzz/test
    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=1 -p "${directory}"=zzz -p TemporaryFileSystem="${path}" test -f "${path}"/zzz/test
    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=1 -p "${directory}"=zzz:yyy test -f "${path}"/yyy/test
-    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=1 -p "${directory}"=zzz:xxx -p TemporaryFileSystem="${path}" test -f "${path}"/xxx/test
+    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=1 -p "${directory}=zzz:xxx zzz:xxx2" \
+                -p TemporaryFileSystem="${path}" -p EnvironmentFile=-/usr/lib/systemd/systemd-asan-env bash -c "test -f ${path}/xxx/test && test -f ${path}/xxx2/test"
    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=1 -p "${directory}"=zzz:xxx -p TemporaryFileSystem="${path}":ro test -f "${path}"/xxx/test
    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=1 -p "${directory}"=zzz test -f "${path}"/zzz/test-missing \
        && { echo 'unexpected success'; exit 1; }

    test -L "${path}"/zzz
-    test -L "${path}"/yyy
    test -d "${path}"/private/zzz
-    test ! -L "${path}"/private/xxx
-    test ! -L "${path}"/xxx
+
+    test ! -e "${path}"/xxx
+    test ! -e "${path}"/private/xxx
+    test ! -e "${path}"/xxx2
+    test ! -e "${path}"/private/xxx2
+    test -L "${path}"/yyy # previous symlink is not removed
+    test ! -e "${path}"/private/yyy

    test -f "${path}"/zzz/test
-    test ! -f "${path}"/zzz/test-missing
+    test ! -e "${path}"/zzz/test-missing

    # Convert back

@@ -56,6 +74,20 @@ function test_directory() {
    systemd-run --wait -p RuntimeDirectoryPreserve=yes -p DynamicUser=0 -p "${directory}"=zzz test -f "${path}"/zzz/test-missing \
        && { echo 'unexpected success'; exit 1; }

+    test -d "${path}"/zzz
+    test ! -L "${path}"/zzz
+    test ! -e "${path}"/private/zzz
+
+    test ! -e "${path}"/xxx
+    test ! -e "${path}"/private/xxx
+    test ! -e "${path}"/xxx2
+    test ! -e "${path}"/private/xxx2
+    test -L "${path}"/yyy
+    test ! -e "${path}"/private/yyy
+
+    test -f "${path}"/zzz/test
+    test ! -e "${path}"/zzz/test-missing
+
    # Exercise the unit parsing paths too
    cat >/run/systemd/system/testservice-34.service <<EOF
 [Service]
@@ -71,24 +103,61 @@ EOF
    systemctl start --wait testservice-34.service

    test -d "${path}"/zzz
-    test ! -L "${path}"/xxx
-    test ! -L "${path}"/xxx2
-    test ! -L "${path}"/private/xxx
-    test ! -L "${path}"/private/xxx2
-    test -L "${path}"/yyy
    test ! -L "${path}"/zzz
    test ! -e "${path}"/private/zzz
-    test -f "${path}"/zzz/test
-    test ! -f "${path}"/zzz/test-missing
+
    test ! -L "${path}"/x:yz
    test ! -L "${path}"/x:yz2
 }

+test_check_writable() {
+    # cleanup for previous invocation
+    for i in aaa quux waldo xxx; do
+        rm -rf "/var/lib/$i" "/var/lib/private/$i"
+    done
+
+    cat >/run/systemd/system/testservice-34-check-writable.service <<EOF
+[Unit]
+Description=Check writable directories when DynamicUser= with StateDirectory=
+
+[Service]
+# Relevant only for sanitizer runs
+EnvironmentFile=-/usr/lib/systemd/systemd-asan-env
+
+Type=oneshot
+DynamicUser=yes
+StateDirectory=waldo quux/pief aaa/bbb aaa aaa/ccc xxx/yyy:aaa/111 xxx:aaa/222 xxx/zzz:aaa/333
+
+# Make sure that the state directories are really the only writable directory besides the obvious candidates
+ExecStart=bash -c ' \
+    set -eux; \
+    set -o pipefail; \
+    declare -a writable_dirs; \
+    readarray -t writable_dirs < <(find / \\( -path /var/tmp -o -path /tmp -o -path /proc -o -path /dev/mqueue -o -path /dev/shm -o \
+                                              -path /sys/fs/bpf -o -path /dev/.lxc -o -path /sys/devices/system/cpu \\) \
+                                   -prune -o -type d -writable -print 2>/dev/null | sort -u); \
+    [[ "\$\${#writable_dirs[@]}" == "8" ]]; \
+    [[ "\$\${writable_dirs[0]}" == "/var/lib/private/aaa" ]]; \
+    [[ "\$\${writable_dirs[1]}" == "/var/lib/private/aaa/bbb" ]]; \
+    [[ "\$\${writable_dirs[2]}" == "/var/lib/private/aaa/ccc" ]]; \
+    [[ "\$\${writable_dirs[3]}" == "/var/lib/private/quux/pief" ]]; \
+    [[ "\$\${writable_dirs[4]}" == "/var/lib/private/waldo" ]]; \
+    [[ "\$\${writable_dirs[5]}" == "/var/lib/private/xxx" ]]; \
+    [[ "\$\${writable_dirs[6]}" == "/var/lib/private/xxx/yyy" ]]; \
+    [[ "\$\${writable_dirs[7]}" == "/var/lib/private/xxx/zzz" ]]; \
+'
+EOF
+    systemctl daemon-reload
+    systemctl start testservice-34-check-writable.service
+}
+
 test_directory "StateDirectory" "/var/lib"
 test_directory "RuntimeDirectory" "/run"
 test_directory "CacheDirectory" "/var/cache"
 test_directory "LogsDirectory" "/var/log"

+test_check_writable
+
 systemd-analyze log-level info

 echo OK >/testok