services will have acquired an <varname>OnFailure=</varname> dependency on
<filename index='false'>failure-handler@%N.service</filename>. The
template instance units will also have gained the dependency which results
- in the creation of a recursive dependency chain. We can break the chain by
- disabling the drop-in for the template instance units via a symlink to
+ in the creation of a recursive dependency chain. systemd will try to detect
+ these recursive dependency chains where a template unit directly and
+ recursively depends on itself and will remove such dependencies
+ automatically if it finds them. If systemd doesn't detect the recursive
+ dependency chain, we can break the chain ourselves by disabling the drop-in
+ for the template instance units via a symlink to
<filename index='false'>/dev/null</filename>:</para>
<programlisting>
#include "alloc-util.h"
#include "glob-util.h"
#include "hexdecoct.h"
+#include "memory-util.h"
#include "path-util.h"
#include "special.h"
#include "string-util.h"
return true;
}
+
+bool unit_name_prefix_equal(const char *a, const char *b) {
+ const char *p, *q;
+
+ assert(a);
+ assert(b);
+
+ if (!unit_name_is_valid(a, UNIT_NAME_ANY) || !unit_name_is_valid(b, UNIT_NAME_ANY))
+ return false;
+
+ p = strchr(a, '@');
+ if (!p)
+ p = strrchr(a, '.');
+
+ q = strchr(b, '@');
+ if (!q)
+ q = strrchr(b, '.');
+
+ assert(p);
+ assert(q);
+
+ return memcmp_nn(a, p - a, b, q - b) == 0;
+}
int slice_build_parent_slice(const char *slice, char **ret);
int slice_build_subslice(const char *slice, const char *name, char **subslice);
bool slice_name_is_valid(const char *name);
+
+bool unit_name_prefix_equal(const char *a, const char *b);
DEFINE_CONFIG_PARSE_ENUM(config_parse_status_unit_format, status_unit_format, StatusUnitFormat, "Failed to parse status unit format");
DEFINE_CONFIG_PARSE_ENUM_FULL(config_parse_socket_timestamping, socket_timestamping_from_string_harder, SocketTimestamping, "Failed to parse timestamping precision");
+bool contains_instance_specifier_superset(const char *s) {
+ const char *p, *q;
+ bool percent = false;
+
+ assert(s);
+
+ p = strchr(s, '@');
+ if (!p)
+ return false;
+
+ p++; /* Skip '@' */
+
+ q = strrchr(p, '.');
+ if (!q)
+ q = p + strlen(p);
+
+ /* If the string is just the instance specifier, it's not a superset of the instance. */
+ if (memcmp_nn(p, q - p, "%i", strlen("%i")) == 0)
+ return false;
+
+ /* %i, %n and %N all expand to the instance or a superset of it. */
+ for (; p < q; p++) {
+ if (*p == '%')
+ percent = !percent;
+ else if (percent) {
+ if (IN_SET(*p, 'n', 'N', 'i'))
+ return true;
+ percent = false;
+ }
+ }
+
+ return false;
+}
+
+/* `name` is the rendered version of `format` via `unit_printf` or similar functions. */
+int unit_is_likely_recursive_template_dependency(Unit *u, const char *name, const char *format) {
+ const char *fragment_path;
+ int r;
+
+ assert(u);
+ assert(name);
+
+ /* If a template unit has a direct dependency on itself that includes the unit instance as part of
+ * the template instance via a unit specifier (%i, %n or %N), this will almost certainly lead to
+ * infinite recursion as systemd will keep instantiating new instances of the template unit.
+ * https://github.com/systemd/systemd/issues/17602 shows a good example of how this can happen in
+ * practice. To guard against this, we check for templates that depend on themselves and have the
+ * instantiated unit instance included as part of the template instance of the dependency via a
+ * specifier.
+ *
+ * For example, if systemd-notify@.service depends on systemd-notify@%n.service, this will result in
+ * infinite recursion.
+ */
+
+ if (!unit_name_is_valid(name, UNIT_NAME_INSTANCE))
+ return false;
+
+ if (!unit_name_prefix_equal(u->id, name))
+ return false;
+
+ if (u->type != unit_name_to_type(name))
+ return false;
+
+ r = unit_file_find_fragment(u->manager->unit_id_map, u->manager->unit_name_map, name, &fragment_path, NULL);
+ if (r < 0)
+ return r;
+
+ /* Fragment paths should also be equal as a custom fragment for a specific template instance
+ * wouldn't necessarily lead to infinite recursion. */
+ if (!path_equal_ptr(u->fragment_path, fragment_path))
+ return false;
+
+ if (!contains_instance_specifier_superset(format))
+ return false;
+
+ return true;
+}
+
int config_parse_unit_deps(
const char *unit,
const char *filename,
continue;
}
+ r = unit_is_likely_recursive_template_dependency(u, k, word);
+ if (r < 0) {
+ log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to determine if '%s' is a recursive dependency, ignoring: %m", k);
+ continue;
+ }
+ if (r > 0) {
+ log_syntax(unit, LOG_DEBUG, filename, line, 0,
+ "Dropping dependency %s=%s that likely leads to infinite recursion.",
+ unit_dependency_to_string(d), word);
+ continue;
+ }
+
r = unit_add_dependency_by_name(u, d, k, true, UNIT_DEPENDENCY_FILE);
if (r < 0)
log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to add dependency on %s, ignoring: %m", k);
#include "conf-parser.h"
#include "unit.h"
+/* These functions are declared in the header to make them accessible to unit tests. */
+bool contains_instance_specifier_superset(const char *s);
+int unit_is_likely_recursive_template_dependency(Unit *u, const char *name, const char *format);
+
/* Config-parsing helpers relevant only for sources under src/core/ */
int parse_crash_chvt(const char *value, int *data);
int parse_confirm_spawn(const char *value, char **console);
}
+static void test_contains_instance_specifier_superset(void) {
+ assert_se(contains_instance_specifier_superset("foobar@a%i"));
+ assert_se(contains_instance_specifier_superset("foobar@%ia"));
+ assert_se(contains_instance_specifier_superset("foobar@%n"));
+ assert_se(contains_instance_specifier_superset("foobar@%n.service"));
+ assert_se(contains_instance_specifier_superset("foobar@%N"));
+ assert_se(contains_instance_specifier_superset("foobar@%N.service"));
+ assert_se(contains_instance_specifier_superset("foobar@baz.%N.service"));
+ assert_se(contains_instance_specifier_superset("@%N.service"));
+ assert_se(contains_instance_specifier_superset("@%N"));
+ assert_se(contains_instance_specifier_superset("@%a%N"));
+
+ assert_se(!contains_instance_specifier_superset("foobar@%i.service"));
+ assert_se(!contains_instance_specifier_superset("foobar%ia.service"));
+ assert_se(!contains_instance_specifier_superset("foobar@%%n.service"));
+ assert_se(!contains_instance_specifier_superset("foobar@baz.service"));
+ assert_se(!contains_instance_specifier_superset("%N.service"));
+ assert_se(!contains_instance_specifier_superset("%N"));
+ assert_se(!contains_instance_specifier_superset("@%aN"));
+ assert_se(!contains_instance_specifier_superset("@%a%b"));
+}
+
+static void test_unit_is_recursive_template_dependency(void) {
+ _cleanup_(manager_freep) Manager *m = NULL;
+ Unit *u;
+ int r;
+
+ r = manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_MINIMAL, &m);
+ if (manager_errno_skip_test(r)) {
+ log_notice_errno(r, "Skipping test: manager_new: %m");
+ return;
+ }
+
+ assert_se(r >= 0);
+ assert_se(manager_startup(m, NULL, NULL, NULL) >= 0);
+
+ assert_se(u = unit_new(m, sizeof(Service)));
+ assert_se(unit_add_name(u, "foobar@1.service") == 0);
+ u->fragment_path = strdup("/foobar@.service");
+
+ assert_se(hashmap_put_strdup(&m->unit_id_map, "foobar@foobar@123.service", "/foobar@.service"));
+ assert_se(hashmap_put_strdup(&m->unit_id_map, "foobar@foobar@456.service", "/custom.service"));
+
+ /* Test that %n, %N and any extension of %i specifiers in the instance are detected as recursive. */
+ assert_se(unit_is_likely_recursive_template_dependency(u, "foobar@foobar@123.service", "foobar@%N.service") == 1);
+ assert_se(unit_is_likely_recursive_template_dependency(u, "foobar@foobar@123.service", "foobar@%n.service") == 1);
+ assert_se(unit_is_likely_recursive_template_dependency(u, "foobar@foobar@123.service", "foobar@a%i.service") == 1);
+ assert_se(unit_is_likely_recursive_template_dependency(u, "foobar@foobar@123.service", "foobar@%ia.service") == 1);
+ assert_se(unit_is_likely_recursive_template_dependency(u, "foobar@foobar@123.service", "foobar@%x%n.service") == 1);
+ /* Test that %i on its own is not detected as recursive. */
+ assert_se(unit_is_likely_recursive_template_dependency(u, "foobar@foobar@123.service", "foobar@%i.service") == 0);
+ /* Test that a specifier other than %i, %n and %N is not detected as recursive. */
+ assert_se(unit_is_likely_recursive_template_dependency(u, "foobar@foobar@123.service", "foobar@%xn.service") == 0);
+ /* Test that an expanded specifier is not detected as recursive. */
+ assert_se(unit_is_likely_recursive_template_dependency(u, "foobar@foobar@123.service", "foobar@foobar@123.service") == 0);
+ /* Test that a dependency with a custom fragment path is not detected as recursive. */
+ assert_se(unit_is_likely_recursive_template_dependency(u, "foobar@foobar@456.service", "foobar@%n.service") == 0);
+ /* Test that a dependency without a fragment path is not detected as recursive. */
+ assert_se(unit_is_likely_recursive_template_dependency(u, "foobar@foobar@789.service", "foobar@%n.service") == 0);
+ /* Test that a dependency with a different prefix is not detected as recursive. */
+ assert_se(unit_is_likely_recursive_template_dependency(u, "quux@foobar@123.service", "quux@%n.service") == 0);
+ /* Test that a dependency of a different type is not detected as recursive. */
+ assert_se(unit_is_likely_recursive_template_dependency(u, "foobar@foobar@123.mount", "foobar@%n.mount") == 0);
+}
+
int main(int argc, char *argv[]) {
_cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL;
int r;
TEST_REQ_RUNNING_SYSTEMD(test_install_printf());
test_unit_dump_config_items();
test_config_parse_memory_limit();
+ test_contains_instance_specifier_superset();
+ test_unit_is_recursive_template_dependency();
return r;
}
test_unit_name_from_dbus_path_one("/org/freedesktop/systemd1/unit/wpa_5fsupplicant_2eservice", 0, "wpa_supplicant.service");
}
+static void test_unit_name_prefix_equal(void) {
+ log_info("/* %s */", __func__);
+
+ assert_se(unit_name_prefix_equal("a.service", "a.service"));
+ assert_se(unit_name_prefix_equal("a.service", "a.mount"));
+ assert_se(unit_name_prefix_equal("a@b.service", "a.service"));
+ assert_se(unit_name_prefix_equal("a@b.service", "a@c.service"));
+
+ assert_se(!unit_name_prefix_equal("a.service", "b.service"));
+ assert_se(!unit_name_prefix_equal("a.service", "b.mount"));
+ assert_se(!unit_name_prefix_equal("a@a.service", "b.service"));
+ assert_se(!unit_name_prefix_equal("a@a.service", "b@a.service"));
+ assert_se(!unit_name_prefix_equal("a", "b"));
+ assert_se(!unit_name_prefix_equal("a", "a"));
+}
+
int main(int argc, char* argv[]) {
_cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL;
int r, rc = 0;
test_unit_name_path_unescape();
test_unit_name_to_prefix();
test_unit_name_from_dbus_path();
+ test_unit_name_prefix_equal();
return rc;
}
projects / thirdparty / systemd.git / commitdiff
