From 05e8f270a9ca97e1014184df113494b143639fb8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jan 2016 19:00:56 +0100 Subject: [PATCH 1/9] machined: when the pool limit is set to infinity don't resize backing loopback file An unlimited quota makes a lot of sense, but we really should try to propagate this onto the loopback file size, since an infinitely sized file makes no sense. Fixes: #2314 #2253 --- src/machine/machined-dbus.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 28134f61bf..521043f6a3 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -833,11 +833,14 @@ static int method_set_pool_limit(sd_bus_message *message, void *userdata, sd_bus if (r < 0) return r; - r = btrfs_resize_loopback("/var/lib/machines", limit, false); - if (r == -ENOTTY) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Quota is only supported on btrfs."); - if (r < 0 && r != -ENODEV) /* ignore ENODEV, as that's what is returned if the file system is not on loopback */ - return sd_bus_error_set_errnof(error, r, "Failed to adjust loopback limit: %m"); + /* Resize the backing loopback device, if there is one, except if we asked to drop any limit */ + if (limit != (uint64_t) -1) { + r = btrfs_resize_loopback("/var/lib/machines", limit, false); + if (r == -ENOTTY) + return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Quota is only supported on btrfs."); + if (r < 0 && r != -ENODEV) /* ignore ENODEV, as that's what is returned if the file system is not on loopback */ + return sd_bus_error_set_errnof(error, r, "Failed to adjust loopback limit: %m"); + } (void) btrfs_qgroup_set_limit("/var/lib/machines", 0, limit); From a90fb858ac91de4c14c9b68da6060731954515b7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jan 2016 19:02:12 +0100 Subject: [PATCH 2/9] machined: add early checks for unrealistically large image/pool sizes --- src/basic/btrfs-util.c | 5 +++++ src/basic/io-util.h | 18 ++++++++++++++++++ src/machine/image-dbus.c | 3 +++ src/machine/machined-dbus.c | 3 +++ 4 files changed, 29 insertions(+) diff --git a/src/basic/btrfs-util.c b/src/basic/btrfs-util.c index d07d1df5a8..03c7609c92 100644 --- a/src/basic/btrfs-util.c +++ b/src/basic/btrfs-util.c @@ -43,6 +43,7 @@ #include "copy.h" #include "fd-util.h" #include "fileio.h" +#include "io-util.h" #include "macro.h" #include "missing.h" #include "path-util.h" @@ -913,6 +914,10 @@ int btrfs_resize_loopback_fd(int fd, uint64_t new_size, bool grow_only) { dev_t dev = 0; int r; + /* In contrast to btrfs quota ioctls ftruncate() cannot make sense of "infinity" or file sizes > 2^31 */ + if (!FILE_SIZE_VALID(new_size)) + return -EINVAL; + /* btrfs cannot handle file systems < 16M, hence use this as minimum */ if (new_size < 16*1024*1024) new_size = 16*1024*1024; diff --git a/src/basic/io-util.h b/src/basic/io-util.h index 5f77a556c0..7d0d2bd810 100644 --- a/src/basic/io-util.h +++ b/src/basic/io-util.h @@ -77,3 +77,21 @@ static inline size_t IOVEC_INCREMENT(struct iovec *i, unsigned n, size_t k) { return k; } + +static inline bool FILE_SIZE_VALID(uint64_t l) { + /* ftruncate() and friends take an unsigned file size, but actually cannot deal with file sizes larger than + * 2^63 since the kernel internally handles it as signed value. This call allows checking for this early. */ + + return (l >> 63) == 0; +} + +static inline bool FILE_SIZE_VALID_OR_INFINITY(uint64_t l) { + + /* Same as above, but allows one extra value: -1 as indication for infinity. */ + + if (l == (uint64_t) -1) + return true; + + return FILE_SIZE_VALID(l); + +} diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c index 4ec1766033..19388b016a 100644 --- a/src/machine/image-dbus.c +++ b/src/machine/image-dbus.c @@ -23,6 +23,7 @@ #include "bus-label.h" #include "bus-util.h" #include "image-dbus.h" +#include "io-util.h" #include "machine-image.h" #include "strv.h" #include "user-util.h" @@ -195,6 +196,8 @@ int bus_image_method_set_limit( r = sd_bus_message_read(message, "t", &limit); if (r < 0) return r; + if (!FILE_SIZE_VALID_OR_INFINITY(limit)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "New limit out of range"); r = bus_verify_polkit_async( message, diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 521043f6a3..6cb70af3aa 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -34,6 +34,7 @@ #include "formats-util.h" #include "hostname-util.h" #include "image-dbus.h" +#include "io-util.h" #include "machine-dbus.h" #include "machine-image.h" #include "machine-pool.h" @@ -813,6 +814,8 @@ static int method_set_pool_limit(sd_bus_message *message, void *userdata, sd_bus r = sd_bus_message_read(message, "t", &limit); if (r < 0) return r; + if (!FILE_SIZE_VALID_OR_INFINITY(limit)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "New limit out of range"); r = bus_verify_polkit_async( message, From 218685865a3a7457cb220d20b8f339618cd1d488 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jan 2016 19:48:29 +0100 Subject: [PATCH 3/9] tests: don't abbreviate function names needlessly THis is otherwise really hard to read... --- src/test/test-unit-name.c | 112 +++++++++++++++++++------------------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c index 842ca40102..a2ff8cc299 100644 --- a/src/test/test-unit-name.c +++ b/src/test/test-unit-name.c @@ -66,26 +66,26 @@ static void test_unit_name_is_valid(void) { assert_se(!unit_name_is_valid("@piep.service", UNIT_NAME_ANY)); } -static void test_u_n_r_i_one(const char *pattern, const char *repl, const char *expected, int ret) { +static void test_unit_name_replace_instance_one(const char *pattern, const char *repl, const char *expected, int ret) { _cleanup_free_ char *t = NULL; assert_se(unit_name_replace_instance(pattern, repl, &t) == ret); puts(strna(t)); assert_se(streq_ptr(t, expected)); } -static void test_u_n_r_i(void) { +static void test_unit_name_replace_instance(void) { puts("-------------------------------------------------"); - test_u_n_r_i_one("foo@.service", "waldo", "foo@waldo.service", 0); - test_u_n_r_i_one("foo@xyz.service", "waldo", "foo@waldo.service", 0); - test_u_n_r_i_one("xyz", "waldo", NULL, -EINVAL); - test_u_n_r_i_one("", "waldo", NULL, -EINVAL); - test_u_n_r_i_one("foo.service", "waldo", NULL, -EINVAL); - test_u_n_r_i_one(".service", "waldo", NULL, -EINVAL); - test_u_n_r_i_one("foo@", "waldo", NULL, -EINVAL); - test_u_n_r_i_one("@bar", "waldo", NULL, -EINVAL); + test_unit_name_replace_instance_one("foo@.service", "waldo", "foo@waldo.service", 0); + test_unit_name_replace_instance_one("foo@xyz.service", "waldo", "foo@waldo.service", 0); + test_unit_name_replace_instance_one("xyz", "waldo", NULL, -EINVAL); + test_unit_name_replace_instance_one("", "waldo", NULL, -EINVAL); + test_unit_name_replace_instance_one("foo.service", "waldo", NULL, -EINVAL); + test_unit_name_replace_instance_one(".service", "waldo", NULL, -EINVAL); + test_unit_name_replace_instance_one("foo@", "waldo", NULL, -EINVAL); + test_unit_name_replace_instance_one("@bar", "waldo", NULL, -EINVAL); } -static void test_u_n_f_p_one(const char *path, const char *suffix, const char *expected, int ret) { +static void test_unit_name_from_path_one(const char *path, const char *suffix, const char *expected, int ret) { _cleanup_free_ char *t = NULL; assert_se(unit_name_from_path(path, suffix, &t) == ret); @@ -100,19 +100,19 @@ static void test_u_n_f_p_one(const char *path, const char *suffix, const char *e } } -static void test_u_n_f_p(void) { +static void test_unit_name_from_path(void) { puts("-------------------------------------------------"); - test_u_n_f_p_one("/waldo", ".mount", "waldo.mount", 0); - test_u_n_f_p_one("/waldo/quuix", ".mount", "waldo-quuix.mount", 0); - test_u_n_f_p_one("/waldo/quuix/", ".mount", "waldo-quuix.mount", 0); - test_u_n_f_p_one("", ".mount", "-.mount", 0); - test_u_n_f_p_one("/", ".mount", "-.mount", 0); - test_u_n_f_p_one("///", ".mount", "-.mount", 0); - test_u_n_f_p_one("/foo/../bar", ".mount", NULL, -EINVAL); - test_u_n_f_p_one("/foo/./bar", ".mount", NULL, -EINVAL); + test_unit_name_from_path_one("/waldo", ".mount", "waldo.mount", 0); + test_unit_name_from_path_one("/waldo/quuix", ".mount", "waldo-quuix.mount", 0); + test_unit_name_from_path_one("/waldo/quuix/", ".mount", "waldo-quuix.mount", 0); + test_unit_name_from_path_one("", ".mount", "-.mount", 0); + test_unit_name_from_path_one("/", ".mount", "-.mount", 0); + test_unit_name_from_path_one("///", ".mount", "-.mount", 0); + test_unit_name_from_path_one("/foo/../bar", ".mount", NULL, -EINVAL); + test_unit_name_from_path_one("/foo/./bar", ".mount", NULL, -EINVAL); } -static void test_u_n_f_p_i_one(const char *pattern, const char *path, const char *suffix, const char *expected, int ret) { +static void test_unit_name_from_path_instance_one(const char *pattern, const char *path, const char *suffix, const char *expected, int ret) { _cleanup_free_ char *t = NULL; assert_se(unit_name_from_path_instance(pattern, path, suffix, &t) == ret); @@ -128,38 +128,38 @@ static void test_u_n_f_p_i_one(const char *pattern, const char *path, const char } } -static void test_u_n_f_p_i(void) { +static void test_unit_name_from_path_instance(void) { puts("-------------------------------------------------"); - test_u_n_f_p_i_one("waldo", "/waldo", ".mount", "waldo@waldo.mount", 0); - test_u_n_f_p_i_one("waldo", "/waldo////quuix////", ".mount", "waldo@waldo-quuix.mount", 0); - test_u_n_f_p_i_one("waldo", "/", ".mount", "waldo@-.mount", 0); - test_u_n_f_p_i_one("waldo", "", ".mount", "waldo@-.mount", 0); - test_u_n_f_p_i_one("waldo", "///", ".mount", "waldo@-.mount", 0); - test_u_n_f_p_i_one("waldo", "..", ".mount", NULL, -EINVAL); - test_u_n_f_p_i_one("waldo", "/foo", ".waldi", NULL, -EINVAL); - test_u_n_f_p_i_one("wa--ldo", "/--", ".mount", "wa--ldo@\\x2d\\x2d.mount", 0); + test_unit_name_from_path_instance_one("waldo", "/waldo", ".mount", "waldo@waldo.mount", 0); + test_unit_name_from_path_instance_one("waldo", "/waldo////quuix////", ".mount", "waldo@waldo-quuix.mount", 0); + test_unit_name_from_path_instance_one("waldo", "/", ".mount", "waldo@-.mount", 0); + test_unit_name_from_path_instance_one("waldo", "", ".mount", "waldo@-.mount", 0); + test_unit_name_from_path_instance_one("waldo", "///", ".mount", "waldo@-.mount", 0); + test_unit_name_from_path_instance_one("waldo", "..", ".mount", NULL, -EINVAL); + test_unit_name_from_path_instance_one("waldo", "/foo", ".waldi", NULL, -EINVAL); + test_unit_name_from_path_instance_one("wa--ldo", "/--", ".mount", "wa--ldo@\\x2d\\x2d.mount", 0); } -static void test_u_n_t_p_one(const char *unit, const char *path, int ret) { +static void test_unit_name_to_path_one(const char *unit, const char *path, int ret) { _cleanup_free_ char *p = NULL; assert_se(unit_name_to_path(unit, &p) == ret); assert_se(streq_ptr(path, p)); } -static void test_u_n_t_p(void) { - test_u_n_t_p_one("home.mount", "/home", 0); - test_u_n_t_p_one("home-lennart.mount", "/home/lennart", 0); - test_u_n_t_p_one("home-lennart-.mount", NULL, -EINVAL); - test_u_n_t_p_one("-home-lennart.mount", NULL, -EINVAL); - test_u_n_t_p_one("-home--lennart.mount", NULL, -EINVAL); - test_u_n_t_p_one("home-..-lennart.mount", NULL, -EINVAL); - test_u_n_t_p_one("", NULL, -EINVAL); - test_u_n_t_p_one("home/foo", NULL, -EINVAL); +static void test_unit_name_to_path(void) { + test_unit_name_to_path_one("home.mount", "/home", 0); + test_unit_name_to_path_one("home-lennart.mount", "/home/lennart", 0); + test_unit_name_to_path_one("home-lennart-.mount", NULL, -EINVAL); + test_unit_name_to_path_one("-home-lennart.mount", NULL, -EINVAL); + test_unit_name_to_path_one("-home--lennart.mount", NULL, -EINVAL); + test_unit_name_to_path_one("home-..-lennart.mount", NULL, -EINVAL); + test_unit_name_to_path_one("", NULL, -EINVAL); + test_unit_name_to_path_one("home/foo", NULL, -EINVAL); } -static void test_u_n_m_one(const char *pattern, const char *expect, int ret) { +static void test_unit_name_mangle_one(const char *pattern, const char *expect, int ret) { _cleanup_free_ char *t = NULL; assert_se(unit_name_mangle(pattern, UNIT_NAME_NOGLOB, &t) == ret); @@ -176,17 +176,17 @@ static void test_u_n_m_one(const char *pattern, const char *expect, int ret) { } } -static void test_u_n_m(void) { +static void test_unit_name_mangle(void) { puts("-------------------------------------------------"); - test_u_n_m_one("foo.service", "foo.service", 0); - test_u_n_m_one("/home", "home.mount", 1); - test_u_n_m_one("/dev/sda", "dev-sda.device", 1); - test_u_n_m_one("üxknürz.service", "\\xc3\\xbcxkn\\xc3\\xbcrz.service", 1); - test_u_n_m_one("foobar-meh...waldi.service", "foobar-meh...waldi.service", 0); - test_u_n_m_one("_____####----.....service", "_____\\x23\\x23\\x23\\x23----.....service", 1); - test_u_n_m_one("_____##@;;;,,,##----.....service", "_____\\x23\\x23@\\x3b\\x3b\\x3b\\x2c\\x2c\\x2c\\x23\\x23----.....service", 1); - test_u_n_m_one("xxx@@@@/////\\\\\\\\\\yyy.service", "xxx@@@@-----\\\\\\\\\\yyy.service", 1); - test_u_n_m_one("", NULL, -EINVAL); + test_unit_name_mangle_one("foo.service", "foo.service", 0); + test_unit_name_mangle_one("/home", "home.mount", 1); + test_unit_name_mangle_one("/dev/sda", "dev-sda.device", 1); + test_unit_name_mangle_one("üxknürz.service", "\\xc3\\xbcxkn\\xc3\\xbcrz.service", 1); + test_unit_name_mangle_one("foobar-meh...waldi.service", "foobar-meh...waldi.service", 0); + test_unit_name_mangle_one("_____####----.....service", "_____\\x23\\x23\\x23\\x23----.....service", 1); + test_unit_name_mangle_one("_____##@;;;,,,##----.....service", "_____\\x23\\x23@\\x3b\\x3b\\x3b\\x2c\\x2c\\x2c\\x23\\x23----.....service", 1); + test_unit_name_mangle_one("xxx@@@@/////\\\\\\\\\\yyy.service", "xxx@@@@-----\\\\\\\\\\yyy.service", 1); + test_unit_name_mangle_one("", NULL, -EINVAL); } static int test_unit_printf(void) { @@ -460,11 +460,11 @@ static void test_unit_name_path_unescape(void) { int main(int argc, char* argv[]) { int rc = 0; test_unit_name_is_valid(); - test_u_n_r_i(); - test_u_n_f_p(); - test_u_n_f_p_i(); - test_u_n_m(); - test_u_n_t_p(); + test_unit_name_replace_instance(); + test_unit_name_from_path(); + test_unit_name_from_path_instance(); + test_unit_name_mangle(); + test_unit_name_to_path(); TEST_REQ_RUNNING_SYSTEMD(rc = test_unit_printf()); test_unit_instance_is_valid(); test_unit_prefix_is_valid(); From 1b53f64b001d2a8acb398eb8d546ec4570c1f235 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jan 2016 19:49:08 +0100 Subject: [PATCH 4/9] systemctl: piece-meal strv extension is expensive If we have many entries to add to an strv we really should try to be smarter than constantly realloc()ing the strv array. Instead, grow it exponentially. --- src/systemctl/systemctl.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 94c99c4d91..73f5710b9c 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2658,14 +2658,25 @@ static int expand_names(sd_bus *bus, char **names, const char* suffix, char ***r if (!strv_isempty(globs)) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_free_ UnitInfo *unit_infos = NULL; + size_t allocated, n; r = get_unit_list(bus, NULL, globs, &unit_infos, 0, &reply); if (r < 0) return r; - for (i = 0; i < r; i++) - if (strv_extend(&mangled, unit_infos[i].id) < 0) + n = strv_length(mangled); + allocated = n + 1; + + for (i = 0; i < r; i++) { + if (!GREEDY_REALLOC(mangled, allocated, n+2)) return log_oom(); + + mangled[n] = strdup(unit_infos[i].id); + if (!mangled[n]) + return log_oom(); + + mangled[++n] = NULL; + } } *ret = mangled; From 1f00ededc7451933e23a95597804897b37fa88d6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jan 2016 19:50:04 +0100 Subject: [PATCH 5/9] man: document that unit file globbing only operates on primary unit names See: #2397 --- man/systemctl.xml | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/man/systemctl.xml b/man/systemctl.xml index a55e06059a..7d98c73fe1 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -683,14 +683,11 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service Start (activate) one or more units specified on the command line. - Note that glob patterns operate on a list of currently - loaded units. Units which are not active and are not in a - failed state usually are not loaded, and would not be - matched by any pattern. In addition, in case of - instantiated units, systemd is often unaware of the - instance name until the instance has been started. Therefore, - using glob patterns with start - has limited usefulness. + Note that glob patterns operate on the set of primary names of currently loaded units. Units which + are not active and are not in a failed state usually are not loaded, and will not be matched by any + pattern. In addition, in case of instantiated units, systemd is often unaware of the instance name until + the instance has been started. Therefore, using glob patterns with start has limited + usefulness. Also, secondary alias names of units are not considered. @@ -1724,11 +1721,10 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service are equivalent to: # systemctl status dev-sda.device # systemctl status home.mount - In the second case, shell-style globs will be matched against - currently loaded units; literal unit names, with or without - a suffix, will be treated as in the first case. This means that - literal unit names always refer to exactly one unit, but globs - may match zero units and this is not considered an error. + In the second case, shell-style globs will be matched against the primary names of all currently loaded units; + literal unit names, with or without a suffix, will be treated as in the first case. This means that literal unit + names always refer to exactly one unit, but globs may match zero units and this is not considered an + error. Glob patterns use fnmatch3, @@ -1736,11 +1732,12 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service *, ?, [] may be used. See glob7 - for more details. The patterns are matched against the names of + for more details. The patterns are matched against the primary names of currently loaded units, and patterns which do not match anything are silently skipped. For example: # systemctl stop sshd@*.service - will stop all sshd@.service instances. + will stop all sshd@.service instances. Note that alias names of units, and units that aren't + loaded are not considered for glob expansion. For unit file commands, the specified From 2aaafcf57048983b2b76d6325f333e50aca4a3a3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jan 2016 20:25:10 +0100 Subject: [PATCH 6/9] basic: don't append suffixes to unit name glob expressions When the user specifies "foo*" as unit name glob expression, we shouldn't turn this into "foo*.service". Hence: only append a suffix if the specified string isn't a glob expression. Fixes: #2397 --- src/basic/unit-name.c | 45 +++++++++++++++++++++++++++++---------- src/test/test-unit-name.c | 33 +++++++++++++++++----------- 2 files changed, 54 insertions(+), 24 deletions(-) diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index 5fc3b9d6fd..d4a3062658 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -27,6 +27,7 @@ #include "alloc-util.h" #include "bus-label.h" +#include "glob-util.h" #include "hexdecoct.h" #include "macro.h" #include "path-util.h" @@ -35,10 +36,22 @@ #include "strv.h" #include "unit-name.h" +/* Characters valid in a unit name. */ #define VALID_CHARS \ - DIGITS LETTERS \ + DIGITS \ + LETTERS \ ":-_.\\" +/* The same, but also permits the single @ character that may appear */ +#define VALID_CHARS_WITH_AT \ + "@" \ + VALID_CHARS + +/* All chars valid in a unit name glob */ +#define VALID_CHARS_GLOB \ + VALID_CHARS_WITH_AT \ + "[]!-*?" + bool unit_name_is_valid(const char *n, UnitNameFlags flags) { const char *e, *i, *at; @@ -637,7 +650,7 @@ static char *do_escape_mangle(const char *f, UnitNameMangle allow_globs, char *t /* We'll only escape the obvious characters here, to play * safe. */ - valid_chars = allow_globs == UNIT_NAME_GLOB ? "@" VALID_CHARS "[]!-*?" : "@" VALID_CHARS; + valid_chars = allow_globs == UNIT_NAME_GLOB ? VALID_CHARS_GLOB : VALID_CHARS_WITH_AT; for (; *f; f++) { if (*f == '/') @@ -672,15 +685,15 @@ int unit_name_mangle_with_suffix(const char *name, UnitNameMangle allow_globs, c if (!unit_suffix_is_valid(suffix)) return -EINVAL; - if (unit_name_is_valid(name, UNIT_NAME_ANY)) { - /* No mangling necessary... */ - s = strdup(name); - if (!s) - return -ENOMEM; + /* Already a fully valid unit name? If so, no mangling is necessary... */ + if (unit_name_is_valid(name, UNIT_NAME_ANY)) + goto good; - *ret = s; - return 0; - } + /* Already a fully valid globbing expression? If so, no mangling is necessary either... */ + if (allow_globs == UNIT_NAME_GLOB && + string_is_glob(name) && + in_charset(name, VALID_CHARS_GLOB)) + goto good; if (is_device_path(name)) { r = unit_name_from_path(name, ".device", ret); @@ -705,11 +718,21 @@ int unit_name_mangle_with_suffix(const char *name, UnitNameMangle allow_globs, c t = do_escape_mangle(name, allow_globs, s); *t = 0; - if (unit_name_to_type(s) < 0) + /* Append a suffix if it doesn't have any, but only if this is not a glob, so that we can allow "foo.*" as a + * valid glob. */ + if ((allow_globs != UNIT_NAME_GLOB || !string_is_glob(s)) && unit_name_to_type(s) < 0) strcpy(t, suffix); *ret = s; return 1; + +good: + s = strdup(name); + if (!s) + return -ENOMEM; + + *ret = s; + return 0; } int slice_build_parent_slice(const char *slice, char **ret) { diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c index a2ff8cc299..5287ee5e6f 100644 --- a/src/test/test-unit-name.c +++ b/src/test/test-unit-name.c @@ -27,6 +27,7 @@ #include #include "alloc-util.h" +#include "glob-util.h" #include "hostname-util.h" #include "macro.h" #include "manager.h" @@ -159,34 +160,40 @@ static void test_unit_name_to_path(void) { test_unit_name_to_path_one("home/foo", NULL, -EINVAL); } -static void test_unit_name_mangle_one(const char *pattern, const char *expect, int ret) { +static void test_unit_name_mangle_one(UnitNameMangle allow_globs, const char *pattern, const char *expect, int ret) { _cleanup_free_ char *t = NULL; - assert_se(unit_name_mangle(pattern, UNIT_NAME_NOGLOB, &t) == ret); + assert_se(unit_name_mangle(pattern, allow_globs, &t) == ret); puts(strna(t)); assert_se(streq_ptr(t, expect)); if (t) { _cleanup_free_ char *k = NULL; - assert_se(unit_name_is_valid(t, UNIT_NAME_ANY)); + assert_se(unit_name_is_valid(t, UNIT_NAME_ANY) || + (allow_globs == UNIT_NAME_GLOB && string_is_glob(t))); - assert_se(unit_name_mangle(t, UNIT_NAME_NOGLOB, &k) == 0); + assert_se(unit_name_mangle(t, allow_globs, &k) == 0); assert_se(streq_ptr(t, k)); } } static void test_unit_name_mangle(void) { puts("-------------------------------------------------"); - test_unit_name_mangle_one("foo.service", "foo.service", 0); - test_unit_name_mangle_one("/home", "home.mount", 1); - test_unit_name_mangle_one("/dev/sda", "dev-sda.device", 1); - test_unit_name_mangle_one("üxknürz.service", "\\xc3\\xbcxkn\\xc3\\xbcrz.service", 1); - test_unit_name_mangle_one("foobar-meh...waldi.service", "foobar-meh...waldi.service", 0); - test_unit_name_mangle_one("_____####----.....service", "_____\\x23\\x23\\x23\\x23----.....service", 1); - test_unit_name_mangle_one("_____##@;;;,,,##----.....service", "_____\\x23\\x23@\\x3b\\x3b\\x3b\\x2c\\x2c\\x2c\\x23\\x23----.....service", 1); - test_unit_name_mangle_one("xxx@@@@/////\\\\\\\\\\yyy.service", "xxx@@@@-----\\\\\\\\\\yyy.service", 1); - test_unit_name_mangle_one("", NULL, -EINVAL); + test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "foo.service", "foo.service", 0); + test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "/home", "home.mount", 1); + test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "/dev/sda", "dev-sda.device", 1); + test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "üxknürz.service", "\\xc3\\xbcxkn\\xc3\\xbcrz.service", 1); + test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "foobar-meh...waldi.service", "foobar-meh...waldi.service", 0); + test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "_____####----.....service", "_____\\x23\\x23\\x23\\x23----.....service", 1); + test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "_____##@;;;,,,##----.....service", "_____\\x23\\x23@\\x3b\\x3b\\x3b\\x2c\\x2c\\x2c\\x23\\x23----.....service", 1); + test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "xxx@@@@/////\\\\\\\\\\yyy.service", "xxx@@@@-----\\\\\\\\\\yyy.service", 1); + test_unit_name_mangle_one(UNIT_NAME_NOGLOB, "", NULL, -EINVAL); + + test_unit_name_mangle_one(UNIT_NAME_GLOB, "foo.service", "foo.service", 0); + test_unit_name_mangle_one(UNIT_NAME_GLOB, "foo", "foo.service", 1); + test_unit_name_mangle_one(UNIT_NAME_GLOB, "foo*", "foo*", 0); + test_unit_name_mangle_one(UNIT_NAME_GLOB, "ü*", "\\xc3\\xbc*", 1); } static int test_unit_printf(void) { From fccd4b67b5fd296cb5840d1b8e0ea8455cb6a1ed Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jan 2016 20:36:54 +0100 Subject: [PATCH 7/9] man: document slices.target Fixes: #2438 --- man/systemd.special.xml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/man/systemd.special.xml b/man/systemd.special.xml index d28f3d5f90..0a37f65956 100644 --- a/man/systemd.special.xml +++ b/man/systemd.special.xml @@ -92,6 +92,7 @@ shutdown.target, sigpwr.target, sleep.target, + slices.target, smartcard.target, sockets.target, sound.target, @@ -502,11 +503,24 @@ hook units into the sleep state logic. + + slices.target + + A special target unit that sets up all slice units (see + systemd.slice5 for + details) that shall be active after boot. By default the generic user.slice, + system.slice, machines.slice slice units, as well as the the root + slice unit -.slice are pulled in and ordered before this unit (see below). + + It's a good idea to add WantedBy=slices.target lines to the [Install] + section of all slices units that may be installed dynamically. + + sockets.target A special target unit that sets up all socket - units.(see + units (see systemd.socket5 for details) that shall be active after boot. From 0c5eb0562abec6f845f07c30b2ad2515900ec1e5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jan 2016 22:34:46 +0100 Subject: [PATCH 8/9] nss: block various signals while running NSS lookups Let's make sure our poll() calls don't get interrupted where they shouldn't (SIGALRM, ...), but allow them to be interrupted where they should (SIGINT, ...). Fixes #1965 --- .gitignore | 1 + Makefile.am | 9 +++++- src/basic/nss-util.h | 2 ++ src/basic/signal-util.h | 11 +++++++ src/nss-myhostname/nss-myhostname.c | 7 +++++ src/nss-mymachines/nss-mymachines.c | 13 ++++++++ src/nss-resolve/nss-resolve.c | 7 +++++ src/test/test-signal-util.c | 49 +++++++++++++++++++++++++++++ 8 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 src/test/test-signal-util.c diff --git a/.gitignore b/.gitignore index c045ea4378..cc2cd22057 100644 --- a/.gitignore +++ b/.gitignore @@ -255,6 +255,7 @@ /test-sched-prio /test-set /test-sigbus +/test-signal-util /test-siphash24 /test-sleep /test-socket-util diff --git a/Makefile.am b/Makefile.am index 73d2375bf9..90bc5d7ddc 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1500,7 +1500,8 @@ tests += \ test-arphrd-list \ test-dns-domain \ test-install-root \ - test-rlimit-util + test-rlimit-util \ + test-signal-util if HAVE_ACL tests += \ @@ -1881,6 +1882,12 @@ test_ask_password_api_SOURCES = \ test_ask_password_api_LDADD = \ libshared.la +test_signal_util_SOURCES = \ + src/test/test-signal-util.c + +test_signal_util_LDADD = \ + libshared.la + BUILT_SOURCES += \ src/test/test-hashmap-ordered.c diff --git a/src/basic/nss-util.h b/src/basic/nss-util.h index cc30d93aad..4be0136da6 100644 --- a/src/basic/nss-util.h +++ b/src/basic/nss-util.h @@ -27,6 +27,8 @@ #include #include +#define NSS_SIGNALS_BLOCK SIGALRM,SIGVTALRM,SIGPIPE,SIGCHLD,SIGTSTP,SIGIO,SIGHUP,SIGUSR1,SIGUSR2,SIGPROF,SIGURG,SIGWINCH + #define NSS_GETHOSTBYNAME_PROTOTYPES(module) \ enum nss_status _nss_##module##_gethostbyname4_r( \ const char *name, \ diff --git a/src/basic/signal-util.h b/src/basic/signal-util.h index e7393e2dac..5d94d1c363 100644 --- a/src/basic/signal-util.h +++ b/src/basic/signal-util.h @@ -41,3 +41,14 @@ int signal_from_string(const char *s) _pure_; int signal_from_string_try_harder(const char *s); void nop_signal_handler(int sig); + +static inline void block_signals_reset(sigset_t *ss) { + assert_se(sigprocmask(SIG_SETMASK, ss, NULL) >= 0); +} + +#define BLOCK_SIGNALS(...) \ + _cleanup_(block_signals_reset) sigset_t _saved_sigset = ({ \ + sigset_t t; \ + assert_se(sigprocmask_many(SIG_BLOCK, &t, __VA_ARGS__, -1) >= 0); \ + t; \ + }) diff --git a/src/nss-myhostname/nss-myhostname.c b/src/nss-myhostname/nss-myhostname.c index ee10b105ea..e438625814 100644 --- a/src/nss-myhostname/nss-myhostname.c +++ b/src/nss-myhostname/nss-myhostname.c @@ -31,6 +31,7 @@ #include "local-addresses.h" #include "macro.h" #include "nss-util.h" +#include "signal-util.h" #include "string-util.h" #include "util.h" @@ -63,6 +64,8 @@ enum nss_status _nss_myhostname_gethostbyname4_r( char *r_name; unsigned n; + BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); + assert(name); assert(pat); assert(buffer); @@ -327,6 +330,8 @@ enum nss_status _nss_myhostname_gethostbyname3_r( uint32_t local_address_ipv4 = 0; int n_addresses = 0; + BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); + assert(name); assert(host); assert(buffer); @@ -409,6 +414,8 @@ enum nss_status _nss_myhostname_gethostbyaddr2_r( bool additional_from_hostname = false; unsigned n; + BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); + assert(addr); assert(host); assert(buffer); diff --git a/src/nss-mymachines/nss-mymachines.c b/src/nss-mymachines/nss-mymachines.c index dcdbc31a78..3cd29500d0 100644 --- a/src/nss-mymachines/nss-mymachines.c +++ b/src/nss-mymachines/nss-mymachines.c @@ -31,6 +31,7 @@ #include "in-addr-util.h" #include "macro.h" #include "nss-util.h" +#include "signal-util.h" #include "string-util.h" #include "user-util.h" #include "util.h" @@ -94,6 +95,8 @@ enum nss_status _nss_mymachines_gethostbyname4_r( char *r_name; int n_ifindices, r; + BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); + assert(name); assert(pat); assert(buffer); @@ -242,6 +245,8 @@ enum nss_status _nss_mymachines_gethostbyname3_r( size_t l, idx, ms, alen; int r; + BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); + assert(name); assert(result); assert(buffer); @@ -404,6 +409,8 @@ enum nss_status _nss_mymachines_getpwnam_r( size_t l; int r; + BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); + assert(name); assert(pwd); @@ -491,6 +498,8 @@ enum nss_status _nss_mymachines_getpwuid_r( uint32_t mapped; int r; + BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); + if (!uid_is_valid(uid)) { r = -EINVAL; goto fail; @@ -564,6 +573,8 @@ enum nss_status _nss_mymachines_getgrnam_r( size_t l; int r; + BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); + assert(name); assert(gr); @@ -649,6 +660,8 @@ enum nss_status _nss_mymachines_getgrgid_r( uint32_t mapped; int r; + BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); + if (!gid_is_valid(gid)) { r = -EINVAL; goto fail; diff --git a/src/nss-resolve/nss-resolve.c b/src/nss-resolve/nss-resolve.c index 2b716a69d1..7864e5370b 100644 --- a/src/nss-resolve/nss-resolve.c +++ b/src/nss-resolve/nss-resolve.c @@ -34,6 +34,7 @@ #include "nss-util.h" #include "string-util.h" #include "util.h" +#include "signal-util.h" NSS_GETHOSTBYNAME_PROTOTYPES(resolve); NSS_GETHOSTBYADDR_PROTOTYPES(resolve); @@ -127,6 +128,8 @@ enum nss_status _nss_resolve_gethostbyname4_r( char *r_name; int c, r, i = 0; + BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); + assert(name); assert(pat); assert(buffer); @@ -307,6 +310,8 @@ enum nss_status _nss_resolve_gethostbyname3_r( const char *canonical; int c, r, i = 0; + BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); + assert(name); assert(result); assert(buffer); @@ -512,6 +517,8 @@ enum nss_status _nss_resolve_gethostbyaddr2_r( const char *n; int r, ifindex; + BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); + assert(addr); assert(result); assert(buffer); diff --git a/src/test/test-signal-util.c b/src/test/test-signal-util.c new file mode 100644 index 0000000000..3083501ce9 --- /dev/null +++ b/src/test/test-signal-util.c @@ -0,0 +1,49 @@ +/*** + This file is part of systemd. + + Copyright 2016 Lennart Poettering + + 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. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include "signal-util.h" + +static void test_block_signals(void) { + sigset_t ss; + + assert_se(sigprocmask(0, NULL, &ss) >= 0); + + assert_se(sigismember(&ss, SIGUSR1) == 0); + assert_se(sigismember(&ss, SIGALRM) == 0); + assert_se(sigismember(&ss, SIGVTALRM) == 0); + + { + BLOCK_SIGNALS(SIGUSR1, SIGVTALRM); + + assert_se(sigprocmask(0, NULL, &ss) >= 0); + assert_se(sigismember(&ss, SIGUSR1) == 1); + assert_se(sigismember(&ss, SIGALRM) == 0); + assert_se(sigismember(&ss, SIGVTALRM) == 1); + + } + + assert_se(sigprocmask(0, NULL, &ss) >= 0); + assert_se(sigismember(&ss, SIGUSR1) == 0); + assert_se(sigismember(&ss, SIGALRM) == 0); + assert_se(sigismember(&ss, SIGVTALRM) == 0); +} + +int main(int argc, char *argv[]) { + test_block_signals(); +} From a464cf80110f0c7424f688ffaa4ec0a8a19f9720 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Jan 2016 23:40:59 +0100 Subject: [PATCH 9/9] nss-resolve: also fall back to nss-dns if dbus doesn't work Fixes #1692 --- src/nss-resolve/nss-resolve.c | 148 +++++++++++++++++----------------- 1 file changed, 75 insertions(+), 73 deletions(-) diff --git a/src/nss-resolve/nss-resolve.c b/src/nss-resolve/nss-resolve.c index 7864e5370b..85649f67dc 100644 --- a/src/nss-resolve/nss-resolve.c +++ b/src/nss-resolve/nss-resolve.c @@ -119,6 +119,13 @@ enum nss_status _nss_resolve_gethostbyname4_r( int *errnop, int *h_errnop, int32_t *ttlp) { + enum nss_status (*fallback)( + const char *name, + struct gaih_addrtuple **pat, + char *buffer, size_t buflen, + int *errnop, int *h_errnop, + int32_t *ttlp); + _cleanup_(sd_bus_message_unrefp) sd_bus_message *req = NULL, *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; struct gaih_addrtuple *r_tuple, *r_tuple_first = NULL; @@ -138,7 +145,7 @@ enum nss_status _nss_resolve_gethostbyname4_r( r = sd_bus_open_system(&bus); if (r < 0) - goto fail; + goto fallback; r = sd_bus_message_new_method_call( bus, @@ -166,28 +173,10 @@ enum nss_status _nss_resolve_gethostbyname4_r( return NSS_STATUS_NOTFOUND; } - if (bus_error_shall_fallback(&error)) { + if (bus_error_shall_fallback(&error)) + goto fallback; - enum nss_status (*fallback)( - const char *name, - struct gaih_addrtuple **pat, - char *buffer, size_t buflen, - int *errnop, int *h_errnop, - int32_t *ttlp); - - fallback = (enum nss_status (*)(const char *name, - struct gaih_addrtuple **pat, - char *buffer, size_t buflen, - int *errnop, int *h_errnop, - int32_t *ttlp)) - find_fallback("libnss_dns.so.2", "_nss_dns_gethostbyname4_r"); - if (fallback) - return fallback(name, pat, buffer, buflen, errnop, h_errnop, ttlp); - } - - *errnop = -r; - *h_errnop = NO_RECOVERY; - return NSS_STATUS_UNAVAIL; + goto fail; } c = count_addresses(reply, AF_UNSPEC, &canonical); @@ -287,9 +276,20 @@ enum nss_status _nss_resolve_gethostbyname4_r( return NSS_STATUS_SUCCESS; +fallback: + fallback = (enum nss_status (*)(const char *name, + struct gaih_addrtuple **pat, + char *buffer, size_t buflen, + int *errnop, int *h_errnop, + int32_t *ttlp)) + find_fallback("libnss_dns.so.2", "_nss_dns_gethostbyname4_r"); + + if (fallback) + return fallback(name, pat, buffer, buflen, errnop, h_errnop, ttlp); + fail: *errnop = -r; - *h_errnop = NO_DATA; + *h_errnop = NO_RECOVERY; return NSS_STATUS_UNAVAIL; } @@ -302,6 +302,15 @@ enum nss_status _nss_resolve_gethostbyname3_r( int32_t *ttlp, char **canonp) { + enum nss_status (*fallback)( + const char *name, + int af, + struct hostent *result, + char *buffer, size_t buflen, + int *errnop, int *h_errnop, + int32_t *ttlp, + char **canonp); + _cleanup_(sd_bus_message_unrefp) sd_bus_message *req = NULL, *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; char *r_name, *r_aliases, *r_addr, *r_addr_list; @@ -328,7 +337,7 @@ enum nss_status _nss_resolve_gethostbyname3_r( r = sd_bus_open_system(&bus); if (r < 0) - goto fail; + goto fallback; r = sd_bus_message_new_method_call( bus, @@ -356,32 +365,10 @@ enum nss_status _nss_resolve_gethostbyname3_r( return NSS_STATUS_NOTFOUND; } - if (bus_error_shall_fallback(&error)) { + if (bus_error_shall_fallback(&error)) + goto fallback; - enum nss_status (*fallback)( - const char *name, - int af, - struct hostent *result, - char *buffer, size_t buflen, - int *errnop, int *h_errnop, - int32_t *ttlp, - char **canonp); - - fallback = (enum nss_status (*)(const char *name, - int af, - struct hostent *result, - char *buffer, size_t buflen, - int *errnop, int *h_errnop, - int32_t *ttlp, - char **canonp)) - find_fallback("libnss_dns.so.2", "_nss_dns_gethostbyname3_r"); - if (fallback) - return fallback(name, af, result, buffer, buflen, errnop, h_errnop, ttlp, canonp); - } - - *errnop = -r; - *h_errnop = NO_RECOVERY; - return NSS_STATUS_UNAVAIL; + goto fail; } c = count_addresses(reply, af, &canonical); @@ -494,9 +481,21 @@ enum nss_status _nss_resolve_gethostbyname3_r( return NSS_STATUS_SUCCESS; +fallback: + fallback = (enum nss_status (*)(const char *name, + int af, + struct hostent *result, + char *buffer, size_t buflen, + int *errnop, int *h_errnop, + int32_t *ttlp, + char **canonp)) + find_fallback("libnss_dns.so.2", "_nss_dns_gethostbyname3_r"); + if (fallback) + return fallback(name, af, result, buffer, buflen, errnop, h_errnop, ttlp, canonp); + fail: *errnop = -r; - *h_errnop = NO_DATA; + *h_errnop = NO_RECOVERY; return NSS_STATUS_UNAVAIL; } @@ -508,6 +507,15 @@ enum nss_status _nss_resolve_gethostbyaddr2_r( int *errnop, int *h_errnop, int32_t *ttlp) { + enum nss_status (*fallback)( + const void* addr, socklen_t len, + int af, + struct hostent *result, + char *buffer, size_t buflen, + int *errnop, int *h_errnop, + int32_t *ttlp); + + _cleanup_(sd_bus_message_unrefp) sd_bus_message *req = NULL, *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; char *r_name, *r_aliases, *r_addr, *r_addr_list; @@ -539,7 +547,7 @@ enum nss_status _nss_resolve_gethostbyaddr2_r( r = sd_bus_open_system(&bus); if (r < 0) - goto fail; + goto fallback; r = sd_bus_message_new_method_call( bus, @@ -575,28 +583,9 @@ enum nss_status _nss_resolve_gethostbyaddr2_r( return NSS_STATUS_NOTFOUND; } - if (bus_error_shall_fallback(&error)) { + if (bus_error_shall_fallback(&error)) + goto fallback; - enum nss_status (*fallback)( - const void* addr, socklen_t len, - int af, - struct hostent *result, - char *buffer, size_t buflen, - int *errnop, int *h_errnop, - int32_t *ttlp); - - fallback = (enum nss_status (*)( - const void* addr, socklen_t len, - int af, - struct hostent *result, - char *buffer, size_t buflen, - int *errnop, int *h_errnop, - int32_t *ttlp)) - find_fallback("libnss_dns.so.2", "_nss_dns_gethostbyaddr2_r"); - - if (fallback) - return fallback(addr, len, af, result, buffer, buflen, errnop, h_errnop, ttlp); - } *errnop = -r; *h_errnop = NO_RECOVERY; @@ -694,9 +683,22 @@ enum nss_status _nss_resolve_gethostbyaddr2_r( return NSS_STATUS_SUCCESS; +fallback: + fallback = (enum nss_status (*)( + const void* addr, socklen_t len, + int af, + struct hostent *result, + char *buffer, size_t buflen, + int *errnop, int *h_errnop, + int32_t *ttlp)) + find_fallback("libnss_dns.so.2", "_nss_dns_gethostbyaddr2_r"); + + if (fallback) + return fallback(addr, len, af, result, buffer, buflen, errnop, h_errnop, ttlp); + fail: *errnop = -r; - *h_errnop = NO_DATA; + *h_errnop = NO_RECOVERY; return NSS_STATUS_UNAVAIL; }