From 0682ed5cf4696cc12f5bcb4776a3433ef3581748 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 7 Dec 2017 14:42:45 +0100 Subject: [PATCH 1/7] tests: add some tests for unit_name_is_valid() and related functions I was surprised to see that foo@bar@bar.service is a valid unit name. Apparently it is according to current code and docs. --- src/basic/unit-name.c | 2 +- src/test/test-cgroup-util.c | 18 ++++++++++-- src/test/test-unit-name.c | 55 +++++++++++++++++++++++++++++-------- 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index 407982e8ff..479079099b 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -710,7 +710,7 @@ int slice_build_parent_slice(const char *slice, char **ret) { return 1; } -int slice_build_subslice(const char *slice, const char*name, char **ret) { +int slice_build_subslice(const char *slice, const char *name, char **ret) { char *subslice; assert(slice); diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c index 2c53956bdb..fc612b3d3a 100644 --- a/src/test/test-cgroup-util.c +++ b/src/test/test-cgroup-util.c @@ -253,13 +253,18 @@ static void test_controller_is_valid(void) { static void test_slice_to_path_one(const char *unit, const char *path, int error) { _cleanup_free_ char *ret = NULL; + int r; - assert_se(cg_slice_to_path(unit, &ret) == error); + log_info("unit: %s", unit); + + r = cg_slice_to_path(unit, &ret); + log_info("actual: %s / %d", strnull(ret), r); + log_info("expect: %s / %d", strnull(path), error); + assert_se(r == error); assert_se(streq_ptr(ret, path)); } static void test_slice_to_path(void) { - test_slice_to_path_one("foobar.slice", "foobar.slice", 0); test_slice_to_path_one("foobar-waldo.slice", "foobar.slice/foobar-waldo.slice", 0); test_slice_to_path_one("foobar-waldo.service", NULL, -EINVAL); @@ -273,6 +278,15 @@ static void test_slice_to_path(void) { test_slice_to_path_one("foo.slice/foo--bar.slice", NULL, -EINVAL); test_slice_to_path_one("a-b.slice", "a.slice/a-b.slice", 0); test_slice_to_path_one("a-b-c-d-e.slice", "a.slice/a-b.slice/a-b-c.slice/a-b-c-d.slice/a-b-c-d-e.slice", 0); + + test_slice_to_path_one("foobar@.slice", NULL, -EINVAL); + test_slice_to_path_one("foobar@waldo.slice", NULL, -EINVAL); + test_slice_to_path_one("foobar@waldo.service", NULL, -EINVAL); + test_slice_to_path_one("-foo@-.slice", NULL, -EINVAL); + test_slice_to_path_one("-foo@.slice", NULL, -EINVAL); + test_slice_to_path_one("foo@-.slice", NULL, -EINVAL); + test_slice_to_path_one("foo@@bar.slice", NULL, -EINVAL); + test_slice_to_path_one("foo.slice/foo@@bar.slice", NULL, -EINVAL); } static void test_shift_path_one(const char *raw, const char *root, const char *shifted) { diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c index b45c152915..347c586663 100644 --- a/src/test/test-unit-name.c +++ b/src/test/test-unit-name.c @@ -31,29 +31,39 @@ #include "util.h" static void test_unit_name_is_valid(void) { - assert_se(unit_name_is_valid("foo.service", UNIT_NAME_ANY)); - assert_se(unit_name_is_valid("foo.service", UNIT_NAME_PLAIN)); + assert_se( unit_name_is_valid("foo.service", UNIT_NAME_ANY)); + assert_se( unit_name_is_valid("foo.service", UNIT_NAME_PLAIN)); assert_se(!unit_name_is_valid("foo.service", UNIT_NAME_INSTANCE)); assert_se(!unit_name_is_valid("foo.service", UNIT_NAME_TEMPLATE)); assert_se(!unit_name_is_valid("foo.service", UNIT_NAME_INSTANCE|UNIT_NAME_TEMPLATE)); - assert_se(unit_name_is_valid("foo@bar.service", UNIT_NAME_ANY)); + assert_se( unit_name_is_valid("foo@bar.service", UNIT_NAME_ANY)); assert_se(!unit_name_is_valid("foo@bar.service", UNIT_NAME_PLAIN)); - assert_se(unit_name_is_valid("foo@bar.service", UNIT_NAME_INSTANCE)); + assert_se( unit_name_is_valid("foo@bar.service", UNIT_NAME_INSTANCE)); assert_se(!unit_name_is_valid("foo@bar.service", UNIT_NAME_TEMPLATE)); - assert_se(unit_name_is_valid("foo@bar.service", UNIT_NAME_INSTANCE|UNIT_NAME_TEMPLATE)); + assert_se( unit_name_is_valid("foo@bar.service", UNIT_NAME_INSTANCE|UNIT_NAME_TEMPLATE)); - assert_se(unit_name_is_valid("foo@.service", UNIT_NAME_ANY)); + assert_se( unit_name_is_valid("foo@bar@bar.service", UNIT_NAME_ANY)); + assert_se(!unit_name_is_valid("foo@bar@bar.service", UNIT_NAME_PLAIN)); + assert_se( unit_name_is_valid("foo@bar@bar.service", UNIT_NAME_INSTANCE)); + assert_se(!unit_name_is_valid("foo@bar@bar.service", UNIT_NAME_TEMPLATE)); + assert_se( unit_name_is_valid("foo@bar@bar.service", UNIT_NAME_INSTANCE|UNIT_NAME_TEMPLATE)); + + assert_se( unit_name_is_valid("foo@.service", UNIT_NAME_ANY)); assert_se(!unit_name_is_valid("foo@.service", UNIT_NAME_PLAIN)); assert_se(!unit_name_is_valid("foo@.service", UNIT_NAME_INSTANCE)); - assert_se(unit_name_is_valid("foo@.service", UNIT_NAME_TEMPLATE)); - assert_se(unit_name_is_valid("foo@.service", UNIT_NAME_INSTANCE|UNIT_NAME_TEMPLATE)); + assert_se( unit_name_is_valid("foo@.service", UNIT_NAME_TEMPLATE)); + assert_se( unit_name_is_valid("foo@.service", UNIT_NAME_INSTANCE|UNIT_NAME_TEMPLATE)); assert_se(!unit_name_is_valid(".service", UNIT_NAME_ANY)); assert_se(!unit_name_is_valid("", UNIT_NAME_ANY)); assert_se(!unit_name_is_valid("foo.waldo", UNIT_NAME_ANY)); assert_se(!unit_name_is_valid("@.service", UNIT_NAME_ANY)); assert_se(!unit_name_is_valid("@piep.service", UNIT_NAME_ANY)); + + assert_se( unit_name_is_valid("user@1000.slice", UNIT_NAME_ANY)); + assert_se( unit_name_is_valid("user@1000.slice", UNIT_NAME_INSTANCE)); + assert_se(!unit_name_is_valid("user@1000.slice", UNIT_NAME_TEMPLATE)); } static void test_unit_name_replace_instance_one(const char *pattern, const char *repl, const char *expected, int ret) { @@ -323,10 +333,10 @@ static void test_unit_name_build(void) { } static void test_slice_name_is_valid(void) { - assert_se(slice_name_is_valid(SPECIAL_ROOT_SLICE)); - assert_se(slice_name_is_valid("foo.slice")); - assert_se(slice_name_is_valid("foo-bar.slice")); - assert_se(slice_name_is_valid("foo-bar-baz.slice")); + assert_se( slice_name_is_valid(SPECIAL_ROOT_SLICE)); + assert_se( slice_name_is_valid("foo.slice")); + assert_se( slice_name_is_valid("foo-bar.slice")); + assert_se( slice_name_is_valid("foo-bar-baz.slice")); assert_se(!slice_name_is_valid("-foo-bar-baz.slice")); assert_se(!slice_name_is_valid("foo-bar-baz-.slice")); assert_se(!slice_name_is_valid("-foo-bar-baz-.slice")); @@ -335,6 +345,20 @@ static void test_slice_name_is_valid(void) { assert_se(!slice_name_is_valid(".slice")); assert_se(!slice_name_is_valid("")); assert_se(!slice_name_is_valid("foo.service")); + + assert_se(!slice_name_is_valid("foo@.slice")); + assert_se(!slice_name_is_valid("foo@bar.slice")); + assert_se(!slice_name_is_valid("foo-bar@baz.slice")); + assert_se(!slice_name_is_valid("foo@bar@baz.slice")); + assert_se(!slice_name_is_valid("foo@bar-baz.slice")); + assert_se(!slice_name_is_valid("-foo-bar-baz@.slice")); + assert_se(!slice_name_is_valid("foo-bar-baz@-.slice")); + assert_se(!slice_name_is_valid("foo-bar-baz@a--b.slice")); + assert_se(!slice_name_is_valid("-foo-bar-baz@-.slice")); + assert_se(!slice_name_is_valid("foo-bar--baz@.slice")); + assert_se(!slice_name_is_valid("foo--bar--baz@.slice")); + assert_se(!slice_name_is_valid("@.slice")); + assert_se(!slice_name_is_valid("foo@bar.service")); } static void test_build_subslice(void) { @@ -372,6 +396,13 @@ static void test_build_parent_slice(void) { test_build_parent_slice_one("foo-bar-.slice", NULL, -EINVAL); test_build_parent_slice_one("foo-bar.service", NULL, -EINVAL); test_build_parent_slice_one(".slice", NULL, -EINVAL); + test_build_parent_slice_one("foo@bar.slice", NULL, -EINVAL); + test_build_parent_slice_one("foo-bar@baz.slice", NULL, -EINVAL); + test_build_parent_slice_one("foo-bar--@baz.slice", NULL, -EINVAL); + test_build_parent_slice_one("-foo-bar@bar.slice", NULL, -EINVAL); + test_build_parent_slice_one("foo-bar@-.slice", NULL, -EINVAL); + test_build_parent_slice_one("foo@bar.service", NULL, -EINVAL); + test_build_parent_slice_one("@.slice", NULL, -EINVAL); } static void test_unit_name_to_instance(void) { From ae98d374d1e1aac325d583575e58097b1137507e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 9 Dec 2017 11:53:17 +0100 Subject: [PATCH 2/7] logind: move two functions to logind_core utility lib In preparation to reusing them later in other places... --- src/login/logind-core.c | 43 +++++++++++++++++++++++++++++++++++++++++ src/login/logind.c | 42 ---------------------------------------- src/login/logind.h | 3 +++ src/login/meson.build | 5 ++--- 4 files changed, 48 insertions(+), 45 deletions(-) diff --git a/src/login/logind-core.c b/src/login/logind-core.c index 7f429aae41..64ec9ca790 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -15,6 +15,7 @@ #include "bus-error.h" #include "bus-util.h" #include "cgroup-util.h" +#include "conf-parser.h" #include "fd-util.h" #include "logind.h" #include "parse-util.h" @@ -24,6 +25,48 @@ #include "udev-util.h" #include "user-util.h" +void manager_reset_config(Manager *m) { + m->n_autovts = 6; + m->reserve_vt = 6; + m->remove_ipc = true; + m->inhibit_delay_max = 5 * USEC_PER_SEC; + m->handle_power_key = HANDLE_POWEROFF; + m->handle_suspend_key = HANDLE_SUSPEND; + m->handle_hibernate_key = HANDLE_HIBERNATE; + m->handle_lid_switch = HANDLE_SUSPEND; + m->handle_lid_switch_ep = _HANDLE_ACTION_INVALID; + m->handle_lid_switch_docked = HANDLE_IGNORE; + m->power_key_ignore_inhibited = false; + m->suspend_key_ignore_inhibited = false; + m->hibernate_key_ignore_inhibited = false; + m->lid_switch_ignore_inhibited = true; + + m->holdoff_timeout_usec = 30 * USEC_PER_SEC; + + m->idle_action_usec = 30 * USEC_PER_MINUTE; + m->idle_action = HANDLE_IGNORE; + + m->runtime_dir_size = physical_memory_scale(10U, 100U); /* 10% */ + m->user_tasks_max = system_tasks_max_scale(DEFAULT_USER_TASKS_MAX_PERCENTAGE, 100U); /* 33% */ + m->sessions_max = 8192; + m->inhibitors_max = 8192; + + m->kill_user_processes = KILL_USER_PROCESSES; + + m->kill_only_users = strv_free(m->kill_only_users); + m->kill_exclude_users = strv_free(m->kill_exclude_users); +} + +int manager_parse_config_file(Manager *m) { + assert(m); + + return config_parse_many_nulstr(PKGSYSCONFDIR "/logind.conf", + CONF_PATHS_NULSTR("systemd/logind.conf.d"), + "Login\0", + config_item_perf_lookup, logind_gperf_lookup, + CONFIG_PARSE_WARN, m); +} + int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_device) { Device *d; diff --git a/src/login/logind.c b/src/login/logind.c index 6bddcb541f..6ad6a695bc 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -33,38 +33,6 @@ static void manager_free(Manager *m); -static void manager_reset_config(Manager *m) { - m->n_autovts = 6; - m->reserve_vt = 6; - m->remove_ipc = true; - m->inhibit_delay_max = 5 * USEC_PER_SEC; - m->handle_power_key = HANDLE_POWEROFF; - m->handle_suspend_key = HANDLE_SUSPEND; - m->handle_hibernate_key = HANDLE_HIBERNATE; - m->handle_lid_switch = HANDLE_SUSPEND; - m->handle_lid_switch_ep = _HANDLE_ACTION_INVALID; - m->handle_lid_switch_docked = HANDLE_IGNORE; - m->power_key_ignore_inhibited = false; - m->suspend_key_ignore_inhibited = false; - m->hibernate_key_ignore_inhibited = false; - m->lid_switch_ignore_inhibited = true; - - m->holdoff_timeout_usec = 30 * USEC_PER_SEC; - - m->idle_action_usec = 30 * USEC_PER_MINUTE; - m->idle_action = HANDLE_IGNORE; - - m->runtime_dir_size = physical_memory_scale(10U, 100U); /* 10% */ - m->user_tasks_max = system_tasks_max_scale(DEFAULT_USER_TASKS_MAX_PERCENTAGE, 100U); /* 33% */ - m->sessions_max = 8192; - m->inhibitors_max = 8192; - - m->kill_user_processes = KILL_USER_PROCESSES; - - m->kill_only_users = strv_free(m->kill_only_users); - m->kill_exclude_users = strv_free(m->kill_exclude_users); -} - static Manager *manager_new(void) { Manager *m; int r; @@ -1091,16 +1059,6 @@ static int manager_dispatch_idle_action(sd_event_source *s, uint64_t t, void *us return 0; } -static int manager_parse_config_file(Manager *m) { - assert(m); - - return config_parse_many_nulstr(PKGSYSCONFDIR "/logind.conf", - CONF_PATHS_NULSTR("systemd/logind.conf.d"), - "Login\0", - config_item_perf_lookup, logind_gperf_lookup, - CONFIG_PARSE_WARN, m); -} - static int manager_dispatch_reload_signal(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) { Manager *m = userdata; int r; diff --git a/src/login/logind.h b/src/login/logind.h index 5574fcd591..be2963c256 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -126,6 +126,9 @@ struct Manager { uint64_t inhibitors_max; }; +void manager_reset_config(Manager *m); +int manager_parse_config_file(Manager *m); + int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_device); int manager_add_button(Manager *m, const char *name, Button **_button); int manager_add_seat(Manager *m, const char *id, Seat **_seat); diff --git a/src/login/meson.build b/src/login/meson.build index 94c9cc5293..a616d5b934 100644 --- a/src/login/meson.build +++ b/src/login/meson.build @@ -13,9 +13,6 @@ logind_gperf_c = custom_target( output : 'logind-gperf.c', command : [gperf, '@INPUT@', '--output-file', '@OUTPUT@']) -systemd_logind_sources += [logind_gperf_c] - - liblogind_core_sources = files(''' logind-core.c logind-device.c @@ -42,6 +39,8 @@ liblogind_core_sources = files(''' logind-acl.h '''.split()) +liblogind_core_sources += [logind_gperf_c] + logind_acl_c = files('logind-acl.c') if conf.get('HAVE_ACL') == 1 liblogind_core_sources += logind_acl_c From e1a7f622e7f9831c07294ad9ffc866d48b2b61a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 23 Apr 2018 17:26:03 +0200 Subject: [PATCH 3/7] man: fix description of %N in systemd.unit(5) The description in the man page disagreed with the code. Let the code win, since if anybody is using this, they are more likely to depend on actual behaviour rather than the docs. (In Fedora workstation installation there's only one use, and it doesn't make much sense either way: SyslogIdentifier=%N in xfs_scrub@.service.) Also adds dots at the end everywhere, because we have multiple sentences in some explanations, so we need dots. --- man/systemd.unit.xml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 1519ef3e6d..d66b6aad08 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -1543,28 +1543,28 @@ %N - Unescaped full unit name - Same as %n, but with escaping undone. This undoes the escaping used when generating unit names from arbitrary strings (see above). + Full unit name + Same as %n, but with the type suffix removed. %p Prefix name - For instantiated units, this refers to the string before the @ character of the unit name. For non-instantiated units, this refers to the name of the unit with the type suffix removed. + For instantiated units, this refers to the string before the first @ character of the unit name. For non-instantiated units, same as %N. %P Unescaped prefix name - Same as %p, but with escaping undone + Same as %p, but with escaping undone. %i Instance name - For instantiated units: this is the string between the @ character and the suffix of the unit name. + For instantiated units this is the string between the first @ character and the type suffix. Empty for non-instantiated units. %I Unescaped instance name - Same as %i, but with escaping undone + Same as %i, but with escaping undone. %f From 250e9fadbcc0ca90e697d7efb40855b054ed3b8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 23 Apr 2018 22:43:20 +0200 Subject: [PATCH 4/7] Add %j/%J unit specifiers Those are quite similar to %i/%I, but refer to the last dash-separated component of the name prefix. The new functionality of dash-dropins could largely supersede the template functionality, so it would be tempting to overload %i/%I. But that would not be backwards compatible. So let's add the two new letters instead. --- man/systemd.unit.xml | 12 +++++- src/core/unit-printf.c | 77 ++++++++++++++++++++++++++----------- src/shared/install-printf.c | 22 +++++++++++ src/test/test-unit-file.c | 1 + src/test/test-unit-name.c | 56 ++++++++++++++++++--------- 5 files changed, 127 insertions(+), 41 deletions(-) diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index d66b6aad08..02e0f499e8 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -1508,7 +1508,7 @@ The following specifiers are interpreted in the Install - section: %n, %N, %p, %i, %U, %u, %m, %H, %b, %v. For their meaning + section: %n, %N, %p, %i, %j, %U, %u, %m, %H, %b, %v. For their meaning see the next section. @@ -1566,6 +1566,16 @@ Unescaped instance name Same as %i, but with escaping undone. + + %j + Final component of the prefix + This is the string between the last - and the end of the prefix name. If there is no -, this is the same as %p. + + + %J + Unescaped final component of the prefix + Same as %j, but with escaping undone. + %f Unescaped filename diff --git a/src/core/unit-printf.c b/src/core/unit-printf.c index 1e260dd9bc..d2948ab6aa 100644 --- a/src/core/unit-printf.c +++ b/src/core/unit-printf.c @@ -55,6 +55,37 @@ static int specifier_instance_unescaped(char specifier, void *data, void *userda return unit_name_unescape(strempty(u->instance), ret); } +static int specifier_last_component(char specifier, void *data, void *userdata, char **ret) { + Unit *u = userdata; + _cleanup_free_ char *prefix = NULL; + char *dash; + int r; + + assert(u); + + r = unit_name_to_prefix(u->id, &prefix); + if (r < 0) + return r; + + dash = strrchr(prefix, '-'); + if (dash) + return specifier_string(specifier, dash + 1, userdata, ret); + + *ret = TAKE_PTR(prefix); + return 0; +} + +static int specifier_last_component_unescaped(char specifier, void *data, void *userdata, char **ret) { + _cleanup_free_ char *p = NULL; + int r; + + r = specifier_last_component(specifier, data, userdata, &p); + if (r < 0) + return r; + + return unit_name_unescape(p, ret); +} + static int specifier_filename(char specifier, void *data, void *userdata, char **ret) { Unit *u = userdata; @@ -213,31 +244,33 @@ int unit_full_printf(Unit *u, const char *format, char **ret) { */ const Specifier table[] = { - { 'n', specifier_string, u->id }, - { 'N', specifier_prefix_and_instance, NULL }, - { 'p', specifier_prefix, NULL }, - { 'P', specifier_prefix_unescaped, NULL }, - { 'i', specifier_string, u->instance }, - { 'I', specifier_instance_unescaped, NULL }, + { 'n', specifier_string, u->id }, + { 'N', specifier_prefix_and_instance, NULL }, + { 'p', specifier_prefix, NULL }, + { 'P', specifier_prefix_unescaped, NULL }, + { 'i', specifier_string, u->instance }, + { 'I', specifier_instance_unescaped, NULL }, + { 'j', specifier_last_component, NULL }, + { 'J', specifier_last_component_unescaped, NULL }, - { 'f', specifier_filename, NULL }, - { 'c', specifier_cgroup, NULL }, - { 'r', specifier_cgroup_slice, NULL }, - { 'R', specifier_cgroup_root, NULL }, - { 't', specifier_special_directory, UINT_TO_PTR(EXEC_DIRECTORY_RUNTIME) }, - { 'S', specifier_special_directory, UINT_TO_PTR(EXEC_DIRECTORY_STATE) }, - { 'C', specifier_special_directory, UINT_TO_PTR(EXEC_DIRECTORY_CACHE) }, - { 'L', specifier_special_directory, UINT_TO_PTR(EXEC_DIRECTORY_LOGS) }, + { 'f', specifier_filename, NULL }, + { 'c', specifier_cgroup, NULL }, + { 'r', specifier_cgroup_slice, NULL }, + { 'R', specifier_cgroup_root, NULL }, + { 't', specifier_special_directory, UINT_TO_PTR(EXEC_DIRECTORY_RUNTIME) }, + { 'S', specifier_special_directory, UINT_TO_PTR(EXEC_DIRECTORY_STATE) }, + { 'C', specifier_special_directory, UINT_TO_PTR(EXEC_DIRECTORY_CACHE) }, + { 'L', specifier_special_directory, UINT_TO_PTR(EXEC_DIRECTORY_LOGS) }, - { 'U', specifier_user_id, NULL }, - { 'u', specifier_user_name, NULL }, - { 'h', specifier_user_home, NULL }, - { 's', specifier_user_shell, NULL }, + { 'U', specifier_user_id, NULL }, + { 'u', specifier_user_name, NULL }, + { 'h', specifier_user_home, NULL }, + { 's', specifier_user_shell, NULL }, - { 'm', specifier_machine_id, NULL }, - { 'H', specifier_host_name, NULL }, - { 'b', specifier_boot_id, NULL }, - { 'v', specifier_kernel_release, NULL }, + { 'm', specifier_machine_id, NULL }, + { 'H', specifier_host_name, NULL }, + { 'b', specifier_boot_id, NULL }, + { 'v', specifier_kernel_release, NULL }, {} }; diff --git a/src/shared/install-printf.c b/src/shared/install-printf.c index c2cbf348e2..599300ceb4 100644 --- a/src/shared/install-printf.c +++ b/src/shared/install-printf.c @@ -88,6 +88,27 @@ static int specifier_instance(char specifier, void *data, void *userdata, char * return 0; } +static int specifier_last_component(char specifier, void *data, void *userdata, char **ret) { + _cleanup_free_ char *prefix = NULL; + char *dash; + int r; + + r = specifier_prefix(specifier, data, userdata, &prefix); + if (r < 0) + return r; + + dash = strrchr(prefix, '-'); + if (dash) { + dash = strdup(dash + 1); + if (!dash) + return -ENOMEM; + *ret = dash; + } else + *ret = TAKE_PTR(prefix); + + return 0; +} + int install_full_printf(UnitFileInstallInfo *i, const char *format, char **ret) { /* This is similar to unit_full_printf() but does not support @@ -111,6 +132,7 @@ int install_full_printf(UnitFileInstallInfo *i, const char *format, char **ret) { 'N', specifier_prefix_and_instance, NULL }, { 'p', specifier_prefix, NULL }, { 'i', specifier_instance, NULL }, + { 'j', specifier_last_component, NULL }, { 'U', specifier_user_id, NULL }, { 'u', specifier_user_name, NULL }, diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c index 6b72fc90fd..3fd4ac9e5d 100644 --- a/src/test/test-unit-file.c +++ b/src/test/test-unit-file.c @@ -662,6 +662,7 @@ static void test_install_printf(void) { expect(i, "%N", "name"); expect(i, "%p", "name"); expect(i, "%i", ""); + expect(i, "%j", "name"); expect(i, "%u", user); expect(i, "%U", uid); diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c index 347c586663..300a1c5a82 100644 --- a/src/test/test-unit-name.c +++ b/src/test/test-unit-name.c @@ -198,7 +198,7 @@ static void test_unit_name_mangle(void) { static int test_unit_printf(void) { _cleanup_free_ char *mid = NULL, *bid = NULL, *host = NULL, *uid = NULL, *user = NULL, *shell = NULL, *home = NULL; _cleanup_(manager_freep) Manager *m = NULL; - Unit *u, *u2; + Unit *u; int r; assert_se(specifier_machine_id('m', NULL, NULL, &mid) >= 0 && mid); @@ -245,6 +245,9 @@ static int test_unit_printf(void) { expect(u, "%p", "blah"); expect(u, "%P", "blah"); expect(u, "%i", ""); + expect(u, "%I", ""); + expect(u, "%j", "blah"); + expect(u, "%J", "blah"); expect(u, "%u", user); expect(u, "%U", uid); expect(u, "%h", home); @@ -254,24 +257,41 @@ static int test_unit_printf(void) { expect(u, "%t", "/run/user/*"); /* templated */ - assert_se(u2 = unit_new(m, sizeof(Service))); - assert_se(unit_add_name(u2, "blah@foo-foo.service") == 0); - assert_se(unit_add_name(u2, "blah@foo-foo.service") == 0); + assert_se(u = unit_new(m, sizeof(Service))); + assert_se(unit_add_name(u, "blah@foo-foo.service") == 0); + assert_se(unit_add_name(u, "blah@foo-foo.service") == 0); + + expect(u, "%n", "blah@foo-foo.service"); + expect(u, "%N", "blah@foo-foo"); + expect(u, "%f", "/foo/foo"); + expect(u, "%p", "blah"); + expect(u, "%P", "blah"); + expect(u, "%i", "foo-foo"); + expect(u, "%I", "foo/foo"); + expect(u, "%j", "blah"); + expect(u, "%J", "blah"); + expect(u, "%u", user); + expect(u, "%U", uid); + expect(u, "%h", home); + expect(u, "%m", mid); + expect(u, "%b", bid); + expect(u, "%H", host); + expect(u, "%t", "/run/user/*"); + + /* templated with components */ + assert_se(u = unit_new(m, sizeof(Slice))); + assert_se(unit_add_name(u, "blah-blah\\x2d.slice") == 0); + + expect(u, "%n", "blah-blah\\x2d.slice"); + expect(u, "%N", "blah-blah\\x2d"); + expect(u, "%f", "/blah/blah-"); + expect(u, "%p", "blah-blah\\x2d"); + expect(u, "%P", "blah/blah-"); + expect(u, "%i", ""); + expect(u, "%I", ""); + expect(u, "%j", "blah\\x2d"); + expect(u, "%J", "blah-"); - expect(u2, "%n", "blah@foo-foo.service"); - expect(u2, "%N", "blah@foo-foo"); - expect(u2, "%f", "/foo/foo"); - expect(u2, "%p", "blah"); - expect(u2, "%P", "blah"); - expect(u2, "%i", "foo-foo"); - expect(u2, "%I", "foo/foo"); - expect(u2, "%u", user); - expect(u2, "%U", uid); - expect(u2, "%h", home); - expect(u2, "%m", mid); - expect(u2, "%b", bid); - expect(u2, "%H", host); - expect(u2, "%t", "/run/user/*"); #undef expect return 0; From 284149392755f086d0a714071c33aa609e61505e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 7 Dec 2017 22:25:26 +0100 Subject: [PATCH 5/7] Use a dash-truncated drop-in for user-%j.slice configuration This removes the UserTasksMax= setting in logind.conf. Instead, the generic TasksMax= setting on the slice should be used. Instead of a transient unit we use a drop-in to tweak the default definition of a .slice. It's better to use the normal unit mechanisms instead of creating units on the fly. This will also make it easier to start user@.service independently of logind, or set additional settings like MemoryMax= for user slices. The setting in logind is removed, because otherwise we would have two sources of "truth": the slice on disk and the logind config. Instead of trying to coordinate those two sources of configuration (and maintainer overrides to both), let's just convert to the new one fully. Right now now automatic transition mechanism is provided. logind will emit a hint when it encounters the setting, but otherwise it will be ignored. Fixes #2556. --- man/logind.conf.xml | 11 ---- src/login/logind-dbus.c | 16 +++++- src/login/logind-gperf.gperf | 48 ++++++++--------- src/login/logind-user.c | 81 ++++------------------------ src/login/logind-user.h | 2 + src/login/logind.conf.in | 1 - src/login/logind.h | 1 - units/meson.build | 3 ++ units/user-.slice.d/10-defaults.conf | 15 ++++++ 9 files changed, 68 insertions(+), 110 deletions(-) create mode 100644 units/user-.slice.d/10-defaults.conf diff --git a/man/logind.conf.xml b/man/logind.conf.xml index 9e21c0b218..a35faf15c5 100644 --- a/man/logind.conf.xml +++ b/man/logind.conf.xml @@ -325,17 +325,6 @@ systemd-logind. - - UserTasksMax= - - Sets the maximum number of OS tasks each user may run concurrently. This controls the - TasksMax= setting of the per-user slice unit, see - systemd.resource-control5 - for details. If assigned the special value infinity, no tasks limit is applied. - Defaults to 33%, which equals 10813 with the kernel's defaults on the host, but might be smaller in - OS containers. - - RemoveIPC= diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index d92437b67f..7078e67dd4 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -332,6 +332,20 @@ static int property_get_current_inhibitors( return sd_bus_message_append(reply, "t", (uint64_t) hashmap_size(m->inhibitors)); } +static int property_get_compat_user_tasks_max( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + assert(reply); + + return sd_bus_message_append(reply, "t", CGROUP_LIMIT_MAX); +} + static int method_get_session(sd_bus_message *message, void *userdata, sd_bus_error *error) { _cleanup_free_ char *p = NULL; Manager *m = userdata; @@ -2720,7 +2734,7 @@ const sd_bus_vtable manager_vtable[] = { SD_BUS_PROPERTY("NCurrentInhibitors", "t", property_get_current_inhibitors, 0, 0), SD_BUS_PROPERTY("SessionsMax", "t", NULL, offsetof(Manager, sessions_max), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("NCurrentSessions", "t", property_get_current_sessions, 0, 0), - SD_BUS_PROPERTY("UserTasksMax", "t", NULL, offsetof(Manager, user_tasks_max), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("UserTasksMax", "t", property_get_compat_user_tasks_max, 0, SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_HIDDEN), SD_BUS_METHOD("GetSession", "s", "o", method_get_session, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("GetSessionByPID", "u", "o", method_get_session_by_pid, SD_BUS_VTABLE_UNPRIVILEGED), diff --git a/src/login/logind-gperf.gperf b/src/login/logind-gperf.gperf index f6f57526f6..c85339dcd3 100644 --- a/src/login/logind-gperf.gperf +++ b/src/login/logind-gperf.gperf @@ -17,27 +17,27 @@ struct ConfigPerfItem; %struct-type %includes %% -Login.NAutoVTs, config_parse_n_autovts, 0, offsetof(Manager, n_autovts) -Login.ReserveVT, config_parse_unsigned, 0, offsetof(Manager, reserve_vt) -Login.KillUserProcesses, config_parse_bool, 0, offsetof(Manager, kill_user_processes) -Login.KillOnlyUsers, config_parse_strv, 0, offsetof(Manager, kill_only_users) -Login.KillExcludeUsers, config_parse_strv, 0, offsetof(Manager, kill_exclude_users) -Login.InhibitDelayMaxSec, config_parse_sec, 0, offsetof(Manager, inhibit_delay_max) -Login.HandlePowerKey, config_parse_handle_action, 0, offsetof(Manager, handle_power_key) -Login.HandleSuspendKey, config_parse_handle_action, 0, offsetof(Manager, handle_suspend_key) -Login.HandleHibernateKey, config_parse_handle_action, 0, offsetof(Manager, handle_hibernate_key) -Login.HandleLidSwitch, config_parse_handle_action, 0, offsetof(Manager, handle_lid_switch) -Login.HandleLidSwitchExternalPower,config_parse_handle_action, 0, offsetof(Manager, handle_lid_switch_ep) -Login.HandleLidSwitchDocked, config_parse_handle_action, 0, offsetof(Manager, handle_lid_switch_docked) -Login.PowerKeyIgnoreInhibited, config_parse_bool, 0, offsetof(Manager, power_key_ignore_inhibited) -Login.SuspendKeyIgnoreInhibited, config_parse_bool, 0, offsetof(Manager, suspend_key_ignore_inhibited) -Login.HibernateKeyIgnoreInhibited, config_parse_bool, 0, offsetof(Manager, hibernate_key_ignore_inhibited) -Login.LidSwitchIgnoreInhibited, config_parse_bool, 0, offsetof(Manager, lid_switch_ignore_inhibited) -Login.HoldoffTimeoutSec, config_parse_sec, 0, offsetof(Manager, holdoff_timeout_usec) -Login.IdleAction, config_parse_handle_action, 0, offsetof(Manager, idle_action) -Login.IdleActionSec, config_parse_sec, 0, offsetof(Manager, idle_action_usec) -Login.RuntimeDirectorySize, config_parse_tmpfs_size, 0, offsetof(Manager, runtime_dir_size) -Login.RemoveIPC, config_parse_bool, 0, offsetof(Manager, remove_ipc) -Login.InhibitorsMax, config_parse_uint64, 0, offsetof(Manager, inhibitors_max) -Login.SessionsMax, config_parse_uint64, 0, offsetof(Manager, sessions_max) -Login.UserTasksMax, config_parse_user_tasks_max,0, offsetof(Manager, user_tasks_max) +Login.NAutoVTs, config_parse_n_autovts, 0, offsetof(Manager, n_autovts) +Login.ReserveVT, config_parse_unsigned, 0, offsetof(Manager, reserve_vt) +Login.KillUserProcesses, config_parse_bool, 0, offsetof(Manager, kill_user_processes) +Login.KillOnlyUsers, config_parse_strv, 0, offsetof(Manager, kill_only_users) +Login.KillExcludeUsers, config_parse_strv, 0, offsetof(Manager, kill_exclude_users) +Login.InhibitDelayMaxSec, config_parse_sec, 0, offsetof(Manager, inhibit_delay_max) +Login.HandlePowerKey, config_parse_handle_action, 0, offsetof(Manager, handle_power_key) +Login.HandleSuspendKey, config_parse_handle_action, 0, offsetof(Manager, handle_suspend_key) +Login.HandleHibernateKey, config_parse_handle_action, 0, offsetof(Manager, handle_hibernate_key) +Login.HandleLidSwitch, config_parse_handle_action, 0, offsetof(Manager, handle_lid_switch) +Login.HandleLidSwitchExternalPower, config_parse_handle_action, 0, offsetof(Manager, handle_lid_switch_ep) +Login.HandleLidSwitchDocked, config_parse_handle_action, 0, offsetof(Manager, handle_lid_switch_docked) +Login.PowerKeyIgnoreInhibited, config_parse_bool, 0, offsetof(Manager, power_key_ignore_inhibited) +Login.SuspendKeyIgnoreInhibited, config_parse_bool, 0, offsetof(Manager, suspend_key_ignore_inhibited) +Login.HibernateKeyIgnoreInhibited, config_parse_bool, 0, offsetof(Manager, hibernate_key_ignore_inhibited) +Login.LidSwitchIgnoreInhibited, config_parse_bool, 0, offsetof(Manager, lid_switch_ignore_inhibited) +Login.HoldoffTimeoutSec, config_parse_sec, 0, offsetof(Manager, holdoff_timeout_usec) +Login.IdleAction, config_parse_handle_action, 0, offsetof(Manager, idle_action) +Login.IdleActionSec, config_parse_sec, 0, offsetof(Manager, idle_action_usec) +Login.RuntimeDirectorySize, config_parse_tmpfs_size, 0, offsetof(Manager, runtime_dir_size) +Login.RemoveIPC, config_parse_bool, 0, offsetof(Manager, remove_ipc) +Login.InhibitorsMax, config_parse_uint64, 0, offsetof(Manager, inhibitors_max) +Login.SessionsMax, config_parse_uint64, 0, offsetof(Manager, sessions_max) +Login.UserTasksMax, config_parse_compat_user_tasks_max, 0, offsetof(Manager, user_tasks_max) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 379d8d0cdd..01b602dbc4 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -368,36 +368,6 @@ fail: return r; } -static int user_start_slice(User *u) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - const char *description; - char *job; - int r; - - assert(u); - - u->slice_job = mfree(u->slice_job); - description = strjoina("User Slice of ", u->name); - - r = manager_start_slice( - u->manager, - u->slice, - description, - "systemd-logind.service", - "systemd-user-sessions.service", - u->manager->user_tasks_max, - &error, - &job); - if (r >= 0) - u->slice_job = job; - else if (!sd_bus_error_has_name(&error, BUS_ERROR_UNIT_EXISTS)) - /* we don't fail due to this, let's try to continue */ - log_error_errno(r, "Failed to start user slice %s, ignoring: %s (%s)", - u->slice, bus_error_message(&error, r), error.name); - - return 0; -} - static int user_start_service(User *u) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; char *job; @@ -453,11 +423,6 @@ int user_start(User *u) { return r; } - /* Create cgroup */ - r = user_start_slice(u); - if (r < 0) - return r; - /* Save the user data so far, because pam_systemd will read the * XDG_RUNTIME_DIR out of it while starting up systemd --user. * We need to do user_save_internal() because we have not @@ -858,8 +823,8 @@ int config_parse_tmpfs_size( return 0; } -int config_parse_user_tasks_max( - const char* unit, +int config_parse_compat_user_tasks_max( + const char *unit, const char *filename, unsigned line, const char *section, @@ -870,45 +835,17 @@ int config_parse_user_tasks_max( void *data, void *userdata) { - uint64_t *m = data; - uint64_t k; - int r; - assert(filename); assert(lvalue); assert(rvalue); assert(data); - if (isempty(rvalue)) { - *m = system_tasks_max_scale(DEFAULT_USER_TASKS_MAX_PERCENTAGE, 100U); - return 0; - } - - if (streq(rvalue, "infinity")) { - *m = CGROUP_LIMIT_MAX; - return 0; - } - - /* Try to parse as percentage */ - r = parse_percent(rvalue); - if (r >= 0) - k = system_tasks_max_scale(r, 100U); - else { - - /* If the passed argument was not a percentage, or out of range, parse as byte size */ - - r = safe_atou64(rvalue, &k); - if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse tasks maximum, ignoring: %s", rvalue); - return 0; - } - } - - if (k <= 0 || k >= UINT64_MAX) { - log_syntax(unit, LOG_ERR, filename, line, 0, "Tasks maximum out of range, ignoring: %s", rvalue); - return 0; - } - - *m = k; + log_syntax(unit, LOG_NOTICE, filename, line, 0, + "Support for option %s= has been removed.", + lvalue); + log_info("Hint: try creating /etc/systemd/system/user-.slice/50-limits.conf with:\n" + " [Slice]\n" + " TasksMax=%s", + rvalue); return 0; } diff --git a/src/login/logind-user.h b/src/login/logind-user.h index d95059d5a9..18a80b3fdf 100644 --- a/src/login/logind-user.h +++ b/src/login/logind-user.h @@ -79,3 +79,5 @@ UserState user_state_from_string(const char *s) _pure_; int bus_user_method_terminate(sd_bus_message *message, void *userdata, sd_bus_error *error); int bus_user_method_kill(sd_bus_message *message, void *userdata, sd_bus_error *error); + +int config_parse_compat_user_tasks_max(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); diff --git a/src/login/logind.conf.in b/src/login/logind.conf.in index 40a77dc7be..1029e29bc7 100644 --- a/src/login/logind.conf.in +++ b/src/login/logind.conf.in @@ -35,4 +35,3 @@ #RemoveIPC=yes #InhibitorsMax=8192 #SessionsMax=8192 -#UserTasksMax=33% diff --git a/src/login/logind.h b/src/login/logind.h index be2963c256..7470516fba 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -182,7 +182,6 @@ int manager_set_lid_switch_ignore(Manager *m, usec_t until); int config_parse_n_autovts(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_tmpfs_size(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); -int config_parse_user_tasks_max(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int manager_get_session_from_creds(Manager *m, sd_bus_message *message, const char *name, sd_bus_error *error, Session **ret); int manager_get_user_from_creds(Manager *m, sd_bus_message *message, uid_t uid, sd_bus_error *error, User **ret); diff --git a/units/meson.build b/units/meson.build index 5e454ab3b1..1c89f36957 100644 --- a/units/meson.build +++ b/units/meson.build @@ -304,6 +304,9 @@ foreach tuple : units endif endforeach +install_data('user-.slice.d/10-defaults.conf', + install_dir : systemunitdir + '/user-.slice.d') + ############################################################ meson.add_install_script(meson_make_symlink, diff --git a/units/user-.slice.d/10-defaults.conf b/units/user-.slice.d/10-defaults.conf new file mode 100644 index 0000000000..95ab11b30b --- /dev/null +++ b/units/user-.slice.d/10-defaults.conf @@ -0,0 +1,15 @@ +# SPDX-License-Identifier: LGPL-2.1+ +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Unit] +Description=User Slice of UID %j +After=systemd-user-sessions.service + +[Slice] +TasksMax=33% From a9f0f5e50104fc5cc511aa10007f7f8b8366fed8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 9 Dec 2017 19:30:17 +0100 Subject: [PATCH 6/7] logind: split %t directory creation to a helper unit Unfortunately this needs a new binary to do the mount because there's just too many special steps to outsource this to systemd-mount: - EPERM needs to be treated specially - UserRuntimeDir= setting must be obeyed - SELinux label must be adjusted This allows user@.service to be started independently of logind. So 'systemctl start user@nnn' will start the user manager for user nnn. Logind will start it too when the user logs in, and will stop it (unless lingering is enabled) when the user logs out. Fixes #7339. --- meson.build | 8 ++ src/login/logind-user.c | 85 +-------------- src/login/meson.build | 5 + src/login/user-runtime-dir.c | 170 +++++++++++++++++++++++++++++ units/meson.build | 1 + units/user-runtime-dir@.service.in | 17 +++ units/user@.service.in | 2 + 7 files changed, 204 insertions(+), 84 deletions(-) create mode 100644 src/login/user-runtime-dir.c create mode 100644 units/user-runtime-dir@.service.in diff --git a/meson.build b/meson.build index 734a289755..426bd32991 100644 --- a/meson.build +++ b/meson.build @@ -1673,6 +1673,14 @@ if conf.get('ENABLE_LOGIND') == 1 endif endif +executable('systemd-user-runtime-dir', + user_runtime_dir_sources, + include_directories : includes, + link_with : [libshared, liblogind_core], + install_rpath : rootlibexecdir, + install : true, + install_dir : rootlibexecdir) + if conf.get('HAVE_PAM') == 1 executable('systemd-user-sessions', 'src/user-sessions/user-sessions.c', diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 01b602dbc4..712067c52c 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -319,55 +319,6 @@ int user_load(User *u) { return r; } -static int user_mkdir_runtime_path(User *u) { - int r; - - assert(u); - - 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"); - - if (path_is_mount_point(u->runtime_path, NULL, 0) <= 0) { - _cleanup_free_ char *t = NULL; - - r = asprintf(&t, "mode=0700,uid=" UID_FMT ",gid=" GID_FMT ",size=%zu%s", - u->uid, u->gid, u->manager->runtime_dir_size, - mac_smack_use() ? ",smackfsroot=*" : ""); - if (r < 0) - return log_oom(); - - (void) mkdir_label(u->runtime_path, 0700); - - r = mount("tmpfs", u->runtime_path, "tmpfs", MS_NODEV|MS_NOSUID, t); - if (r < 0) { - if (!IN_SET(errno, EPERM, EACCES)) { - r = log_error_errno(errno, "Failed to mount per-user tmpfs directory %s: %m", u->runtime_path); - goto fail; - } - - log_debug_errno(errno, "Failed to mount per-user tmpfs directory %s, assuming containerized execution, ignoring: %m", u->runtime_path); - - r = chmod_and_chown(u->runtime_path, 0700, u->uid, u->gid); - if (r < 0) { - log_error_errno(r, "Failed to change runtime directory ownership and mode: %m"); - goto fail; - } - } - - r = label_fix(u->runtime_path, 0); - if (r < 0) - log_warning_errno(r, "Failed to fix label of '%s', ignoring: %m", u->runtime_path); - } - - return 0; - -fail: - /* Try to clean up, but ignore errors */ - (void) rmdir(u->runtime_path); - return r; -} - static int user_start_service(User *u) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; char *job; @@ -414,15 +365,9 @@ int user_start(User *u) { */ u->stopping = false; - if (!u->started) { + if (!u->started) log_debug("Starting services for new user %s.", u->name); - /* Make XDG_RUNTIME_DIR */ - r = user_mkdir_runtime_path(u); - if (r < 0) - return r; - } - /* Save the user data so far, because pam_systemd will read the * XDG_RUNTIME_DIR out of it while starting up systemd --user. * We need to do user_save_internal() because we have not @@ -483,29 +428,6 @@ static int user_stop_service(User *u) { return r; } -static int user_remove_runtime_path(User *u) { - int r; - - assert(u); - - r = rm_rf(u->runtime_path, 0); - if (r < 0) - log_error_errno(r, "Failed to remove runtime directory %s (before unmounting): %m", u->runtime_path); - - /* Ignore cases where the directory isn't mounted, as that's - * quite possible, if we lacked the permissions to mount - * something */ - r = umount2(u->runtime_path, MNT_DETACH); - if (r < 0 && !IN_SET(errno, EINVAL, ENOENT)) - log_error_errno(errno, "Failed to unmount user runtime directory %s: %m", u->runtime_path); - - r = rm_rf(u->runtime_path, REMOVE_ROOT); - if (r < 0) - log_error_errno(r, "Failed to remove runtime directory %s (after unmounting): %m", u->runtime_path); - - return r; -} - int user_stop(User *u, bool force) { Session *s; int r = 0, k; @@ -555,11 +477,6 @@ int user_finalize(User *u) { r = k; } - /* Kill XDG_RUNTIME_DIR */ - k = user_remove_runtime_path(u); - if (k < 0) - r = k; - /* Clean SysV + POSIX IPC objects, but only if this is not a system user. Background: in many setups cronjobs * are run in full PAM and thus logind sessions, even if the code run doesn't belong to actual users but to * system components. Since enable RemoveIPC= globally for all users, we need to be a bit careful with such diff --git a/src/login/meson.build b/src/login/meson.build index a616d5b934..1e2cf49c43 100644 --- a/src/login/meson.build +++ b/src/login/meson.build @@ -58,6 +58,11 @@ loginctl_sources = files(''' sysfs-show.c '''.split()) +user_runtime_dir_sources = files(''' + user-runtime-dir.c + logind.h +'''.split()) + if conf.get('ENABLE_LOGIND') == 1 logind_conf = configure_file( input : 'logind.conf.in', diff --git a/src/login/user-runtime-dir.c b/src/login/user-runtime-dir.c new file mode 100644 index 0000000000..1bb26c99e4 --- /dev/null +++ b/src/login/user-runtime-dir.c @@ -0,0 +1,170 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include +#include + +#include "fs-util.h" +#include "label.h" +#include "logind.h" +#include "mkdir.h" +#include "mount-util.h" +#include "path-util.h" +#include "rm-rf.h" +#include "smack-util.h" +#include "stdio-util.h" +#include "string-util.h" +#include "strv.h" +#include "user-util.h" + +static int gather_configuration(size_t *runtime_dir_size) { + Manager m = {}; + int r; + + manager_reset_config(&m); + + r = manager_parse_config_file(&m); + if (r < 0) + log_warning_errno(r, "Failed to parse logind.conf: %m"); + + *runtime_dir_size = m.runtime_dir_size; + return 0; +} + +static int user_mkdir_runtime_path(const char *runtime_path, uid_t uid, gid_t gid, size_t runtime_dir_size) { + int r; + + assert(runtime_path); + assert(path_is_absolute(runtime_path)); + assert(uid_is_valid(uid)); + assert(gid_is_valid(gid)); + + 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"); + + if (path_is_mount_point(runtime_path, NULL, 0) >= 0) + log_debug("%s is already a mount point", runtime_path); + else { + char options[sizeof("mode=0700,uid=,gid=,size=,smackfsroot=*") + + DECIMAL_STR_MAX(uid_t) + + DECIMAL_STR_MAX(gid_t) + + DECIMAL_STR_MAX(size_t)]; + + xsprintf(options, + "mode=0700,uid=" UID_FMT ",gid=" GID_FMT ",size=%zu%s", + uid, gid, runtime_dir_size, + mac_smack_use() ? ",smackfsroot=*" : ""); + + (void) mkdir_label(runtime_path, 0700); + + r = mount("tmpfs", runtime_path, "tmpfs", MS_NODEV|MS_NOSUID, options); + if (r < 0) { + if (!IN_SET(errno, EPERM, EACCES)) { + r = log_error_errno(errno, "Failed to mount per-user tmpfs directory %s: %m", runtime_path); + goto fail; + } + + log_debug_errno(errno, "Failed to mount per-user tmpfs directory %s.\n" + "Assuming containerized execution, ignoring: %m", runtime_path); + + r = chmod_and_chown(runtime_path, 0700, uid, gid); + if (r < 0) { + log_error_errno(r, "Failed to change ownership and mode of \"%s\": %m", runtime_path); + goto fail; + } + } + + r = label_fix(runtime_path, 0); + if (r < 0) + log_warning_errno(r, "Failed to fix label of \"%s\", ignoring: %m", runtime_path); + } + + return 0; + +fail: + /* Try to clean up, but ignore errors */ + (void) rmdir(runtime_path); + return r; +} + +static int user_remove_runtime_path(const char *runtime_path) { + int r; + + assert(runtime_path); + assert(path_is_absolute(runtime_path)); + + r = rm_rf(runtime_path, 0); + if (r < 0) + log_error_errno(r, "Failed to remove runtime directory %s (before unmounting): %m", runtime_path); + + /* Ignore cases where the directory isn't mounted, as that's + * quite possible, if we lacked the permissions to mount + * something */ + r = umount2(runtime_path, MNT_DETACH); + if (r < 0 && !IN_SET(errno, EINVAL, ENOENT)) + log_error_errno(errno, "Failed to unmount user runtime directory %s: %m", runtime_path); + + r = rm_rf(runtime_path, REMOVE_ROOT); + if (r < 0) + log_error_errno(r, "Failed to remove runtime directory %s (after unmounting): %m", runtime_path); + + return r; +} + +static int do_mount(const char *runtime_path, uid_t uid, gid_t gid) { + size_t runtime_dir_size; + + assert_se(gather_configuration(&runtime_dir_size) == 0); + + log_debug("Will mount %s owned by "UID_FMT":"GID_FMT, runtime_path, uid, gid); + return user_mkdir_runtime_path(runtime_path, uid, gid, runtime_dir_size); +} + +static int do_umount(const char *runtime_path) { + log_debug("Will remove %s", runtime_path); + return user_remove_runtime_path(runtime_path); +} + +int main(int argc, char *argv[]) { + const char *user; + uid_t uid; + gid_t gid; + char runtime_path[sizeof("/run/user") + DECIMAL_STR_MAX(uid_t)]; + int r; + + log_parse_environment(); + log_open(); + + if (argc != 3) { + log_error("This program takes two arguments."); + return EXIT_FAILURE; + } + if (!STR_IN_SET(argv[1], "start", "stop")) { + log_error("First argument must be either \"start\" or \"stop\"."); + return EXIT_FAILURE; + } + + umask(0022); + + user = argv[2]; + r = get_user_creds(&user, &uid, &gid, NULL, NULL); + if (r < 0) { + log_error_errno(r, + r == -ESRCH ? "No such user \"%s\"" : + r == -ENOMSG ? "UID \"%s\" is invalid or has an invalid main group" + : "Failed to look up user \"%s\": %m", + user); + return EXIT_FAILURE; + } + + xsprintf(runtime_path, "/run/user/" UID_FMT, uid); + + if (streq(argv[1], "start")) + r = do_mount(runtime_path, uid, gid); + else if (streq(argv[1], "stop")) + r = do_umount(runtime_path); + else + assert_not_reached("Unknown verb!"); + + return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; +} diff --git a/units/meson.build b/units/meson.build index 1c89f36957..fed2f10753 100644 --- a/units/meson.build +++ b/units/meson.build @@ -218,6 +218,7 @@ in_units = [ 'multi-user.target.wants/'], ['systemd-vconsole-setup.service', 'ENABLE_VCONSOLE'], ['systemd-volatile-root.service', ''], + ['user-runtime-dir@.service', ''], ['user@.service', ''], ] diff --git a/units/user-runtime-dir@.service.in b/units/user-runtime-dir@.service.in new file mode 100644 index 0000000000..8c02beda3b --- /dev/null +++ b/units/user-runtime-dir@.service.in @@ -0,0 +1,17 @@ +# SPDX-License-Identifier: LGPL-2.1+ +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Unit] +Description=/run/user/%i mount wrapper +StopWhenUnneeded=yes + +[Service] +ExecStart=@rootlibexecdir@/systemd-user-runtime-dir start %i +ExecStop=@rootlibexecdir@/systemd-user-runtime-dir stop %i +RemainAfterExit=true diff --git a/units/user@.service.in b/units/user@.service.in index 372ffa56d3..b88108e1b7 100644 --- a/units/user@.service.in +++ b/units/user@.service.in @@ -10,6 +10,8 @@ [Unit] Description=User Manager for UID %i After=systemd-user-sessions.service +After=user-runtime-dir@%i.service +Requires=user-runtime-dir@%i.service [Service] User=%i From 79bb7cb3ffb1ea307cc8fc9b84f02dd971f52fcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 24 Apr 2018 09:41:34 +0200 Subject: [PATCH 7/7] logind: remove manager_start_slice() It is now unused. --- src/login/logind-dbus.c | 72 ----------------------------------------- src/login/logind.h | 1 - 2 files changed, 73 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 7078e67dd4..58b624f597 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2994,78 +2994,6 @@ static int strdup_job(sd_bus_message *reply, char **job) { return 1; } -int manager_start_slice( - Manager *manager, - const char *slice, - const char *description, - const char *after, - const char *after2, - uint64_t tasks_max, - sd_bus_error *error, - char **job) { - - _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL; - int r; - - assert(manager); - assert(slice); - assert(job); - - r = sd_bus_message_new_method_call( - manager->bus, - &m, - "org.freedesktop.systemd1", - "/org/freedesktop/systemd1", - "org.freedesktop.systemd1.Manager", - "StartTransientUnit"); - if (r < 0) - return r; - - r = sd_bus_message_append(m, "ss", strempty(slice), "fail"); - if (r < 0) - return r; - - r = sd_bus_message_open_container(m, 'a', "(sv)"); - if (r < 0) - return r; - - if (!isempty(description)) { - r = sd_bus_message_append(m, "(sv)", "Description", "s", description); - if (r < 0) - return r; - } - - if (!isempty(after)) { - r = sd_bus_message_append(m, "(sv)", "After", "as", 1, after); - if (r < 0) - return r; - } - - if (!isempty(after2)) { - r = sd_bus_message_append(m, "(sv)", "After", "as", 1, after2); - if (r < 0) - return r; - } - - r = sd_bus_message_append(m, "(sv)", "TasksMax", "t", tasks_max); - if (r < 0) - return r; - - r = sd_bus_message_close_container(m); - if (r < 0) - return r; - - r = sd_bus_message_append(m, "a(sa(sv))", 0); - if (r < 0) - return r; - - r = sd_bus_call(manager->bus, m, 0, error, &reply); - if (r < 0) - return r; - - return strdup_job(reply, job); -} - int manager_start_scope( Manager *manager, const char *scope, diff --git a/src/login/logind.h b/src/login/logind.h index 7470516fba..498a371361 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -166,7 +166,6 @@ int bus_manager_shutdown_or_sleep_now_or_later(Manager *m, const char *unit_name int manager_send_changed(Manager *manager, const char *property, ...) _sentinel_; -int manager_start_slice(Manager *manager, const char *slice, const char *description, const char *after, const char *after2, uint64_t tasks_max, sd_bus_error *error, char **job); int manager_start_scope(Manager *manager, const char *scope, pid_t pid, const char *slice, const char *description, const char *after, const char *after2, sd_bus_message *more_properties, sd_bus_error *error, char **job); int manager_start_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job); int manager_stop_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job);