From 499cdfde1b3f5d812912e89b1a1a0d7a4fb83306 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 1 Apr 2022 18:58:34 +0100 Subject: [PATCH 1/2] activation-helper: Never crash if unable to write oom_score_adj _dbus_warn() normally only logs a warning, but can be made fatal by environment variables. In particular, we do that during unit testing, which can result in a build-time test failure if dbus is built in a sandbox environment that prevents write access. _dbus_log() does only the logging part of _dbus_warn(), which seems more appropriate here. Signed-off-by: Simon McVittie --- bus/activation-helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bus/activation-helper.c b/bus/activation-helper.c index 8a4fd732..df0472ce 100644 --- a/bus/activation-helper.c +++ b/bus/activation-helper.c @@ -348,7 +348,7 @@ exec_for_correct_user (char *exec, char *user, DBusError *error) /* Resetting the OOM score adjustment is best-effort, so we don't * treat a failure to do so as fatal. */ if (!_dbus_reset_oom_score_adj (&error_str)) - _dbus_warn ("%s: %s", error_str, strerror (errno)); + _dbus_log (DBUS_SYSTEM_LOG_WARNING, "%s: %s", error_str, strerror (errno)); if (!switch_user (user, error)) return FALSE; -- GitLab From 5c8e5b7140c4f1f88684c7a83ce9587fff2def0c Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 1 Apr 2022 18:56:26 +0100 Subject: [PATCH 2/2] sysdeps: Only open oom_score_adj read/write if we need to write it If we're running in a sandbox, we might not have write access to oom_score_adj. In the common case where we don't have any special protection from the OOM-killer, we can detect that with only read access, and skip the part where we open it for writing. (We would also not have write access to oom_score_adj if we're running with elevated Linux capabilities while not root, but that should never actually happen for dbus-daemon-launch-helper, which is setuid root for production use or has no capabilities during unit-testing.) Signed-off-by: Simon McVittie --- dbus/dbus-sysdeps-util-unix.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c index 8f079cbf..ca130cff 100644 --- a/dbus/dbus-sysdeps-util-unix.c +++ b/dbus/dbus-sysdeps-util-unix.c @@ -1627,12 +1627,12 @@ _dbus_reset_oom_score_adj (const char **error_str_p) const char *error_str = NULL; #ifdef O_CLOEXEC - fd = open ("/proc/self/oom_score_adj", O_RDWR | O_CLOEXEC); + fd = open ("/proc/self/oom_score_adj", O_RDONLY | O_CLOEXEC); #endif if (fd < 0) { - fd = open ("/proc/self/oom_score_adj", O_RDWR); + fd = open ("/proc/self/oom_score_adj", O_RDONLY); if (fd >= 0) _dbus_fd_set_close_on_exec (fd); } @@ -1680,6 +1680,26 @@ _dbus_reset_oom_score_adj (const char **error_str_p) goto out; } + close (fd); +#ifdef O_CLOEXEC + fd = open ("/proc/self/oom_score_adj", O_WRONLY | O_CLOEXEC); + + if (fd < 0) +#endif + { + fd = open ("/proc/self/oom_score_adj", O_WRONLY); + if (fd >= 0) + _dbus_fd_set_close_on_exec (fd); + } + + if (fd < 0) + { + ret = FALSE; + error_str = "open(/proc/self/oom_score_adj) for writing"; + saved_errno = errno; + goto out; + } + if (pwrite (fd, "0", sizeof (char), 0) < 0) { ret = FALSE; @@ -1700,7 +1720,7 @@ _dbus_reset_oom_score_adj (const char **error_str_p) else { ret = FALSE; - error_str = "open(/proc/self/oom_score_adj)"; + error_str = "open(/proc/self/oom_score_adj) for reading"; saved_errno = errno; goto out; } -- GitLab