From 8493a82d0bd5915eb951512cff5e570c43386283 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 May 2023 06:48:16 +0900 Subject: [PATCH 1/5] test: add tests for JoinsNamespaceOf= To illustrate the current behavior of the dependency. --- .../testsuite-23-joins-namespace-of-1.service | 7 +++++ .../testsuite-23-joins-namespace-of-2.service | 10 ++++++ .../testsuite-23-joins-namespace-of-3.service | 10 ++++++ .../testsuite-23-joins-namespace-of-4.service | 10 ++++++ .../testsuite-23-joins-namespace-of-5.service | 6 ++++ .../testsuite-23-joins-namespace-of-6.service | 10 ++++++ .../testsuite-23-joins-namespace-of-7.service | 11 +++++++ .../testsuite-23-joins-namespace-of-8.service | 9 ++++++ .../testsuite-23-joins-namespace-of-9.service | 11 +++++++ test/units/testsuite-23.JoinsNamespaceOf.sh | 31 +++++++++++++++++++ 10 files changed, 115 insertions(+) create mode 100644 test/testsuite-23.units/testsuite-23-joins-namespace-of-1.service create mode 100644 test/testsuite-23.units/testsuite-23-joins-namespace-of-2.service create mode 100644 test/testsuite-23.units/testsuite-23-joins-namespace-of-3.service create mode 100644 test/testsuite-23.units/testsuite-23-joins-namespace-of-4.service create mode 100644 test/testsuite-23.units/testsuite-23-joins-namespace-of-5.service create mode 100644 test/testsuite-23.units/testsuite-23-joins-namespace-of-6.service create mode 100644 test/testsuite-23.units/testsuite-23-joins-namespace-of-7.service create mode 100644 test/testsuite-23.units/testsuite-23-joins-namespace-of-8.service create mode 100644 test/testsuite-23.units/testsuite-23-joins-namespace-of-9.service create mode 100755 test/units/testsuite-23.JoinsNamespaceOf.sh diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-1.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-1.service new file mode 100644 index 0000000000..9919a9fa82 --- /dev/null +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-1.service @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Service] +Type=notify +NotifyAccess=all +MountAPIVFS=yes +PrivateTmp=yes +ExecStart=/bin/bash -c 'touch /tmp/shared-private-file && systemd-notify --ready && sleep infinity' diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-2.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-2.service new file mode 100644 index 0000000000..36b4c272fd --- /dev/null +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-2.service @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +JoinsNamespaceOf=testsuite-23-joins-namespace-of-1.service + +[Service] +Type=oneshot +MountAPIVFS=yes +PrivateTmp=yes +ExecStart=test -e /tmp/shared-private-file +ExecStart=touch /tmp/hoge diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-3.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-3.service new file mode 100644 index 0000000000..9094445020 --- /dev/null +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-3.service @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +JoinsNamespaceOf=testsuite-23-joins-namespace-of-1.service + +[Service] +Type=oneshot +MountAPIVFS=yes +PrivateTmp=yes +ExecStart=test -e /tmp/shared-private-file +ExecStart=test -e /tmp/hoge diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-4.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-4.service new file mode 100644 index 0000000000..5e823a1778 --- /dev/null +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-4.service @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +JoinsNamespaceOf=testsuite-23-joins-namespace-of-5.service + +[Service] +Type=notify +NotifyAccess=all +MountAPIVFS=yes +PrivateTmp=yes +ExecStart=/bin/bash -c 'touch /tmp/shared-private-file && systemd-notify --ready && sleep infinity' diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-5.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-5.service new file mode 100644 index 0000000000..80594ccba2 --- /dev/null +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-5.service @@ -0,0 +1,6 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Service] +Type=oneshot +MountAPIVFS=yes +PrivateTmp=yes +ExecStart=test ! -e /tmp/shared-private-file diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-6.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-6.service new file mode 100644 index 0000000000..bbbfd7c67d --- /dev/null +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-6.service @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +JoinsNamespaceOf=testsuite-23-joins-namespace-of-8.service + +[Service] +Type=notify +NotifyAccess=all +MountAPIVFS=yes +PrivateTmp=yes +ExecStart=/bin/bash -c 'touch /tmp/shared-private-file-x && systemd-notify --ready && sleep infinity' diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-7.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-7.service new file mode 100644 index 0000000000..6c7bbdb097 --- /dev/null +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-7.service @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +JoinsNamespaceOf=testsuite-23-joins-namespace-of-8.service + +[Service] +Type=oneshot +MountAPIVFS=yes +PrivateTmp=yes +ExecStart=test ! -e /tmp/shared-private-file-x +ExecStart=test ! -e /tmp/shared-private-file-y +ExecStart=touch /tmp/hoge diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-8.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-8.service new file mode 100644 index 0000000000..f3ec0668de --- /dev/null +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-8.service @@ -0,0 +1,9 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Service] +Type=notify +NotifyAccess=all +MountAPIVFS=yes +PrivateTmp=yes +ExecStartPre=test ! -e /tmp/shared-private-file-x +ExecStartPre=test ! -e /tmp/hoge +ExecStart=/bin/bash -c 'touch /tmp/shared-private-file-y && systemd-notify --ready && sleep infinity' diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-9.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-9.service new file mode 100644 index 0000000000..01de7f9054 --- /dev/null +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-9.service @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +JoinsNamespaceOf=testsuite-23-joins-namespace-of-8.service + +[Service] +Type=oneshot +MountAPIVFS=yes +PrivateTmp=yes +ExecStart=test ! -e /tmp/shared-private-file-x +ExecStart=test -e /tmp/shared-private-file-y +ExecStart=test ! -e /tmp/hoge diff --git a/test/units/testsuite-23.JoinsNamespaceOf.sh b/test/units/testsuite-23.JoinsNamespaceOf.sh new file mode 100755 index 0000000000..68ba465072 --- /dev/null +++ b/test/units/testsuite-23.JoinsNamespaceOf.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later + +set -eux +set -o pipefail + +# Test JoinsNamespaceOf= with PrivateTmp=yes + +systemd-analyze log-level debug +systemd-analyze log-target journal + +# simple case +systemctl start testsuite-23-joins-namespace-of-1.service +systemctl start testsuite-23-joins-namespace-of-2.service +systemctl start testsuite-23-joins-namespace-of-3.service +systemctl stop testsuite-23-joins-namespace-of-1.service + +# inverse dependency +systemctl start testsuite-23-joins-namespace-of-4.service +systemctl start testsuite-23-joins-namespace-of-5.service +systemctl stop testsuite-23-joins-namespace-of-4.service + +# transitive dependency +systemctl start testsuite-23-joins-namespace-of-6.service +systemctl start testsuite-23-joins-namespace-of-7.service +systemctl start testsuite-23-joins-namespace-of-8.service +systemctl start testsuite-23-joins-namespace-of-9.service +systemctl stop testsuite-23-joins-namespace-of-6.service +systemctl stop testsuite-23-joins-namespace-of-8.service + +systemd-analyze log-level info From 512df9de23890fcfd5fdbfe633250fe848195d4b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 23 May 2023 06:03:52 +0900 Subject: [PATCH 2/5] core/unit: drop doubled empty line --- src/core/unit.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/unit.c b/src/core/unit.c index 6c2682f0aa..7a43355832 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1053,7 +1053,6 @@ static int unit_per_dependency_type_hashmap_update( if (r < 0) return r; - return 1; } From a60f96fcf55c3452e5b13d6daec537af1909eda3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 23 May 2023 06:36:44 +0900 Subject: [PATCH 3/5] core/unit: make JoinsNamespaceOf= implies the inverse dependency Previously, even if a.service has JoinsNamespaceOf=b.service, the inverse direction of reference was not introduced. Hence, a.service is started earlier than b.service, the namespace will not shared with b.service. Also, even if a.service had the reference to b.service, b.service did not. If b.service is freed earlier, then unit_clear_dependencies() does not clear the reference from a to b, and will cause use-after-free on unit_free() for a.service. Let's make JoinsNamespaceOf=b.service in a.service implies the inverse dependency, i.e. JoinsNamespaceOf=a.service for b.service. Then, we can safely free b.service. --- man/systemd.unit.xml | 12 +++++++----- src/core/unit.c | 11 +++++------ .../testsuite-23-joins-namespace-of-5.service | 2 +- .../testsuite-23-joins-namespace-of-8.service | 2 +- .../testsuite-23-joins-namespace-of-9.service | 2 +- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index d603ec9744..fcd1f914a8 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -856,16 +856,18 @@ JoinsNamespaceOf= For units that start processes (such as service units), lists one or more other units - whose network and/or temporary file namespace to join. This only applies to unit types which support - the PrivateNetwork=, NetworkNamespacePath=, + whose network and/or temporary file namespace to join. If this is specified on a unit (say, a.service + has JoinsNamespaceOf=b.service), then this the inverse dependency + (JoinsNamespaceOf=a.service for b.service) is implied. This only applies to unit + types which support the PrivateNetwork=, NetworkNamespacePath=, PrivateIPC=, IPCNamespacePath=, and PrivateTmp= directives (see systemd.exec5 for details). If a unit that has this setting set is started, its processes will see the same /tmp/, /var/tmp/, IPC namespace and network namespace as - one listed unit that is started. If multiple listed units are already started, it is not defined - which namespace is joined. Note that this setting only has an effect if - PrivateNetwork=/NetworkNamespacePath=, + one listed unit that is started. If multiple listed units are already started and these do not share + their namespace, then it is not defined which namespace is joined. Note that this setting only has an + effect if PrivateNetwork=/NetworkNamespacePath=, PrivateIPC=/IPCNamespacePath= and/or PrivateTmp= is enabled for both the unit that joins the namespace and the unit whose namespace is joined. diff --git a/src/core/unit.c b/src/core/unit.c index 7a43355832..be57bdbd1d 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3209,12 +3209,11 @@ int unit_add_dependency( return r; notify = r > 0; - if (inverse_table[d] != _UNIT_DEPENDENCY_INVALID && inverse_table[d] != d) { - r = unit_add_dependency_hashmap(&other->dependencies, inverse_table[d], u, 0, mask); - if (r < 0) - return r; - notify_other = r > 0; - } + assert(inverse_table[d] >= 0 && inverse_table[d] < _UNIT_DEPENDENCY_MAX); + r = unit_add_dependency_hashmap(&other->dependencies, inverse_table[d], u, 0, mask); + if (r < 0) + return r; + notify_other = r > 0; if (add_reference) { r = unit_add_dependency_hashmap(&u->dependencies, UNIT_REFERENCES, other, mask, 0); diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-5.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-5.service index 80594ccba2..c3d316bfa2 100644 --- a/test/testsuite-23.units/testsuite-23-joins-namespace-of-5.service +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-5.service @@ -3,4 +3,4 @@ Type=oneshot MountAPIVFS=yes PrivateTmp=yes -ExecStart=test ! -e /tmp/shared-private-file +ExecStart=test -e /tmp/shared-private-file diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-8.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-8.service index f3ec0668de..42053b99f8 100644 --- a/test/testsuite-23.units/testsuite-23-joins-namespace-of-8.service +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-8.service @@ -4,6 +4,6 @@ Type=notify NotifyAccess=all MountAPIVFS=yes PrivateTmp=yes -ExecStartPre=test ! -e /tmp/shared-private-file-x +ExecStartPre=test -e /tmp/shared-private-file-x ExecStartPre=test ! -e /tmp/hoge ExecStart=/bin/bash -c 'touch /tmp/shared-private-file-y && systemd-notify --ready && sleep infinity' diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-9.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-9.service index 01de7f9054..a50a7fcdc2 100644 --- a/test/testsuite-23.units/testsuite-23-joins-namespace-of-9.service +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-9.service @@ -7,5 +7,5 @@ Type=oneshot MountAPIVFS=yes PrivateTmp=yes ExecStart=test ! -e /tmp/shared-private-file-x -ExecStart=test -e /tmp/shared-private-file-y +ExecStart=test ! -e /tmp/shared-private-file-y ExecStart=test ! -e /tmp/hoge From 83123a44989c095f9b7a89841db9917417fc451a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 May 2023 18:08:37 +0900 Subject: [PATCH 4/5] core/unit: search shared namespace in transitive relation of JoinsNamespaceOf= Previously, dependency chain of JoinsNamespaceOf= did not work, e.g. - a.service has JoinsNamespaceOf=b.service - b.service has JoinsNamespaceOf=c.service if, first c.service, next a.service, finally b.service is started, then a.service is not joined to the namespace of c.service. And, as mentioned in the document, the namespace used by b.service is not deterministic. This makes when searching exsiting namespace to be joined, all units in the transitive dependency of JoinsNamespaceOf= are checked. --- src/core/unit.c | 34 ++++++++++++++++++- src/core/unit.h | 1 + .../testsuite-23-joins-namespace-of-7.service | 2 +- .../testsuite-23-joins-namespace-of-8.service | 2 +- .../testsuite-23-joins-namespace-of-9.service | 6 ++-- 5 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index be57bdbd1d..35f41531c4 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4888,6 +4888,7 @@ int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask) int unit_setup_exec_runtime(Unit *u) { _cleanup_(exec_shared_runtime_unrefp) ExecSharedRuntime *esr = NULL; _cleanup_(dynamic_creds_unrefp) DynamicCreds *dcreds = NULL; + _cleanup_set_free_ Set *units = NULL; ExecRuntime **rt; ExecContext *ec; size_t offset; @@ -4905,8 +4906,12 @@ int unit_setup_exec_runtime(Unit *u) { ec = unit_get_exec_context(u); assert(ec); + r = unit_get_transitive_dependency_set(u, UNIT_ATOM_JOINS_NAMESPACE_OF, &units); + if (r < 0) + return r; + /* Try to get it from somebody else */ - UNIT_FOREACH_DEPENDENCY(other, u, UNIT_ATOM_JOINS_NAMESPACE_OF) { + SET_FOREACH(other, units) { r = exec_shared_runtime_acquire(u->manager, NULL, other->id, false, &esr); if (r < 0) return r; @@ -6111,6 +6116,33 @@ int unit_get_dependency_array(const Unit *u, UnitDependencyAtom atom, Unit ***re return (int) n; } +int unit_get_transitive_dependency_set(Unit *u, UnitDependencyAtom atom, Set **ret) { + _cleanup_set_free_ Set *units = NULL, *queue = NULL; + Unit *other; + int r; + + assert(u); + assert(ret); + + /* Similar to unit_get_dependency_array(), but also search the same dependency in other units. */ + + do { + UNIT_FOREACH_DEPENDENCY(other, u, atom) { + r = set_ensure_put(&units, NULL, other); + if (r < 0) + return r; + if (r == 0) + continue; + r = set_ensure_put(&queue, NULL, other); + if (r < 0) + return r; + } + } while ((u = set_steal_first(queue))); + + *ret = TAKE_PTR(units); + return 0; +} + const ActivationDetailsVTable * const activation_details_vtable[_UNIT_TYPE_MAX] = { [UNIT_PATH] = &activation_details_path_vtable, [UNIT_TIMER] = &activation_details_timer_vtable, diff --git a/src/core/unit.h b/src/core/unit.h index 09af8915dc..f2d4fd6a4b 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -810,6 +810,7 @@ static inline const UnitVTable* UNIT_VTABLE(const Unit *u) { Unit* unit_has_dependency(const Unit *u, UnitDependencyAtom atom, Unit *other); int unit_get_dependency_array(const Unit *u, UnitDependencyAtom atom, Unit ***ret_array); +int unit_get_transitive_dependency_set(Unit *u, UnitDependencyAtom atom, Set **ret); static inline Hashmap* unit_get_dependencies(Unit *u, UnitDependency d) { return hashmap_get(u->dependencies, UNIT_DEPENDENCY_TO_PTR(d)); diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-7.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-7.service index 6c7bbdb097..60c083a3f4 100644 --- a/test/testsuite-23.units/testsuite-23-joins-namespace-of-7.service +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-7.service @@ -6,6 +6,6 @@ JoinsNamespaceOf=testsuite-23-joins-namespace-of-8.service Type=oneshot MountAPIVFS=yes PrivateTmp=yes -ExecStart=test ! -e /tmp/shared-private-file-x +ExecStart=test -e /tmp/shared-private-file-x ExecStart=test ! -e /tmp/shared-private-file-y ExecStart=touch /tmp/hoge diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-8.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-8.service index 42053b99f8..dac1cea7bd 100644 --- a/test/testsuite-23.units/testsuite-23-joins-namespace-of-8.service +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-8.service @@ -5,5 +5,5 @@ NotifyAccess=all MountAPIVFS=yes PrivateTmp=yes ExecStartPre=test -e /tmp/shared-private-file-x -ExecStartPre=test ! -e /tmp/hoge +ExecStartPre=test -e /tmp/hoge ExecStart=/bin/bash -c 'touch /tmp/shared-private-file-y && systemd-notify --ready && sleep infinity' diff --git a/test/testsuite-23.units/testsuite-23-joins-namespace-of-9.service b/test/testsuite-23.units/testsuite-23-joins-namespace-of-9.service index a50a7fcdc2..6c64873b24 100644 --- a/test/testsuite-23.units/testsuite-23-joins-namespace-of-9.service +++ b/test/testsuite-23.units/testsuite-23-joins-namespace-of-9.service @@ -6,6 +6,6 @@ JoinsNamespaceOf=testsuite-23-joins-namespace-of-8.service Type=oneshot MountAPIVFS=yes PrivateTmp=yes -ExecStart=test ! -e /tmp/shared-private-file-x -ExecStart=test ! -e /tmp/shared-private-file-y -ExecStart=test ! -e /tmp/hoge +ExecStart=test -e /tmp/shared-private-file-x +ExecStart=test -e /tmp/shared-private-file-y +ExecStart=test -e /tmp/hoge From 831108245eb757f41fe0ebbccf1b42c9dd0ce297 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 23 May 2023 17:49:16 +0900 Subject: [PATCH 5/5] core/unit: update bidirectional dependency simultaneously Previously, if unit_add_dependency_hashmap() failed, then a one-directional unit dependency reference might be created, and triggeres use-after-free. See issue #27742 for more details. This makes unit dependency always bidirectional, and cleanly revert partial update on failure. Fixes #27742. --- src/core/unit.c | 164 ++++++++++++++++++++++++++++++------------------ 1 file changed, 103 insertions(+), 61 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index 35f41531c4..90f87a95f5 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1056,46 +1056,6 @@ static int unit_per_dependency_type_hashmap_update( return 1; } -static int unit_add_dependency_hashmap( - Hashmap **dependencies, - UnitDependency d, - Unit *other, - UnitDependencyMask origin_mask, - UnitDependencyMask destination_mask) { - - Hashmap *per_type; - int r; - - assert(dependencies); - assert(other); - assert(origin_mask < _UNIT_DEPENDENCY_MASK_FULL); - assert(destination_mask < _UNIT_DEPENDENCY_MASK_FULL); - assert(origin_mask > 0 || destination_mask > 0); - - /* Ensure the top-level dependency hashmap exists that maps UnitDependency → Hashmap(Unit* → - * UnitDependencyInfo) */ - r = hashmap_ensure_allocated(dependencies, NULL); - if (r < 0) - return r; - - /* Acquire the inner hashmap, that maps Unit* → UnitDependencyInfo, for the specified dependency - * type, and if it's missing allocate it and insert it. */ - per_type = hashmap_get(*dependencies, UNIT_DEPENDENCY_TO_PTR(d)); - if (!per_type) { - per_type = hashmap_new(NULL); - if (!per_type) - return -ENOMEM; - - r = hashmap_put(*dependencies, UNIT_DEPENDENCY_TO_PTR(d), per_type); - if (r < 0) { - hashmap_free(per_type); - return r; - } - } - - return unit_per_dependency_type_hashmap_update(per_type, other, origin_mask, destination_mask); -} - static void unit_merge_dependencies(Unit *u, Unit *other) { Hashmap *deps; void *dt; /* Actually of type UnitDependency, except that we don't bother casting it here, @@ -3103,11 +3063,38 @@ bool unit_job_is_applicable(Unit *u, JobType j) { } } -int unit_add_dependency( +static Hashmap *unit_get_dependency_hashmap_per_type(Unit *u, UnitDependency d) { + Hashmap *deps; + + assert(u); + assert(d >= 0 && d < _UNIT_DEPENDENCY_MAX); + + deps = hashmap_get(u->dependencies, UNIT_DEPENDENCY_TO_PTR(d)); + if (!deps) { + _cleanup_hashmap_free_ Hashmap *h = NULL; + + h = hashmap_new(NULL); + if (!h) + return NULL; + + if (hashmap_ensure_put(&u->dependencies, NULL, UNIT_DEPENDENCY_TO_PTR(d), h) < 0) + return NULL; + + deps = TAKE_PTR(h); + } + + return deps; +} + +typedef enum NotifyDependencyFlags { + NOTIFY_DEPENDENCY_UPDATE_FROM = 1 << 0, + NOTIFY_DEPENDENCY_UPDATE_TO = 1 << 1, +} NotifyDependencyFlags; + +static int unit_add_dependency_impl( Unit *u, UnitDependency d, Unit *other, - bool add_reference, UnitDependencyMask mask) { static const UnitDependency inverse_table[_UNIT_DEPENDENCY_MAX] = { @@ -3143,12 +3130,78 @@ int unit_add_dependency( [UNIT_IN_SLICE] = UNIT_SLICE_OF, [UNIT_SLICE_OF] = UNIT_IN_SLICE, }; + + Hashmap *u_deps, *other_deps; + UnitDependencyInfo u_info, u_info_old, other_info, other_info_old; + NotifyDependencyFlags flags = 0; + int r; + + assert(u); + assert(other); + assert(d >= 0 && d < _UNIT_DEPENDENCY_MAX); + assert(inverse_table[d] >= 0 && inverse_table[d] < _UNIT_DEPENDENCY_MAX); + assert(mask > 0 && mask < _UNIT_DEPENDENCY_MASK_FULL); + + /* Ensure the following two hashmaps for each unit exist: + * - the top-level dependency hashmap that maps UnitDependency → Hashmap(Unit* → UnitDependencyInfo), + * - the inner hashmap, that maps Unit* → UnitDependencyInfo, for the specified dependency type. */ + u_deps = unit_get_dependency_hashmap_per_type(u, d); + if (!u_deps) + return -ENOMEM; + + other_deps = unit_get_dependency_hashmap_per_type(other, inverse_table[d]); + if (!other_deps) + return -ENOMEM; + + /* Save the original dependency info. */ + u_info.data = u_info_old.data = hashmap_get(u_deps, other); + other_info.data = other_info_old.data = hashmap_get(other_deps, u); + + /* Update dependency info. */ + u_info.origin_mask |= mask; + other_info.destination_mask |= mask; + + /* Save updated dependency info. */ + if (u_info.data != u_info_old.data) { + r = hashmap_replace(u_deps, other, u_info.data); + if (r < 0) + return r; + + flags = NOTIFY_DEPENDENCY_UPDATE_FROM; + } + + if (other_info.data != other_info_old.data) { + r = hashmap_replace(other_deps, u, other_info.data); + if (r < 0) { + if (u_info.data != u_info_old.data) { + /* Restore the old dependency. */ + if (u_info_old.data) + (void) hashmap_update(u_deps, other, u_info_old.data); + else + hashmap_remove(u_deps, other); + } + return r; + } + + flags |= NOTIFY_DEPENDENCY_UPDATE_TO; + } + + return flags; +} + +int unit_add_dependency( + Unit *u, + UnitDependency d, + Unit *other, + bool add_reference, + UnitDependencyMask mask) { + UnitDependencyAtom a; int r; /* Helper to know whether sending a notification is necessary or not: if the dependency is already * there, no need to notify! */ - bool notify, notify_other = false; + NotifyDependencyFlags notify_flags; assert(u); assert(d >= 0 && d < _UNIT_DEPENDENCY_MAX); @@ -3204,35 +3257,24 @@ int unit_add_dependency( return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL), "Requested dependency SliceOf=%s refused (%s is not a cgroup unit).", other->id, other->id); - r = unit_add_dependency_hashmap(&u->dependencies, d, other, mask, 0); + r = unit_add_dependency_impl(u, d, other, mask); if (r < 0) return r; - notify = r > 0; - - assert(inverse_table[d] >= 0 && inverse_table[d] < _UNIT_DEPENDENCY_MAX); - r = unit_add_dependency_hashmap(&other->dependencies, inverse_table[d], u, 0, mask); - if (r < 0) - return r; - notify_other = r > 0; + notify_flags = r; if (add_reference) { - r = unit_add_dependency_hashmap(&u->dependencies, UNIT_REFERENCES, other, mask, 0); + r = unit_add_dependency_impl(u, UNIT_REFERENCES, other, mask); if (r < 0) return r; - notify = notify || r > 0; - - r = unit_add_dependency_hashmap(&other->dependencies, UNIT_REFERENCED_BY, u, 0, mask); - if (r < 0) - return r; - notify_other = notify_other || r > 0; + notify_flags |= r; } - if (notify) + if (FLAGS_SET(notify_flags, NOTIFY_DEPENDENCY_UPDATE_FROM)) unit_add_to_dbus_queue(u); - if (notify_other) + if (FLAGS_SET(notify_flags, NOTIFY_DEPENDENCY_UPDATE_TO)) unit_add_to_dbus_queue(other); - return notify || notify_other; + return notify_flags != 0; } int unit_add_two_dependencies(Unit *u, UnitDependency d, UnitDependency e, Unit *other, bool add_reference, UnitDependencyMask mask) {