From 6fdb8de42fbd314f1d06019f9ba4c4a060898e5f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Jan 2018 17:33:07 +0100 Subject: [PATCH 1/5] log: make log_set_upgrade_syslog_to_journal() take effect immediately This doesn't matter much, and we don't rely on it, but I think it's much nicer if we log_set_target() and log_set_upgrade_syslog_to_journal() can be called in either order and have the same effect. --- src/basic/log.c | 8 ++++++++ src/basic/log.h | 1 + src/core/main.c | 1 + 3 files changed, 10 insertions(+) diff --git a/src/basic/log.c b/src/basic/log.c index 9169474289..8a65aa349e 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -1314,6 +1314,14 @@ int log_syntax_invalid_utf8_internal( void log_set_upgrade_syslog_to_journal(bool b) { upgrade_syslog_to_journal = b; + + /* Make the change effective immediately */ + if (b) { + if (log_target == LOG_TARGET_SYSLOG) + log_target = LOG_TARGET_JOURNAL; + else if (log_target == LOG_TARGET_SYSLOG_OR_KMSG) + log_target = LOG_TARGET_JOURNAL_OR_KMSG; + } } void log_set_always_reopen_console(bool b) { diff --git a/src/basic/log.h b/src/basic/log.h index b97cdcd703..910a89a255 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -302,6 +302,7 @@ LogTarget log_target_from_string(const char *s) _pure_; void log_received_signal(int level, const struct signalfd_siginfo *si); +/* If turned on, any requests for a log target involving "syslog" will be implicitly upgraded to the equivalent journal target */ void log_set_upgrade_syslog_to_journal(bool b); void log_set_always_reopen_console(bool b); void log_set_open_when_needed(bool b); diff --git a/src/core/main.c b/src/core/main.c index 7d20f4e67b..9ecb8e9835 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2328,6 +2328,7 @@ int main(int argc, char *argv[]) { saved_argv = argv; saved_argc = argc; + /* Make sure that if the user says "syslog" we actually log to the journal. */ log_set_upgrade_syslog_to_journal(true); if (getpid_cached() == 1) { From adf47c919abbd62e287ebaad9e2ade38ce66dcbf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Jan 2018 17:36:25 +0100 Subject: [PATCH 2/5] log: add new "prohibit_ipc" flag to logging system If set, we'll avoid logging to any IPC log targets, i.e. syslog or the journal, but allow stderr, kmsg, console logging. This is useful as PID 1 wants to turn this off explicitly as long as the journal is not up. Previously we'd open/close the log stream to these services whenever needed but this is incompatible with the "open_when_needed" logic introduced in #6915, which might open the log streams whenever it likes, including possibly inside of the child process we fork off that'll become journald later on. Hence, let's make this all explicit, and instead of managing when we open/close log streams add a boolean that clearly prohibits the IPC targets when needed, so that opening can be done at any time, but will honour this. See: #7985 --- src/basic/log.c | 11 +++++++++-- src/basic/log.h | 4 ++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/basic/log.c b/src/basic/log.c index 8a65aa349e..ae0ac72b57 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -77,6 +77,7 @@ static bool show_location = false; static bool upgrade_syslog_to_journal = false; static bool always_reopen_console = false; static bool open_when_needed = false; +static bool prohibit_ipc = false; /* Akin to glibc's __abort_msg; which is private and we hence cannot * use here. */ @@ -254,7 +255,8 @@ int log_open(void) { getpid_cached() == 1 || isatty(STDERR_FILENO) <= 0) { - if (IN_SET(log_target, LOG_TARGET_AUTO, + if (!prohibit_ipc && + IN_SET(log_target, LOG_TARGET_AUTO, LOG_TARGET_JOURNAL_OR_KMSG, LOG_TARGET_JOURNAL)) { r = log_open_journal(); @@ -265,7 +267,8 @@ int log_open(void) { } } - if (IN_SET(log_target, LOG_TARGET_SYSLOG_OR_KMSG, + if (!prohibit_ipc && + IN_SET(log_target, LOG_TARGET_SYSLOG_OR_KMSG, LOG_TARGET_SYSLOG)) { r = log_open_syslog(); if (r >= 0) { @@ -1332,6 +1335,10 @@ void log_set_open_when_needed(bool b) { open_when_needed = b; } +void log_set_prohibit_ipc(bool b) { + prohibit_ipc = b; +} + int log_emergency_level(void) { /* Returns the log level to use for log_emergency() logging. We use LOG_EMERG only when we are PID 1, as only * then the system of the whole system is obviously affected. */ diff --git a/src/basic/log.h b/src/basic/log.h index 910a89a255..0a8fa22866 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -307,6 +307,10 @@ void log_set_upgrade_syslog_to_journal(bool b); void log_set_always_reopen_console(bool b); void log_set_open_when_needed(bool b); +/* If turned on, then we'll never use IPC-based logging, i.e. never log to syslog or the journal. We'll only log to + * stderr, the console or kmsg */ +void log_set_prohibit_ipc(bool b); + int log_syntax_internal( const char *unit, int level, From d075092f14ce02df39aee02bca2eb8a577a69076 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Jan 2018 17:42:12 +0100 Subject: [PATCH 3/5] pid1: make use of new "prohibit_ipc" logging flag in PID 1 Let's set it initially, and then toggle it only when we know its safe. --- src/core/main.c | 4 ++++ src/core/manager.c | 52 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 9ecb8e9835..245b4c076c 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2335,6 +2335,10 @@ int main(int argc, char *argv[]) { /* Disable the umask logic */ umask(0); + /* Make sure that at least initially we do not ever log to journald/syslogd, because it might not be activated + * yet (even though the log socket for it exists). */ + log_set_prohibit_ipc(true); + /* Always reopen /dev/console when running as PID 1 or one of its pre-execve() children. This is * important so that we never end up logging to any foreign stderr, for example if we have to log in a * child process right before execve()'ing the actual binary, at a point in time where socket diff --git a/src/core/manager.c b/src/core/manager.c index 4130b24972..2952f217c3 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3132,6 +3132,9 @@ int manager_reload(Manager *m) { manager_vacuum_uid_refs(m); manager_vacuum_gid_refs(m); + /* It might be safe to log to the journal now. */ + manager_recheck_journal(m); + /* Sync current state of bus names with our set of listening units */ if (m->api_bus) manager_sync_bus_names(m, m->api_bus); @@ -3482,29 +3485,52 @@ int manager_set_default_rlimits(Manager *m, struct rlimit **default_rlimit) { return 0; } -void manager_recheck_journal(Manager *m) { +static bool manager_journal_is_running(Manager *m) { Unit *u; assert(m); + /* If we are the user manager we can safely assume that the journal is up */ if (!MANAGER_IS_SYSTEM(m)) - return; + return true; + /* Check that the socket is not only up, but in RUNNING state */ u = manager_get_unit(m, SPECIAL_JOURNALD_SOCKET); - if (u && SOCKET(u)->state != SOCKET_RUNNING) { - log_close_journal(); - return; - } + if (!u) + return false; + if (SOCKET(u)->state != SOCKET_RUNNING) + return false; + /* Similar, check if the daemon itself is fully up, too */ u = manager_get_unit(m, SPECIAL_JOURNALD_SERVICE); - if (u && SERVICE(u)->state != SERVICE_RUNNING) { - log_close_journal(); - return; - } + if (!u) + return false; + if (SERVICE(u)->state != SERVICE_RUNNING) + return false; - /* Hmm, OK, so the socket is fully up and the service is up - * too, then let's make use of the thing. */ - log_open(); + return true; +} + +void manager_recheck_journal(Manager *m) { + + assert(m); + + /* Don't bother with this unless we are in the special situation of being PID 1 */ + if (getpid_cached() != 1) + return; + + if (manager_journal_is_running(m)) { + + /* The journal is fully and entirely up? If so, let's permit logging to it, if that's configured. */ + log_set_prohibit_ipc(false); + log_open(); + } else { + + /* If the journal is down, don't ever log to it, otherwise we might end up deadlocking ourselves as we + * might trigger an activation ourselves we can't fulfill */ + log_set_prohibit_ipc(true); + log_close_journal(); + } } void manager_set_show_status(Manager *m, ShowStatus mode) { From 7a3be263d2ac9ecb17072c956789c4882c230f1e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Jan 2018 17:45:14 +0100 Subject: [PATCH 4/5] log: add brief comment for log_set_open_when_needed() and log_set_always_reopen_console() These two deserve some explanation... --- src/basic/log.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/basic/log.h b/src/basic/log.h index 0a8fa22866..cc767ba1a9 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -304,7 +304,12 @@ void log_received_signal(int level, const struct signalfd_siginfo *si); /* If turned on, any requests for a log target involving "syslog" will be implicitly upgraded to the equivalent journal target */ void log_set_upgrade_syslog_to_journal(bool b); + +/* If turned on, and log_open() is called, we'll not use STDERR_FILENO for logging ever, but rather open /dev/console */ void log_set_always_reopen_console(bool b); + +/* If turned on, we'll open the log stream implicitly if needed on each individual log call. This is normally not + * desired as we want to reuse our logging streams. It is useful however */ void log_set_open_when_needed(bool b); /* If turned on, then we'll never use IPC-based logging, i.e. never log to syslog or the journal. We'll only log to From 6c347d5024e7451336d0c5d0f7eaf90ae11d9dc9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Jan 2018 18:01:01 +0100 Subject: [PATCH 5/5] log: remove LOG_TARGET_SAFE pseudo log target This removes LOG_TARGET_SAFE. It's made redundant by the new "prohibit-ipc" logging flag, as it used to have a similar effect: avoid logging to the journal/syslog, i.e. any local services in order to avoid deadlocks when we lock from PID 1 or its utility processes (such as generators). All previous users of LOG_TARGET_SAFE are switched over to the new setting. This makes things a bit safer for all, as not even the SYSTEMD_LOG_TARGET env var can be used to accidentally log to the journal anymore in these programs. --- src/basic/log.c | 7 ++----- src/basic/log.h | 1 - src/cryptsetup/cryptsetup-generator.c | 3 ++- src/debug-generator/debug-generator.c | 3 ++- src/fstab-generator/fstab-generator.c | 3 ++- src/getty-generator/getty-generator.c | 3 ++- src/gpt-auto-generator/gpt-auto-generator.c | 3 ++- src/hibernate-resume/hibernate-resume-generator.c | 3 ++- src/journal/journald.c | 3 ++- src/rc-local-generator/rc-local-generator.c | 3 ++- src/system-update-generator/system-update-generator.c | 3 ++- src/sysv-generator/sysv-generator.c | 3 ++- src/veritysetup/veritysetup-generator.c | 3 ++- 13 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/basic/log.c b/src/basic/log.c index ae0ac72b57..2b15ecba84 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -251,7 +251,7 @@ int log_open(void) { return 0; } - if (!IN_SET(log_target, LOG_TARGET_AUTO, LOG_TARGET_SAFE) || + if (log_target != LOG_TARGET_AUTO || getpid_cached() == 1 || isatty(STDERR_FILENO) <= 0) { @@ -279,7 +279,6 @@ int log_open(void) { } if (IN_SET(log_target, LOG_TARGET_AUTO, - LOG_TARGET_SAFE, LOG_TARGET_JOURNAL_OR_KMSG, LOG_TARGET_SYSLOG_OR_KMSG, LOG_TARGET_KMSG)) { @@ -631,7 +630,6 @@ int log_dispatch_internal( if (k <= 0 && IN_SET(log_target, LOG_TARGET_AUTO, - LOG_TARGET_SAFE, LOG_TARGET_SYSLOG_OR_KMSG, LOG_TARGET_JOURNAL_OR_KMSG, LOG_TARGET_KMSG)) { @@ -1223,8 +1221,7 @@ static const char *const log_target_table[_LOG_TARGET_MAX] = { [LOG_TARGET_SYSLOG] = "syslog", [LOG_TARGET_SYSLOG_OR_KMSG] = "syslog-or-kmsg", [LOG_TARGET_AUTO] = "auto", - [LOG_TARGET_SAFE] = "safe", - [LOG_TARGET_NULL] = "null" + [LOG_TARGET_NULL] = "null", }; DEFINE_STRING_TABLE_LOOKUP(log_target, LogTarget); diff --git a/src/basic/log.h b/src/basic/log.h index cc767ba1a9..f2c6cd8c0a 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -50,7 +50,6 @@ typedef enum LogTarget{ LOG_TARGET_SYSLOG, LOG_TARGET_SYSLOG_OR_KMSG, LOG_TARGET_AUTO, /* console if stderr is tty, JOURNAL_OR_KMSG otherwise */ - LOG_TARGET_SAFE, /* console if stderr is tty, KMSG otherwise */ LOG_TARGET_NULL, _LOG_TARGET_MAX, _LOG_TARGET_INVALID = -1 diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c index 0b46e2db2a..acde2a6a32 100644 --- a/src/cryptsetup/cryptsetup-generator.c +++ b/src/cryptsetup/cryptsetup-generator.c @@ -472,7 +472,8 @@ int main(int argc, char *argv[]) { if (argc > 1) arg_dest = argv[1]; - log_set_target(LOG_TARGET_SAFE); + log_set_prohibit_ipc(true); + log_set_target(LOG_TARGET_AUTO); log_parse_environment(); log_open(); diff --git a/src/debug-generator/debug-generator.c b/src/debug-generator/debug-generator.c index 604faa0d18..61c890d05a 100644 --- a/src/debug-generator/debug-generator.c +++ b/src/debug-generator/debug-generator.c @@ -165,7 +165,8 @@ int main(int argc, char *argv[]) { if (argc > 1) arg_dest = argv[2]; - log_set_target(LOG_TARGET_SAFE); + log_set_prohibit_ipc(true); + log_set_target(LOG_TARGET_AUTO); log_parse_environment(); log_open(); diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index 95bc5e38d5..f392f89099 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -891,7 +891,8 @@ int main(int argc, char *argv[]) { if (argc > 3) arg_dest_late = argv[3]; - log_set_target(LOG_TARGET_SAFE); + log_set_prohibit_ipc(true); + log_set_target(LOG_TARGET_AUTO); log_parse_environment(); log_open(); diff --git a/src/getty-generator/getty-generator.c b/src/getty-generator/getty-generator.c index 3f62a81abc..f475aef66c 100644 --- a/src/getty-generator/getty-generator.c +++ b/src/getty-generator/getty-generator.c @@ -138,7 +138,8 @@ int main(int argc, char *argv[]) { if (argc > 1) arg_dest = argv[1]; - log_set_target(LOG_TARGET_SAFE); + log_set_prohibit_ipc(true); + log_set_target(LOG_TARGET_AUTO); log_parse_environment(); log_open(); diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index 85899ab301..cbdbc5afe2 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -713,7 +713,8 @@ int main(int argc, char *argv[]) { if (argc > 1) arg_dest = argv[3]; - log_set_target(LOG_TARGET_SAFE); + log_set_prohibit_ipc(true); + log_set_target(LOG_TARGET_AUTO); log_parse_environment(); log_open(); diff --git a/src/hibernate-resume/hibernate-resume-generator.c b/src/hibernate-resume/hibernate-resume-generator.c index 01222db516..a81386ef63 100644 --- a/src/hibernate-resume/hibernate-resume-generator.c +++ b/src/hibernate-resume/hibernate-resume-generator.c @@ -86,7 +86,8 @@ int main(int argc, char *argv[]) { if (argc > 1) arg_dest = argv[1]; - log_set_target(LOG_TARGET_SAFE); + log_set_prohibit_ipc(true); + log_set_target(LOG_TARGET_AUTO); log_parse_environment(); log_open(); diff --git a/src/journal/journald.c b/src/journal/journald.c index 2724a2df95..10a6955769 100644 --- a/src/journal/journald.c +++ b/src/journal/journald.c @@ -40,7 +40,8 @@ int main(int argc, char *argv[]) { return EXIT_FAILURE; } - log_set_target(LOG_TARGET_SAFE); + log_set_prohibit_ipc(true); + log_set_target(LOG_TARGET_AUTO); log_set_facility(LOG_SYSLOG); log_parse_environment(); log_open(); diff --git a/src/rc-local-generator/rc-local-generator.c b/src/rc-local-generator/rc-local-generator.c index b10b18eb8e..2762fa7e89 100644 --- a/src/rc-local-generator/rc-local-generator.c +++ b/src/rc-local-generator/rc-local-generator.c @@ -64,7 +64,8 @@ int main(int argc, char *argv[]) { if (argc > 1) arg_dest = argv[1]; - log_set_target(LOG_TARGET_SAFE); + log_set_prohibit_ipc(true); + log_set_target(LOG_TARGET_AUTO); log_parse_environment(); log_open(); diff --git a/src/system-update-generator/system-update-generator.c b/src/system-update-generator/system-update-generator.c index 1796bae340..ec05e7b9e6 100644 --- a/src/system-update-generator/system-update-generator.c +++ b/src/system-update-generator/system-update-generator.c @@ -79,7 +79,8 @@ int main(int argc, char *argv[]) { if (argc > 1) arg_dest = argv[2]; - log_set_target(LOG_TARGET_SAFE); + log_set_prohibit_ipc(true); + log_set_target(LOG_TARGET_AUTO); log_parse_environment(); log_open(); diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c index 51306b5625..087ba08559 100644 --- a/src/sysv-generator/sysv-generator.c +++ b/src/sysv-generator/sysv-generator.c @@ -950,7 +950,8 @@ int main(int argc, char *argv[]) { if (argc > 1) arg_dest = argv[3]; - log_set_target(LOG_TARGET_SAFE); + log_set_prohibit_ipc(true); + log_set_target(LOG_TARGET_AUTO); log_parse_environment(); log_open(); diff --git a/src/veritysetup/veritysetup-generator.c b/src/veritysetup/veritysetup-generator.c index 24bee41243..d09f295392 100644 --- a/src/veritysetup/veritysetup-generator.c +++ b/src/veritysetup/veritysetup-generator.c @@ -228,7 +228,8 @@ int main(int argc, char *argv[]) { if (argc > 1) arg_dest = argv[1]; - log_set_target(LOG_TARGET_SAFE); + log_set_prohibit_ipc(true); + log_set_target(LOG_TARGET_AUTO); log_parse_environment(); log_open();