From b5cb2d2847a3516ba794acf07d08f70d988ff7c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 31 May 2022 15:02:56 +0200 Subject: [PATCH 1/5] tty-ask-password-agent: drop unnecessary code for non-absolute paths utmp_wall() always prepends /dev/, so we don't need to do this a second time here. --- src/tty-ask-password-agent/tty-ask-password-agent.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 7fc0ed7c8d..09835ca903 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -99,8 +99,7 @@ static bool wall_tty_match(const char *path, void *userdata) { _cleanup_close_ int fd = -1; struct stat st; - if (!path_is_absolute(path)) - path = strjoina("/dev/", path); + assert(path_is_absolute(path)); if (lstat(path, &st) < 0) { log_debug_errno(errno, "Failed to stat %s: %m", path); From 16618332388442f2f1c3e52b0a9fde00121564a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 31 May 2022 15:10:18 +0200 Subject: [PATCH 2/5] shared/utmp-wtmp: pass information if entry is local to filter function This just adds an unused parameter for future use. No change in behaviour. --- src/login/logind-utmp.c | 2 +- src/login/logind.h | 2 +- src/shared/utmp-wtmp.c | 11 +++++++---- src/shared/utmp-wtmp.h | 2 +- src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +- 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/login/logind-utmp.c b/src/login/logind-utmp.c index 1db5050c3b..ccea8f968f 100644 --- a/src/login/logind-utmp.c +++ b/src/login/logind-utmp.c @@ -42,7 +42,7 @@ _const_ static usec_t when_wall(usec_t n, usec_t elapse) { return left % USEC_PER_HOUR; } -bool logind_wall_tty_filter(const char *tty, void *userdata) { +bool logind_wall_tty_filter(const char *tty, bool is_local, void *userdata) { Manager *m = userdata; const char *p; diff --git a/src/login/logind.h b/src/login/logind.h index 2136486c60..27f9e9729f 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -180,6 +180,6 @@ CONFIG_PARSER_PROTOTYPE(config_parse_n_autovts); CONFIG_PARSER_PROTOTYPE(config_parse_tmpfs_size); int manager_setup_wall_message_timer(Manager *m); -bool logind_wall_tty_filter(const char *tty, void *userdata); +bool logind_wall_tty_filter(const char *tty, bool is_local, void *userdata); int manager_read_efi_boot_loader_entries(Manager *m); diff --git a/src/shared/utmp-wtmp.c b/src/shared/utmp-wtmp.c index f2f53380ad..c7e89ba6a6 100644 --- a/src/shared/utmp-wtmp.c +++ b/src/shared/utmp-wtmp.c @@ -337,7 +337,7 @@ int utmp_wall( const char *message, const char *username, const char *origin_tty, - bool (*match_tty)(const char *tty, void *userdata), + bool (*match_tty)(const char *tty, bool is_local, void *userdata), void *userdata) { _unused_ _cleanup_(utxent_cleanup) bool utmpx = false; @@ -381,17 +381,20 @@ int utmp_wall( if (u->ut_type != USER_PROCESS || u->ut_user[0] == 0) continue; - /* this access is fine, because STRLEN("/dev/") << 32 (UT_LINESIZE) */ + /* This access is fine, because strlen("/dev/") < 32 (UT_LINESIZE) */ if (path_startswith(u->ut_line, "/dev/")) path = u->ut_line; else { if (asprintf(&buf, "/dev/%.*s", (int) sizeof(u->ut_line), u->ut_line) < 0) return -ENOMEM; - path = buf; } - if (!match_tty || match_tty(path, userdata)) { + /* It seems that the address field is always set for remote logins. + * For local logins and other local entries, we get [0,0,0,0]. */ + bool is_local = memeqzero(u->ut_addr_v6, sizeof(u->ut_addr_v6)); + + if (!match_tty || match_tty(path, is_local, userdata)) { q = write_to_terminal(path, text); if (q < 0) r = q; diff --git a/src/shared/utmp-wtmp.h b/src/shared/utmp-wtmp.h index 3e71f76b27..36e4203b4f 100644 --- a/src/shared/utmp-wtmp.h +++ b/src/shared/utmp-wtmp.h @@ -23,7 +23,7 @@ int utmp_wall( const char *message, const char *username, const char *origin_tty, - bool (*match_tty)(const char *tty, void *userdata), + bool (*match_tty)(const char *tty, bool is_local, void *userdata), void *userdata); static inline bool utxent_start(void) { diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 09835ca903..b7d279cc22 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -94,7 +94,7 @@ static int send_passwords(const char *socket_name, char **passwords) { return (int) n; } -static bool wall_tty_match(const char *path, void *userdata) { +static bool wall_tty_match(const char *path, bool is_local, void *userdata) { _cleanup_free_ char *p = NULL; _cleanup_close_ int fd = -1; struct stat st; From e31355bbc14e6ee96162c56c5e625aafcf42f9ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 31 May 2022 15:23:35 +0200 Subject: [PATCH 3/5] tests: add a helper that dumps /run/utmp in detail utmpdump doesn't print all the details. Looking at the list if useful when trying to tweak the wall filtering logic. This doesn't do much, but at least it serves as a smoke test for the cleanup functions. --- src/test/meson.build | 2 ++ src/test/test-utmp.c | 61 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 src/test/test-utmp.c diff --git a/src/test/meson.build b/src/test/meson.build index d9304d538b..d2bdd201ee 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -614,6 +614,8 @@ tests += [ [files('test-journal-importer.c')], + [files('test-utmp.c')], + [files('test-udev.c'), [libudevd_core, libshared], diff --git a/src/test/test-utmp.c b/src/test/test-utmp.c new file mode 100644 index 0000000000..2f66003e65 --- /dev/null +++ b/src/test/test-utmp.c @@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "format-util.h" +#include "socket-util.h" +#include "stdio-util.h" +#include "string-util.h" +#include "utmp-wtmp.h" +#include "tests.h" + +#ifndef UT_LINESIZE +# define UT_LINESIZE 32 +#endif +#ifndef UT_NAMESIZE +# define UT_NAMESIZE 32 +#endif +#ifndef UT_HOSTSIZE +# define UT_HOSTSIZE 256 +#endif + +TEST(dump_run_utmp) { + _unused_ _cleanup_(utxent_cleanup) bool utmpx = false; + + utmpx = utxent_start(); + + for (struct utmpx *u; (u = getutxent()); ) { + char _type_buf[DECIMAL_STR_MAX(short)]; + const char *type = + u->ut_type == EMPTY ? "EMPTY" : + u->ut_type == RUN_LVL ? "RUN_LVL" : + u->ut_type == BOOT_TIME ? "BOOT_TIME" : + u->ut_type == NEW_TIME ? "NEW_TIME" : + u->ut_type == OLD_TIME ? "OLD_TIME" : + u->ut_type == INIT_PROCESS ? "INIT_PROCESS" : + u->ut_type == LOGIN_PROCESS ? "LOGIN_PROCESS" : + u->ut_type == USER_PROCESS ? "USER_PROCESS" : + u->ut_type == DEAD_PROCESS ? "DEAD_PROCESS" : + u->ut_type == ACCOUNTING ? "ACCOUNTING" : + _type_buf; + if (type == _type_buf) + xsprintf(_type_buf, "%hd", u->ut_type); + + union in_addr_union addr = {}; + memcpy(&addr, u->ut_addr_v6, MIN(sizeof(addr), sizeof(u->ut_addr_v6))); + _cleanup_free_ char *pretty = NULL; + bool is_ipv4 = memeqzero((const uint8_t*) &addr + 4, sizeof(addr) - 4); + (void) in_addr_to_string(is_ipv4 ? AF_INET : AF_INET6, + &addr, &pretty); + + log_info("%14s %10"PID_PRI" line=%-7.*s id=%-4.4s name=%-8.*s session=%lu host=%.*s addr=%s", + type, + u->ut_pid, + UT_LINESIZE, u->ut_line, + u->ut_id, + UT_NAMESIZE, u->ut_user, + (long unsigned) u->ut_session, + UT_HOSTSIZE, u->ut_host, + strempty(pretty)); + } +} + +DEFINE_TEST_MAIN(LOG_DEBUG); From 51a2b575d751c257f2603f12fe9bb883014c37c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 31 May 2022 15:17:52 +0200 Subject: [PATCH 4/5] logind: do not print wall messages to local pseudoterminals Fixes #23520. Replaces #23555. The problem started with cdf370626f08ed509a5dde9d5618eed29d625032 and 90b1ec03b2ce939f589239133a32f4429f2ad6a6 which together started printing the wall message in more cases. The motivation for those change was reasonable, but this clearly causes problems described in #23520: users are getting unexpected wall messages. Xterm, urxvt, (anything using libutempter?), and tmux (in some configurations), register local pty sessions in utmp. So let's try to suppress the message for local pseudo-terminal logins. This patch based on #23538, but instead of filtering just on /dev/pts, it uses the .ut_addr_v6 to only filter out local entries. --- src/login/logind-utmp.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/login/logind-utmp.c b/src/login/logind-utmp.c index ccea8f968f..138c01e9b2 100644 --- a/src/login/logind-utmp.c +++ b/src/login/logind-utmp.c @@ -43,19 +43,17 @@ _const_ static usec_t when_wall(usec_t n, usec_t elapse) { } bool logind_wall_tty_filter(const char *tty, bool is_local, void *userdata) { - Manager *m = userdata; - const char *p; + Manager *m = ASSERT_PTR(userdata); - assert(m); - - if (!m->scheduled_shutdown_tty) - return true; - - p = path_startswith(tty, "/dev/"); + const char *p = path_startswith(tty, "/dev/"); if (!p) return true; - return !streq(p, m->scheduled_shutdown_tty); + /* Do not write to local pseudo-terminals */ + if (startswith(p, "pts/") && is_local) + return false; + + return !streq_ptr(p, m->scheduled_shutdown_tty); } static int warn_wall(Manager *m, usec_t n) { From ea74f39c24344eafc238d1c69155bd5aca5f2e08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Fri, 27 May 2022 13:47:23 +0200 Subject: [PATCH 5/5] login: do not issue wall messages on local terminals for suspend and hibernate Fixes: #23520 [zjs: I added the comment and tweaked the patch a bit. The call to reset_scheduled_shutdown() is moved down a bit to allow the callback to have access to information about the operation being cancelled. This all happens within the same function, so there should be no observable change in behaviour.] --- src/login/logind-dbus.c | 4 ++-- src/login/logind-utmp.c | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index f7cff58005..c40d8defaf 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2342,8 +2342,6 @@ static int method_cancel_scheduled_shutdown(sd_bus_message *message, void *userd if (r == 0) return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ - reset_scheduled_shutdown(m); - if (m->enable_wall_messages) { _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; _cleanup_free_ char *username = NULL; @@ -2361,6 +2359,8 @@ static int method_cancel_scheduled_shutdown(sd_bus_message *message, void *userd username, tty, logind_wall_tty_filter, m); } + reset_scheduled_shutdown(m); + return sd_bus_reply_method_return(message, "b", true); } diff --git a/src/login/logind-utmp.c b/src/login/logind-utmp.c index 138c01e9b2..7d761a0d67 100644 --- a/src/login/logind-utmp.c +++ b/src/login/logind-utmp.c @@ -45,12 +45,22 @@ _const_ static usec_t when_wall(usec_t n, usec_t elapse) { bool logind_wall_tty_filter(const char *tty, bool is_local, void *userdata) { Manager *m = ASSERT_PTR(userdata); + assert(m->scheduled_shutdown_action); + const char *p = path_startswith(tty, "/dev/"); if (!p) return true; - /* Do not write to local pseudo-terminals */ - if (startswith(p, "pts/") && is_local) + /* Do not send information about events which do not destroy local sessions to local terminals. We + * can assume that if the system enters sleep or hibernation, this will be visible in an obvious way + * for any local user. And once the systems exits sleep or hibernation, the notication would be just + * noise, in particular for auto-suspend. */ + if (is_local && + IN_SET(m->scheduled_shutdown_action->handle, + HANDLE_SUSPEND, + HANDLE_HIBERNATE, + HANDLE_HYBRID_SLEEP, + HANDLE_SUSPEND_THEN_HIBERNATE)) return false; return !streq_ptr(p, m->scheduled_shutdown_tty);