packages/systemd
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

!735 fix nss deadlock between dbus-daemon and PID 1

Browse Source 
From: @zhang-yao-2022 
Reviewed-by: @shenyangyang01 
Signed-off-by: @shenyangyang01
This commit is contained in:
openeuler-ci-bot 2024-11-06 07:52:10 +00:00 committed by Gitee
commit 8cd2917618
No known key found for this signature in database
GPG Key ID: 173E9B9CA92EEF8F
4 changed files with 348 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

160
backport-pid1-lookup-owning-PID-of-BusName-name-of-services-a.patch Normal file
View File

@ -0,0 +1,160 @@
From 9d3aec2603e9d10889797f1412fafd16c25a4920 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 17 Feb 2022 14:40:25 +0100
Subject: [PATCH] pid1: lookup owning PID of BusName= name of services
asynchronously
A first step of removing blocking calls to the D-Bus broker from PID 1.
There's a lot more to got (i.e. grep src/core/ for sd_bus_creds
basically), but it's a start.
Removing blocking calls to D-Bus broker deals systematicallly with
deadlocks caused by dbus-daemon blocking on synchronous IPC calls back
to PID1 (e.g. Varlink calls through nss-systemd). Bugs such as #15316.
Also-see: https://github.com/systemd/systemd/pull/22038#issuecomment-1042958390
(cherry picked from commit e39eb045a502d599e6cd3fda7a46020dd438d018)
(cherry picked from commit cf390149cb25248169c482e315a1a7ff02eaf956)
Conflict:NA
Reference:https://github.com/systemd/systemd/commit/1daa382a7f9e55d11f7b59b144a9963688169843
---
src/core/service.c | 91 ++++++++++++++++++++++++++++++++++++----------
src/core/service.h | 2 +
2 files changed, 74 insertions(+), 19 deletions(-)
diff --git a/src/core/service.c b/src/core/service.c
index 4cada77..2e3720b 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -407,6 +407,8 @@ static void service_done(Unit *u) {
s->timer_event_source = sd_event_source_unref(s->timer_event_source);
s->exec_fd_event_source = sd_event_source_unref(s->exec_fd_event_source);
+ s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot);
+
service_release_resources(u);
}
@@ -4079,6 +4081,60 @@ static int service_get_timeout(Unit *u, usec_t *timeout) {
return 1;
}
+static bool pick_up_pid_from_bus_name(Service *s) {
+ assert(s);
+
+ /* If the service is running but we have no main PID yet, get it from the owner of the D-Bus name */
+
+ return !pid_is_valid(s->main_pid) &&
+ IN_SET(s->state,
+ SERVICE_START,
+ SERVICE_START_POST,
+ SERVICE_RUNNING,
+ SERVICE_RELOAD);
+}
+
+static int bus_name_pid_lookup_callback(sd_bus_message *reply, void *userdata, sd_bus_error *ret_error) {
+ const sd_bus_error *e;
+ Unit *u = userdata;
+ uint32_t pid;
+ Service *s;
+ int r;
+
+ assert(reply);
+ assert(u);
+
+ s = SERVICE(u);
+ s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot);
+
+ if (!s->bus_name || !pick_up_pid_from_bus_name(s))
+ return 1;
+
+ e = sd_bus_message_get_error(reply);
+ if (e) {
+ r = sd_bus_error_get_errno(e);
+ log_warning_errno(r, "GetConnectionUnixProcessID() failed: %s", bus_error_message(e, r));
+ return 1;
+ }
+
+ r = sd_bus_message_read(reply, "u", &pid);
+ if (r < 0) {
+ bus_log_parse_error(r);
+ return 1;
+ }
+
+ if (!pid_is_valid(pid)) {
+ log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "GetConnectionUnixProcessID() returned invalid PID");
+ return 1;
+ }
+
+ log_unit_debug(u, "D-Bus name %s is now owned by process " PID_FMT, s->bus_name, (pid_t) pid);
+
+ service_set_main_pid(s, pid);
+ unit_watch_pid(UNIT(s), pid, false);
+ return 1;
+}
+
static void service_bus_name_owner_change(Unit *u, const char *new_owner) {
Service *s = SERVICE(u);
@@ -4109,28 +4165,25 @@ static void service_bus_name_owner_change(Unit *u, const char *new_owner) {
else if (s->state == SERVICE_START && new_owner)
service_enter_start_post(s);
- } else if (new_owner &&
- s->main_pid <= 0 &&
- IN_SET(s->state,
- SERVICE_START,
- SERVICE_START_POST,
- SERVICE_RUNNING,
- SERVICE_RELOAD)) {
-
- _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
- pid_t pid;
+ } else if (new_owner && pick_up_pid_from_bus_name(s)) {
/* Try to acquire PID from bus service */
- r = sd_bus_get_name_creds(u->manager->api_bus, s->bus_name, SD_BUS_CREDS_PID, &creds);
- if (r >= 0)
- r = sd_bus_creds_get_pid(creds, &pid);
- if (r >= 0) {
- log_unit_debug(u, "D-Bus name %s is now owned by process " PID_FMT, s->bus_name, pid);
-
- service_set_main_pid(s, pid);
- unit_watch_pid(UNIT(s), pid, false);
- }
+ s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot);
+
+ r = sd_bus_call_method_async(
+ u->manager->api_bus,
+ &s->bus_name_pid_lookup_slot,
+ "org.freedesktop.DBus",
+ "/org/freedesktop/DBus",
+ "org.freedesktop.DBus",
+ "GetConnectionUnixProcessID",
+ bus_name_pid_lookup_callback,
+ s,
+ "s",
+ s->bus_name);
+ if (r < 0)
+ log_debug_errno(r, "Failed to request owner PID of service name, ignoring: %m");
}
}
diff --git a/src/core/service.h b/src/core/service.h
index 11e861a..04809e9 100644
--- a/src/core/service.h
+++ b/src/core/service.h
@@ -174,6 +174,8 @@ struct Service {
NotifyAccess notify_access;
NotifyState notify_state;
+ sd_bus_slot *bus_name_pid_lookup_slot;
+
sd_event_source *exec_fd_event_source;
ServiceFDStore *fd_store;
--
2.33.0

125
backport-pid1-set-SYSTEMD_NSS_DYNAMIC_BYPASS-1-env-var-for-db.patch Normal file
View File

@ -0,0 +1,125 @@
From 0863a55ae95fe6bf7312b7a864d07a9e3fbee563 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 17 Feb 2022 14:49:54 +0100
Subject: [PATCH] pid1: set SYSTEMD_NSS_DYNAMIC_BYPASS=1 env var for
dbus-daemon
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
There's currently a deadlock between PID 1 and dbus-daemon: in some
cases dbus-daemon will do NSS lookups (which are blocking) at the same
time PID 1 synchronously blocks on some call to dbus-daemon. Let's break
that by setting SYSTEMD_NSS_DYNAMIC_BYPASS=1 env var for dbus-daemon,
which will disable synchronously blocking varlink calls from nss-systemd
to PID 1.
In the long run we should fix this differently: remove all synchronous
calls to dbus-daemon from PID 1. This is not trivial however: so far we
had the rule that synchronous calls from PID 1 to the dbus broker are OK
as long as they only go to interfaces implemented by the broke itself
rather than services reachable through it. Given that the relationship
between PID 1 and dbus is kinda special anyway, this was considered
acceptable for the sake of simplicity, since we quite often need
metadata about bus peers from the broker, and the asynchronous logic
would substantially complicate even the simplest method handlers.
This mostly reworks the existing code that sets SYSTEMD_NSS_BYPASS_BUS=
(which is a similar hack to deal with deadlocks between nss-systemd and
dbus-daemon itself) to set SYSTEMD_NSS_DYNAMIC_BYPASS=1 instead. No code
was checking SYSTEMD_NSS_BYPASS_BUS= anymore anyway, and it used to
solve a similar problem, hence it's an obvious piece of code to rework
like this.
Issue originally tracked down by Lukas Märdian. This patch is inspired
and closely based on his patch:
https://github.com/systemd/systemd/pull/22038
Fixes: #15316
Co-authored-by: Lukas Märdian <slyon@ubuntu.com>
(cherry picked from commit de90700f36f2126528f7ce92df0b5b5d5e277558)
(cherry picked from commit 367041af816d48d4852140f98fd0ba78ed83f9e4)
Conflict:different code contexts, manual synchronization patch
Reference:https://github.com/systemd/systemd/commit/0863a55ae95fe6bf7312b7a864d07a9e3fbee563
---
src/core/execute.c | 10 +++++-----
src/core/execute.h | 26 +++++++++++++-------------
src/core/service.c | 2 +-
3 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/src/core/execute.c b/src/core/execute.c
index 28efe5c36f..37f63a9378 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -1828,11 +1828,11 @@ static int build_environment(
our_env[n_env++] = x;
}
- /* If this is D-Bus, tell the nss-systemd module, since it relies on being able to use D-Bus look up dynamic
- * users via PID 1, possibly dead-locking the dbus daemon. This way it will not use D-Bus to resolve names, but
- * check the database directly. */
- if (p->flags & EXEC_NSS_BYPASS_BUS) {
- x = strdup("SYSTEMD_NSS_BYPASS_BUS=1");
+ /* If this is D-Bus, tell the nss-systemd module, since it relies on being able to use blocking
+ * Varlink calls back to us for look up dynamic users in PID 1. Break the deadlock between D-Bus and
+ * PID 1 by disabling use of PID1' NSS interface for looking up dynamic users. */
+ if (p->flags & EXEC_NSS_DYNAMIC_BYPASS) {
+ x = strdup("SYSTEMD_NSS_DYNAMIC_BYPASS=1");
if (!x)
return -ENOMEM;
our_env[n_env++] = x;
diff --git a/src/core/execute.h b/src/core/execute.h
index 4c7a5b874f..47349a69a2 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -297,20 +297,20 @@ static inline bool exec_context_restrict_namespaces_set(const ExecContext *c) {
}
typedef enum ExecFlags {
- EXEC_APPLY_SANDBOXING = 1 << 0,
- EXEC_APPLY_CHROOT = 1 << 1,
- EXEC_APPLY_TTY_STDIN = 1 << 2,
- EXEC_PASS_LOG_UNIT = 1 << 3, /* Whether to pass the unit name to the service's journal stream connection */
- EXEC_CHOWN_DIRECTORIES = 1 << 4, /* chown() the runtime/state/cache/log directories to the user we run as, under all conditions */
- EXEC_NSS_BYPASS_BUS = 1 << 5, /* Set the SYSTEMD_NSS_BYPASS_BUS environment variable, to disable nss-systemd for dbus */
- EXEC_CGROUP_DELEGATE = 1 << 6,
- EXEC_IS_CONTROL = 1 << 7,
- EXEC_CONTROL_CGROUP = 1 << 8, /* Place the process not in the indicated cgroup but in a subcgroup '/.control', but only EXEC_CGROUP_DELEGATE and EXEC_IS_CONTROL is set, too */
+ EXEC_APPLY_SANDBOXING = 1 << 0,
+ EXEC_APPLY_CHROOT = 1 << 1,
+ EXEC_APPLY_TTY_STDIN = 1 << 2,
+ EXEC_PASS_LOG_UNIT = 1 << 3, /* Whether to pass the unit name to the service's journal stream connection */
+ EXEC_CHOWN_DIRECTORIES = 1 << 4, /* Set the SYSTEMD_NSS_DYNAMIC_BYPASS environment variable, to disable nss-systemd blocking on PID 1, for use by dbus-daemon */
+ EXEC_NSS_DYNAMIC_BYPASS = 1 << 5, /* Set the EXEC_NSS_DYNAMIC_BYPASS environment variable, to disable nss-systemd for dbus */
+ EXEC_CGROUP_DELEGATE = 1 << 6,
+ EXEC_IS_CONTROL = 1 << 7,
+ EXEC_CONTROL_CGROUP = 1 << 8, /* Place the process not in the indicated cgroup but in a subcgroup '/.control', but only EXEC_CGROUP_DELEGATE and EXEC_IS_CONTROL is set, too */
/* The following are not used by execute.c, but by consumers internally */
- EXEC_PASS_FDS = 1 << 9,
- EXEC_SETENV_RESULT = 1 << 10,
- EXEC_SET_WATCHDOG = 1 << 11,
+ EXEC_PASS_FDS = 1 << 9,
+ EXEC_SETENV_RESULT = 1 << 10,
+ EXEC_SET_WATCHDOG = 1 << 11,
} ExecFlags;
/* Parameters for a specific invocation of a command. This structure is put together right before a command is
diff --git a/src/core/service.c b/src/core/service.c
index f6eb46cb54..a480edc439 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1573,7 +1573,7 @@ static int service_spawn(
return -ENOMEM;
/* System D-Bus needs nss-systemd disabled, so that we don't deadlock */
- SET_FLAG(exec_params.flags, EXEC_NSS_BYPASS_BUS,
+ SET_FLAG(exec_params.flags, EXEC_NSS_DYNAMIC_BYPASS,
MANAGER_IS_SYSTEM(UNIT(s)->manager) && unit_has_name(UNIT(s), SPECIAL_DBUS_SERVICE));
strv_free_and_replace(exec_params.environment, final_env);
--
2.33.0

53
backport-pid1-watch-bus-name-always-when-we-have-it.patch Normal file
View File

@ -0,0 +1,53 @@
From b301230a6ce52989053b12324fcaef0d45610ee6 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 17 Feb 2022 17:23:48 +0100
Subject: [PATCH] pid1: watch bus name always when we have it
Previously we'd only watch configured service bus names if Type=dbus was
set. Let's also watch it for other types. This is useful to pick up the
main PID of such a service. In fact the code to pick it up was already
in place, alas it didn't do anything given the signal was never received
for it. Fix that.
(It's also useful for debugging)
(cherry picked from commit 1e8b312e5a22538f91defb89cf2997e09e106297)
(cherry picked from commit a51e540b278827c0fc59760b9c77cd42cbddc0d2)
Conflict:different code contexts, manual synchronization patch
Reference:https://github.com/systemd/systemd/commit/b301230a6ce52989053b12324fcaef0d45610ee6
---
src/core/service.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/core/service.c b/src/core/service.c
index 7b90822f68..5f56217904 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -697,14 +697,16 @@ static int service_setup_bus_name(Service *s) {
if (!s->bus_name)
return 0;
- r = unit_add_dependency_by_name(UNIT(s), UNIT_REQUIRES, SPECIAL_DBUS_SOCKET, true, UNIT_DEPENDENCY_FILE);
- if (r < 0)
- return log_unit_error_errno(UNIT(s), r, "Failed to add dependency on " SPECIAL_DBUS_SOCKET ": %m");
+ if (s->type == SERVICE_DBUS) {
+ r = unit_add_dependency_by_name(UNIT(s), UNIT_REQUIRES, SPECIAL_DBUS_SOCKET, true, UNIT_DEPENDENCY_FILE);
+ if (r < 0)
+ return log_unit_error_errno(UNIT(s), r, "Failed to add dependency on " SPECIAL_DBUS_SOCKET ": %m");
- /* We always want to be ordered against dbus.socket if both are in the transaction. */
- r = unit_add_dependency_by_name(UNIT(s), UNIT_AFTER, SPECIAL_DBUS_SOCKET, true, UNIT_DEPENDENCY_FILE);
- if (r < 0)
- return log_unit_error_errno(UNIT(s), r, "Failed to add dependency on " SPECIAL_DBUS_SOCKET ": %m");
+ /* We always want to be ordered against dbus.socket if both are in the transaction. */
+ r = unit_add_dependency_by_name(UNIT(s), UNIT_AFTER, SPECIAL_DBUS_SOCKET, true, UNIT_DEPENDENCY_FILE);
+ if (r < 0)
+ return log_unit_error_errno(UNIT(s), r, "Failed to add dependency on " SPECIAL_DBUS_SOCKET ": %m");
+ }
r = unit_watch_bus_name(UNIT(s), s->bus_name);
if (r == -EEXIST)
--
2.33.0

11
systemd.spec
View File

@ -16,7 +16,7 @@
Name: systemd
Url: https://systemd.io/
Version: 243
Release: 81
Release: 82
License: MIT and LGPLv2+ and GPLv2+
Summary: System and Service Manager
@ -308,6 +308,9 @@ Patch0260: backport-CVE-2023-50868.patch
Patch0261: backport-login-user-runtime-dir-properly-check-for-mount-poin.patch
Patch0262: backport-user-util-validate-the-right-field.patch
Patch0263: backport-fix-cgtop-sscanf-return-code-checks.patch
Patch0264: backport-pid1-set-SYSTEMD_NSS_DYNAMIC_BYPASS-1-env-var-for-db.patch
Patch0265: backport-pid1-lookup-owning-PID-of-BusName-name-of-services-a.patch
Patch0266: backport-pid1-watch-bus-name-always-when-we-have-it.patch
#openEuler
Patch9002: 1509-fix-journal-file-descriptors-leak-problems.patch
@ -1715,6 +1718,12 @@ fi
%exclude /usr/share/man/man3/*
%changelog
* Tue Nov 5 2024 zhangyao <zhangyao108@huawei.com> - 243-82
- sync community patches
add backport-pid1-set-SYSTEMD_NSS_DYNAMIC_BYPASS-1-env-var-for-db.patch
backport-pid1-lookup-owning-PID-of-BusName-name-of-services-a.patch
backport-pid1-watch-bus-name-always-when-we-have-it.patch
* Thu Aug 22 2024 zhangyao <zhangyao108@huawei.com> - 243-81
- remove redundant patch
del 1610-add-new-rules-for-lower-priority-events-to-preempt.patch