From f90d2d7bf1b6fc7469fbdeaca17c33a1f5b73cc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 4 Sep 2020 16:06:07 +0200 Subject: [PATCH 1/7] test-execute: simplify condition tests is always a static array, it cannot be NULL. --- src/test/test-execute.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 9ca0620216..b759786eaf 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -782,7 +782,6 @@ typedef struct test_entry { #define entry(x) {x, #x} static int run_tests(UnitFileScope scope, const test_entry tests[], char **patterns) { - const test_entry *test = NULL; _cleanup_(manager_freep) Manager *m = NULL; int r; @@ -795,7 +794,7 @@ static int run_tests(UnitFileScope scope, const test_entry tests[], char **patte assert_se(r >= 0); assert_se(manager_startup(m, NULL, NULL) >= 0); - for (test = tests; test && test->f; test++) + for (const test_entry *test = tests; test->f; test++) if (strv_fnmatch_or_empty(patterns, test->name, FNM_NOESCAPE)) test->f(m); else From 0b3861d2247fd96ca1ff018bbf35c8465c43323c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 4 Sep 2020 16:12:40 +0200 Subject: [PATCH 2/7] test-execute/exec-dynamicuser-statedir.service: fix quoting All backslashes that should be single in shell syntax need to be written as "\\" because our parser will remove one level of quoting. Also, single quotes were doubly nested, which cannot work. Should fix the following message: test-execute/exec-dynamicuser-statedir.service:16: Ignoring unknown escape sequences: "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 \) -prune -o -type d -writable -print 2>/dev/null | sort -u | tr -d \\n) = /var/lib/private/quux/pief/var/lib/private/waldo" --- test/test-execute/exec-dynamicuser-statedir.service | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test-execute/exec-dynamicuser-statedir.service b/test/test-execute/exec-dynamicuser-statedir.service index ca40934de8..6103193ba3 100644 --- a/test/test-execute/exec-dynamicuser-statedir.service +++ b/test/test-execute/exec-dynamicuser-statedir.service @@ -10,10 +10,10 @@ 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=/bin/sh -x -c 'test "$$STATE_DIRECTORY" = "%S/waldo:%S/quux/pief"' +ExecStart=sh -x -c 'test "$$STATE_DIRECTORY" = "%S/waldo:%S/quux/pief"' # 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 \) -prune -o -type d -writable -print 2>/dev/null | sort -u | tr -d '\\\\n') = /var/lib/private/quux/pief/var/lib/private/waldo' +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 \\) -prune -o -type d -writable -print 2>/dev/null | sort -u | tr -d "\\\\n") = /var/lib/private/quux/pief/var/lib/private/waldo' Type=oneshot DynamicUser=yes From 5b10116e4946f5ff050a53bc9d0bccf6eac89337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 4 Sep 2020 17:28:53 +0200 Subject: [PATCH 3/7] core/{execute, manager}: reduce scope of iterator variables a bit --- src/core/execute.c | 141 +++++++++++++++++---------------------------- src/core/manager.c | 14 ++--- 2 files changed, 56 insertions(+), 99 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index e02a55e222..842750ba0f 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -101,8 +101,6 @@ #define SNDBUF_SIZE (8*1024*1024) static int shift_fds(int fds[], size_t n_fds) { - int start, restart_from; - if (n_fds <= 0) return 0; @@ -110,13 +108,10 @@ static int shift_fds(int fds[], size_t n_fds) { assert(fds); - start = 0; - for (;;) { - int i; + for (int start = 0;;) { + int restart_from = -1; - restart_from = -1; - - for (i = start; i < (int) n_fds; i++) { + for (int i = start; i < (int) n_fds; i++) { int nfd; /* Already at right index? */ @@ -146,7 +141,7 @@ static int shift_fds(int fds[], size_t n_fds) { } static int flags_fds(const int fds[], size_t n_socket_fds, size_t n_storage_fds, bool nonblock) { - size_t i, n_fds; + size_t n_fds; int r; n_fds = n_socket_fds + n_storage_fds; @@ -158,7 +153,7 @@ static int flags_fds(const int fds[], size_t n_socket_fds, size_t n_storage_fds, /* Drops/Sets O_NONBLOCK and FD_CLOEXEC from the file flags. * O_NONBLOCK only applies to socket activation though. */ - for (i = 0; i < n_fds; i++) { + for (size_t i = 0; i < n_fds; i++) { if (i < n_socket_fds) { r = fd_nonblock(fds[i], nonblock); @@ -1745,7 +1740,6 @@ static int build_environment( char ***ret) { _cleanup_strv_free_ char **our_env = NULL; - ExecDirectoryType t; size_t n_env = 0; char *x; @@ -1873,7 +1867,7 @@ static int build_environment( our_env[n_env++] = x; } - for (t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { + for (ExecDirectoryType t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { _cleanup_free_ char *pre = NULL, *joined = NULL; const char *n; @@ -1991,12 +1985,10 @@ static bool exec_needs_mount_namespace( return true; if (context->root_directory) { - ExecDirectoryType t; - if (context->mount_apivfs) return true; - for (t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { + for (ExecDirectoryType t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { if (!params->prefix[t]) continue; @@ -2880,8 +2872,7 @@ static int compile_bind_mounts( _cleanup_strv_free_ char **empty_directories = NULL; BindMount *bind_mounts; - size_t n, h = 0, i; - ExecDirectoryType t; + size_t n, h = 0; int r; assert(context); @@ -2891,7 +2882,7 @@ static int compile_bind_mounts( assert(ret_empty_directories); n = context->n_bind_mounts; - for (t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { + for (ExecDirectoryType t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { if (!params->prefix[t]) continue; @@ -2909,7 +2900,7 @@ static int compile_bind_mounts( if (!bind_mounts) return -ENOMEM; - for (i = 0; i < context->n_bind_mounts; i++) { + for (size_t i = 0; i < context->n_bind_mounts; i++) { BindMount *item = context->bind_mounts + i; char *s, *d; @@ -2935,7 +2926,7 @@ static int compile_bind_mounts( }; } - for (t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { + for (ExecDirectoryType t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { char **suffix; if (!params->prefix[t]) @@ -3020,8 +3011,6 @@ static bool insist_on_sandboxing( const BindMount *bind_mounts, size_t n_bind_mounts) { - size_t i; - assert(context); assert(n_bind_mounts == 0 || bind_mounts); @@ -3043,7 +3032,7 @@ static bool insist_on_sandboxing( /* If there are any bind mounts set that don't map back onto themselves, fs namespacing becomes * essential. */ - for (i = 0; i < n_bind_mounts; i++) + for (size_t i = 0; i < n_bind_mounts; i++) if (!path_equal(bind_mounts[i].source, bind_mounts[i].destination)) return true; @@ -3236,13 +3225,12 @@ static int apply_root_directory( assert(context); assert(exit_status); - if (params->flags & EXEC_APPLY_CHROOT) { + if (params->flags & EXEC_APPLY_CHROOT) if (!needs_mount_ns && context->root_directory) if (chroot(context->root_directory) < 0) { *exit_status = EXIT_CHROOT; return -errno; } - } return 0; } @@ -3467,7 +3455,6 @@ static int acquire_home(const ExecContext *c, uid_t uid, const char** home, char static int compile_suggested_paths(const ExecContext *c, const ExecParameters *p, char ***ret) { _cleanup_strv_free_ char ** list = NULL; - ExecDirectoryType t; int r; assert(c); @@ -3480,7 +3467,7 @@ static int compile_suggested_paths(const ExecContext *c, const ExecParameters *p * dynamic UID allocation, in order to save us from doing costly recursive chown()s of the special * directories. */ - for (t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { + for (ExecDirectoryType t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { char **i; if (t == EXEC_DIRECTORY_CONFIGURATION) @@ -3615,7 +3602,6 @@ static int exec_child( uid_t uid = UID_INVALID; gid_t gid = GID_INVALID; size_t n_fds; - ExecDirectoryType dt; int secure_bits; _cleanup_free_ gid_t *gids_after_pam = NULL; int ngids_after_pam = 0; @@ -3954,7 +3940,7 @@ static int exec_child( } } - for (dt = 0; dt < _EXEC_DIRECTORY_TYPE_MAX; dt++) { + for (ExecDirectoryType dt = 0; dt < _EXEC_DIRECTORY_TYPE_MAX; dt++) { r = setup_exec_directory(context, params, uid, gid, dt, exit_status); if (r < 0) return log_unit_error_errno(unit, r, "Failed to set up special execution directory in %s: %m", params->prefix[dt]); @@ -4674,8 +4660,6 @@ int exec_spawn(Unit *unit, } void exec_context_init(ExecContext *c) { - ExecDirectoryType i; - assert(c); c->umask = 0022; @@ -4686,8 +4670,8 @@ void exec_context_init(ExecContext *c) { c->ignore_sigpipe = true; c->timer_slack_nsec = NSEC_INFINITY; c->personality = PERSONALITY_INVALID; - for (i = 0; i < _EXEC_DIRECTORY_TYPE_MAX; i++) - c->directories[i].mode = 0755; + for (ExecDirectoryType t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) + c->directories[t].mode = 0755; c->timeout_clean_usec = USEC_INFINITY; c->capability_bounding_set = CAP_ALL; assert_cc(NAMESPACE_FLAGS_INITIAL != NAMESPACE_FLAGS_ALL); @@ -4697,9 +4681,6 @@ void exec_context_init(ExecContext *c) { } void exec_context_done(ExecContext *c) { - ExecDirectoryType i; - size_t l; - assert(c); c->environment = strv_free(c->environment); @@ -4709,7 +4690,7 @@ void exec_context_done(ExecContext *c) { rlimit_free_all(c->rlimit); - for (l = 0; l < 3; l++) { + for (size_t l = 0; l < 3; l++) { c->stdio_fdname[l] = mfree(c->stdio_fdname[l]); c->stdio_file[l] = mfree(c->stdio_file[l]); } @@ -4758,8 +4739,8 @@ void exec_context_done(ExecContext *c) { c->syscall_archs = set_free(c->syscall_archs); c->address_families = set_free(c->address_families); - for (i = 0; i < _EXEC_DIRECTORY_TYPE_MAX; i++) - c->directories[i].paths = strv_free(c->directories[i].paths); + for (ExecDirectoryType t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) + c->directories[t].paths = strv_free(c->directories[t].paths); c->log_level_max = -1; @@ -4852,23 +4833,17 @@ ExecCommand* exec_command_free_list(ExecCommand *c) { } void exec_command_free_array(ExecCommand **c, size_t n) { - size_t i; - - for (i = 0; i < n; i++) + for (size_t i = 0; i < n; i++) c[i] = exec_command_free_list(c[i]); } void exec_command_reset_status_array(ExecCommand *c, size_t n) { - size_t i; - - for (i = 0; i < n; i++) + for (size_t i = 0; i < n; i++) exec_status_reset(&c[i].exec_status); } void exec_command_reset_status_list_array(ExecCommand **c, size_t n) { - size_t i; - - for (i = 0; i < n; i++) { + for (size_t i = 0; i < n; i++) { ExecCommand *z; LIST_FOREACH(command, z, c[i]) @@ -4920,7 +4895,7 @@ static int exec_context_named_iofds( const ExecParameters *p, int named_iofds[static 3]) { - size_t i, targets; + size_t targets; const char* stdio_fdname[3]; size_t n_fds; @@ -4932,12 +4907,12 @@ static int exec_context_named_iofds( (c->std_output == EXEC_OUTPUT_NAMED_FD) + (c->std_error == EXEC_OUTPUT_NAMED_FD); - for (i = 0; i < 3; i++) + for (size_t i = 0; i < 3; i++) stdio_fdname[i] = exec_context_fdname(c, i); n_fds = p->n_storage_fds + p->n_socket_fds; - for (i = 0; i < n_fds && targets > 0; i++) + for (size_t i = 0; i < n_fds && targets > 0; i++) if (named_iofds[STDIN_FILENO] < 0 && c->std_input == EXEC_INPUT_NAMED_FD && stdio_fdname[STDIN_FILENO] && @@ -4975,7 +4950,6 @@ static int exec_context_load_environment(const Unit *unit, const ExecContext *c, STRV_FOREACH(i, c->environment_files) { char *fn; int k; - unsigned n; bool ignore = false; char **p; _cleanup_globfree_ glob_t pglob = {}; @@ -5008,7 +4982,7 @@ static int exec_context_load_environment(const Unit *unit, const ExecContext *c, /* When we don't match anything, -ENOENT should be returned */ assert(pglob.gl_pathc > 0); - for (n = 0; n < pglob.gl_pathc; n++) { + for (unsigned n = 0; n < pglob.gl_pathc; n++) { k = load_env_file(NULL, pglob.gl_pathv[n], &p); if (k < 0) { if (ignore) @@ -5095,8 +5069,6 @@ 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; - unsigned i; int r; assert(c); @@ -5207,7 +5179,7 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { fprintf(f, "%sRuntimeDirectoryPreserve: %s\n", prefix, exec_preserve_mode_to_string(c->runtime_directory_preserve_mode)); - for (dt = 0; dt < _EXEC_DIRECTORY_TYPE_MAX; dt++) { + for (ExecDirectoryType dt = 0; dt < _EXEC_DIRECTORY_TYPE_MAX; dt++) { fprintf(f, "%s%sMode: %04o\n", prefix, exec_directory_type_to_string(dt), c->directories[dt].mode); STRV_FOREACH(d, c->directories[dt].paths) @@ -5233,7 +5205,7 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { "%sCoredumpFilter: 0x%"PRIx64"\n", prefix, c->coredump_filter); - for (i = 0; i < RLIM_NLIMITS; i++) + for (unsigned i = 0; i < RLIM_NLIMITS; i++) if (c->rlimit[i]) { fprintf(f, "%sLimit%s: " RLIM_FMT "\n", prefix, rlimit_to_string(i), c->rlimit[i]->rlim_max); @@ -5361,16 +5333,12 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { if (c->log_ratelimit_burst > 0) fprintf(f, "%sLogRateLimitBurst: %u\n", prefix, c->log_ratelimit_burst); - if (c->n_log_extra_fields > 0) { - size_t j; - - for (j = 0; j < c->n_log_extra_fields; j++) { - fprintf(f, "%sLogExtraFields: ", prefix); - fwrite(c->log_extra_fields[j].iov_base, - 1, c->log_extra_fields[j].iov_len, - f); - fputc('\n', f); - } + for (size_t j = 0; j < c->n_log_extra_fields; j++) { + fprintf(f, "%sLogExtraFields: ", prefix); + fwrite(c->log_extra_fields[j].iov_base, + 1, c->log_extra_fields[j].iov_len, + f); + fputc('\n', f); } if (c->log_namespace) @@ -5434,24 +5402,22 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { fputs("\n", f); } - if (c->n_bind_mounts > 0) - for (i = 0; i < c->n_bind_mounts; i++) - fprintf(f, "%s%s: %s%s:%s:%s\n", prefix, - c->bind_mounts[i].read_only ? "BindReadOnlyPaths" : "BindPaths", - c->bind_mounts[i].ignore_enoent ? "-": "", - c->bind_mounts[i].source, - c->bind_mounts[i].destination, - c->bind_mounts[i].recursive ? "rbind" : "norbind"); + for (size_t i = 0; i < c->n_bind_mounts; i++) + fprintf(f, "%s%s: %s%s:%s:%s\n", prefix, + c->bind_mounts[i].read_only ? "BindReadOnlyPaths" : "BindPaths", + c->bind_mounts[i].ignore_enoent ? "-": "", + c->bind_mounts[i].source, + c->bind_mounts[i].destination, + c->bind_mounts[i].recursive ? "rbind" : "norbind"); - if (c->n_temporary_filesystems > 0) - for (i = 0; i < c->n_temporary_filesystems; i++) { - TemporaryFileSystem *t = c->temporary_filesystems + i; + for (size_t i = 0; i < c->n_temporary_filesystems; i++) { + const TemporaryFileSystem *t = c->temporary_filesystems + i; - fprintf(f, "%sTemporaryFileSystem: %s%s%s\n", prefix, - t->path, - isempty(t->options) ? "" : ":", - strempty(t->options)); - } + fprintf(f, "%sTemporaryFileSystem: %s%s%s\n", prefix, + t->path, + isempty(t->options) ? "" : ":", + strempty(t->options)); + } if (c->utmp_id) fprintf(f, @@ -5566,7 +5532,7 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { fprintf(f, "%d\n", c->syscall_errno); } - for (i = 0; i < c->n_mount_images; i++) { + for (size_t i = 0; i < c->n_mount_images; i++) { MountOptions *o; fprintf(f, "%sMountImages: %s%s:%s%s", prefix, @@ -5613,11 +5579,9 @@ int exec_context_get_effective_ioprio(const ExecContext *c) { } void exec_context_free_log_extra_fields(ExecContext *c) { - size_t l; - assert(c); - for (l = 0; l < c->n_log_extra_fields; l++) + for (size_t l = 0; l < c->n_log_extra_fields; l++) free(c->log_extra_fields[l].iov_base); c->log_extra_fields = mfree(c->log_extra_fields); c->n_log_extra_fields = 0; @@ -5654,14 +5618,13 @@ int exec_context_get_clean_directories( char ***ret) { _cleanup_strv_free_ char **l = NULL; - ExecDirectoryType t; int r; assert(c); assert(prefix); assert(ret); - for (t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { + for (ExecDirectoryType t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { char **i; if (!FLAGS_SET(mask, 1U << t)) diff --git a/src/core/manager.c b/src/core/manager.c index 5372e81d97..6110fc5799 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1883,7 +1883,6 @@ Unit *manager_get_unit(Manager *m, const char *name) { static int manager_dispatch_target_deps_queue(Manager *m) { Unit *u; - unsigned k; int r = 0; static const UnitDependency deps[] = { @@ -1901,7 +1900,7 @@ static int manager_dispatch_target_deps_queue(Manager *m) { LIST_REMOVE(target_deps_queue, u->manager->target_deps_queue, u); u->in_target_deps_queue = false; - for (k = 0; k < ELEMENTSOF(deps); k++) { + for (size_t k = 0; k < ELEMENTSOF(deps); k++) { Unit *target; Iterator i; void *v; @@ -2123,12 +2122,10 @@ void manager_dump_units(Manager *s, FILE *f, const char *prefix) { } void manager_dump(Manager *m, FILE *f, const char *prefix) { - ManagerTimestamp q; - assert(m); assert(f); - for (q = 0; q < _MANAGER_TIMESTAMP_MAX; q++) { + for (ManagerTimestamp q = 0; q < _MANAGER_TIMESTAMP_MAX; q++) { const dual_timestamp *t = m->timestamps + q; char buf[CONST_MAX(FORMAT_TIMESPAN_MAX, FORMAT_TIMESTAMP_MAX)]; @@ -3228,7 +3225,6 @@ int manager_serialize( FDSet *fds, bool switching_root) { - ManagerTimestamp q; const char *t; Iterator i; Unit *u; @@ -3264,7 +3260,7 @@ int manager_serialize( (void) serialize_usec(f, "reboot-watchdog-overridden", m->watchdog_overridden[WATCHDOG_REBOOT]); (void) serialize_usec(f, "kexec-watchdog-overridden", m->watchdog_overridden[WATCHDOG_KEXEC]); - for (q = 0; q < _MANAGER_TIMESTAMP_MAX; q++) { + for (ManagerTimestamp q = 0; q < _MANAGER_TIMESTAMP_MAX; q++) { _cleanup_free_ char *joined = NULL; if (!manager_timestamp_shall_serialize(q)) @@ -4193,11 +4189,9 @@ int manager_get_effective_environment(Manager *m, char ***ret) { } int manager_set_default_rlimits(Manager *m, struct rlimit **default_rlimit) { - int i; - assert(m); - for (i = 0; i < _RLIMIT_MAX; i++) { + for (unsigned i = 0; i < _RLIMIT_MAX; i++) { m->rlimit[i] = mfree(m->rlimit[i]); if (!default_rlimit[i]) From 9978e631cdfac4d4ac9b38e9496b9df1bd042b60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 4 Sep 2020 17:34:11 +0200 Subject: [PATCH 4/7] core/manager: reindent table for readability --- src/core/manager.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 6110fc5799..f73afe3ba9 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -672,19 +672,19 @@ static int manager_setup_prefix(Manager *m) { }; static const struct table_entry paths_system[_EXEC_DIRECTORY_TYPE_MAX] = { - [EXEC_DIRECTORY_RUNTIME] = { SD_PATH_SYSTEM_RUNTIME, NULL }, - [EXEC_DIRECTORY_STATE] = { SD_PATH_SYSTEM_STATE_PRIVATE, NULL }, - [EXEC_DIRECTORY_CACHE] = { SD_PATH_SYSTEM_STATE_CACHE, NULL }, - [EXEC_DIRECTORY_LOGS] = { SD_PATH_SYSTEM_STATE_LOGS, NULL }, + [EXEC_DIRECTORY_RUNTIME] = { SD_PATH_SYSTEM_RUNTIME, NULL }, + [EXEC_DIRECTORY_STATE] = { SD_PATH_SYSTEM_STATE_PRIVATE, NULL }, + [EXEC_DIRECTORY_CACHE] = { SD_PATH_SYSTEM_STATE_CACHE, NULL }, + [EXEC_DIRECTORY_LOGS] = { SD_PATH_SYSTEM_STATE_LOGS, NULL }, [EXEC_DIRECTORY_CONFIGURATION] = { SD_PATH_SYSTEM_CONFIGURATION, NULL }, }; static const struct table_entry paths_user[_EXEC_DIRECTORY_TYPE_MAX] = { - [EXEC_DIRECTORY_RUNTIME] = { SD_PATH_USER_RUNTIME, NULL }, - [EXEC_DIRECTORY_STATE] = { SD_PATH_USER_CONFIGURATION, NULL }, - [EXEC_DIRECTORY_CACHE] = { SD_PATH_USER_STATE_CACHE, NULL }, - [EXEC_DIRECTORY_LOGS] = { SD_PATH_USER_CONFIGURATION, "log" }, - [EXEC_DIRECTORY_CONFIGURATION] = { SD_PATH_USER_CONFIGURATION, NULL }, + [EXEC_DIRECTORY_RUNTIME] = { SD_PATH_USER_RUNTIME, NULL }, + [EXEC_DIRECTORY_STATE] = { SD_PATH_USER_CONFIGURATION, NULL }, + [EXEC_DIRECTORY_CACHE] = { SD_PATH_USER_STATE_CACHE, NULL }, + [EXEC_DIRECTORY_LOGS] = { SD_PATH_USER_CONFIGURATION, "log" }, + [EXEC_DIRECTORY_CONFIGURATION] = { SD_PATH_USER_CONFIGURATION, NULL }, }; assert(m); From cced2b98efcf60f9dce573451ff637bb8227d90b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 4 Sep 2020 18:02:57 +0200 Subject: [PATCH 5/7] test-execute: check if private directories have bad permissions before running test_exec_dynamicuser() If the directory (/var/lib/private is most likely) has borked permissions, the test will fail with a cryptic message and EXIT_STATE_DIRECTORY or similar. The message from the child with more details gets lost somewhere. Let's avoid running the test in that case and provide a simple error message instead. E.g. systemd-238-12.git07f8cd5.fc28.ppc64 (which I encountered on a test machine) has /var/lib/private with 0755. --- src/test/test-execute.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/test/test-execute.c b/src/test/test-execute.c index b759786eaf..e32e0c0b6c 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -543,7 +543,29 @@ static void test_exec_supplementarygroups(Manager *m) { test(__func__, m, "exec-supplementarygroups-multiple-groups-withuid.service", 0, CLD_EXITED); } +static char* private_directory_bad(Manager *m) { + /* This mirrors setup_exec_directory(). */ + + for (ExecDirectoryType dt = 0; dt < _EXEC_DIRECTORY_TYPE_MAX; dt++) { + _cleanup_free_ char *p = NULL; + struct stat st; + + assert_se(p = path_join(m->prefix[dt], "private")); + + if (stat(p, &st) >= 0 && + (st.st_mode & (S_IRWXG|S_IRWXO))) + return TAKE_PTR(p); + } + + return NULL; +} + static void test_exec_dynamicuser(Manager *m) { + _cleanup_free_ char *bad = private_directory_bad(m); + if (bad) { + log_warning("%s: %s has bad permissions, skipping test.", __func__, bad); + return; + } test(__func__, m, "exec-dynamicuser-fixeduser.service", can_unshare ? 0 : EXIT_NAMESPACE, CLD_EXITED); if (check_user_has_group_with_same_name("adm")) From a9030b81c154c3ec92227d04cad6b13cc1125608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 4 Sep 2020 18:09:20 +0200 Subject: [PATCH 6/7] udev-test: do not rely on "mail" group being defined "audio" should be there, at least we declare it. "mail" nowadays is less likely to exist than in the past. Fixes one of the items in #16942. --- test/udev-test.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/udev-test.pl b/test/udev-test.pl index 2480e4120b..1ab6828d71 100755 --- a/test/udev-test.pl +++ b/test/udev-test.pl @@ -629,9 +629,9 @@ EOF desc => "textual user/group id", devpath => "/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda", exp_name => "node", - exp_perms => "root:mail:0660", + exp_perms => "root:audio:0660", rules => < Date: Fri, 4 Sep 2020 18:32:43 +0200 Subject: [PATCH 7/7] test-sizeof: print pointer sizes This is useful information, I don't know why we forgot to add it there. gcc doesn't like arithemetic on a pointer to a function or void*, so don't print signedness info there. It doesn't matter anyway. C says function pointers can be different... Though I guess our code isn't prepared for that. --- src/test/test-sizeof.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/test/test-sizeof.c b/src/test/test-sizeof.c index b9d63d6b41..6dee2022e8 100644 --- a/src/test/test-sizeof.c +++ b/src/test/test-sizeof.c @@ -16,6 +16,11 @@ DISABLE_WARNING_TYPE_LIMITS; +#define info_no_sign(t) \ + printf("%s → %zu bits, %zu byte alignment\n", STRINGIFY(t), \ + sizeof(t)*CHAR_BIT, \ + __alignof__(t)) + #define info(t) \ printf("%s → %zu bits%s, %zu byte alignment\n", STRINGIFY(t), \ sizeof(t)*CHAR_BIT, \ @@ -37,6 +42,12 @@ enum BigEnum2 { }; int main(void) { + int (*function_pointer)(void); + + info_no_sign(function_pointer); + info_no_sign(void*); + info(char*); + info(char); info(signed char); info(unsigned char);