From 1b24357c4176047cc9b450888bb54b52df83afdd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Aug 2024 10:30:19 +0200 Subject: [PATCH 1/3] terminal-util: fix isatty_safe() on hung-up TTYs glibc returs EIO on ttys that are hung up. That's not really correct, POSIX seems to disagree. Work around this in our code, and turn this into a clean "1", since a hung up tty doesn't stop being a tty just because it is hung up. Background: https://github.com/systemd/systemd/pull/34039 --- src/basic/terminal-util.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 6088320186..4219ac5ee9 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -65,6 +65,12 @@ bool isatty_safe(int fd) { if (isatty(fd)) return true; + /* Linux/glibc returns EIO for hung up TTY on isatty(). Which is wrong, the thing doesn't stop being + * a TTY after all, just because it is temporarily hung up. Let's work around this here, until this + * is fixed in glibc. See: https://sourceware.org/bugzilla/show_bug.cgi?id=32103 */ + if (errno == EIO) + return true; + /* Be resilient if we're working on stdio, since they're set up by parent process. */ assert(errno != EBADF || IN_SET(fd, STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO)); From aae47bf7a352f2ae8cca14187b31e6a00a01bd93 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Aug 2024 10:32:14 +0200 Subject: [PATCH 2/3] terminal-util: don't assume errno is correctly set when using isatty_safe() let's instead generate ENOTTY on our own. This is more correct with out coding style (since we generally do not propagate errors via errno), and also addresses #34039 as side effect. (#34039 really needs to be fixed in musl though, too, this is just a work-around as side-effect). Fixes: #34039 --- src/basic/terminal-util.c | 6 +++--- src/getty-generator/getty-generator.c | 2 +- src/shared/wall.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 4219ac5ee9..d6de92ee89 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -287,7 +287,7 @@ int open_terminal(const char *name, int mode) { } if (!isatty_safe(fd)) - return negative_errno(); + return -ENOTTY; return TAKE_FD(fd); } @@ -1508,7 +1508,7 @@ int vt_restore(int fd) { assert(fd >= 0); if (!isatty_safe(fd)) - return log_debug_errno(errno, "Asked to restore the VT for an fd that does not refer to a terminal: %m"); + return log_debug_errno(SYNTHETIC_ERRNO(ENOTTY), "Asked to restore the VT for an fd that does not refer to a terminal: %m"); if (ioctl(fd, KDSETMODE, KD_TEXT) < 0) RET_GATHER(ret, log_debug_errno(errno, "Failed to set VT to text mode, ignoring: %m")); @@ -1535,7 +1535,7 @@ int vt_release(int fd, bool restore) { * VT-switching modes. */ if (!isatty_safe(fd)) - return log_debug_errno(errno, "Asked to release the VT for an fd that does not refer to a terminal: %m"); + return log_debug_errno(SYNTHETIC_ERRNO(ENOTTY), "Asked to release the VT for an fd that does not refer to a terminal: %m"); if (ioctl(fd, VT_RELDISP, 1) < 0) return -errno; diff --git a/src/getty-generator/getty-generator.c b/src/getty-generator/getty-generator.c index acce71b1a8..9ad5597b05 100644 --- a/src/getty-generator/getty-generator.c +++ b/src/getty-generator/getty-generator.c @@ -95,7 +95,7 @@ static int verify_tty(const char *name) { return -errno; if (!isatty_safe(fd)) - return -errno; + return -ENOTTY; return 0; } diff --git a/src/shared/wall.c b/src/shared/wall.c index c5d6439db6..119d18f5a4 100644 --- a/src/shared/wall.c +++ b/src/shared/wall.c @@ -32,7 +32,7 @@ static int write_to_terminal(const char *tty, const char *message) { return -errno; if (!isatty_safe(fd)) - return -errno; + return -ENOTTY; return loop_write_full(fd, message, SIZE_MAX, TIMEOUT_USEC); } From 300b7e7620e03cf85efcf821b2c7a6872266accc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Aug 2024 10:33:52 +0200 Subject: [PATCH 3/3] tree-wide: use isatty_safe() more --- src/analyze/analyze-srk.c | 2 +- src/basic/terminal-util.c | 4 ++-- src/basic/user-util.c | 3 ++- src/busctl/busctl.c | 2 +- src/core/exec-invoke.c | 2 +- src/dissect/dissect.c | 8 ++++---- src/journal/journald.c | 2 +- src/nspawn/nspawn.c | 6 +++--- src/partition/repart.c | 2 +- src/run/run.c | 6 +++--- src/shared/ask-password-agent.c | 6 +++--- src/shared/ask-password-api.c | 4 ++-- src/shared/exec-util.c | 4 ++-- src/shared/format-table.c | 2 +- src/shared/polkit-agent.c | 3 ++- src/storagetm/storagetm.c | 4 ++-- src/test/test-format-table.c | 3 ++- src/varlinkctl/varlinkctl.c | 2 +- src/vmspawn/vmspawn-register.c | 3 ++- 19 files changed, 36 insertions(+), 32 deletions(-) diff --git a/src/analyze/analyze-srk.c b/src/analyze/analyze-srk.c index acfd8b0c47..adbc51316a 100644 --- a/src/analyze/analyze-srk.c +++ b/src/analyze/analyze-srk.c @@ -33,7 +33,7 @@ int verb_srk(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to marshal SRK: %m"); - if (isatty(STDOUT_FILENO)) + if (isatty_safe(STDOUT_FILENO)) return log_error_errno(SYNTHETIC_ERRNO(EIO), "Refusing to write binary data to TTY, please redirect output to file."); diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index d6de92ee89..a97312c9bf 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -1052,8 +1052,8 @@ bool on_tty(void) { if (cached_on_tty < 0) cached_on_tty = - isatty(STDOUT_FILENO) > 0 && - isatty(STDERR_FILENO) > 0; + isatty_safe(STDOUT_FILENO) && + isatty_safe(STDERR_FILENO); return cached_on_tty; } diff --git a/src/basic/user-util.c b/src/basic/user-util.c index 6bdf5bf1cd..858d712189 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -27,6 +27,7 @@ #include "random-util.h" #include "string-util.h" #include "strv.h" +#include "terminal-util.h" #include "user-util.h" #include "utf8.h" @@ -120,7 +121,7 @@ char* getlogname_malloc(void) { uid_t uid; struct stat st; - if (isatty(STDIN_FILENO) && fstat(STDIN_FILENO, &st) >= 0) + if (isatty_safe(STDIN_FILENO) && fstat(STDIN_FILENO, &st) >= 0) uid = st.st_uid; else uid = getuid(); diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 3735337a80..1b0b72085e 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -1401,7 +1401,7 @@ static int verb_capture(int argc, char **argv, void *userdata) { "busctl (systemd) " PROJECT_VERSION_FULL " (Git " GIT_VERSION ")"; int r; - if (isatty(STDOUT_FILENO)) + if (isatty_safe(STDOUT_FILENO)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Refusing to write message data to console, please redirect output to a file."); diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index 6bef4e7675..a867480300 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -354,7 +354,7 @@ static int setup_input( return -errno; /* Try to make this the controlling tty, if it is a tty */ - if (isatty(STDIN_FILENO)) + if (isatty_safe(STDIN_FILENO)) (void) ioctl(STDIN_FILENO, TIOCSCTTY, context->std_input == EXEC_INPUT_TTY_FORCE); return STDIN_FILENO; diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index 1ea2f248d8..1cad45c8ff 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -718,7 +718,7 @@ static int parse_argv(int argc, char *argv[]) { if (!IN_SET(arg_action, ACTION_DISSECT, ACTION_LIST, ACTION_MTREE, ACTION_COPY_FROM, ACTION_COPY_TO, ACTION_DISCOVER, ACTION_VALIDATE) && geteuid() != 0) return log_error_errno(SYNTHETIC_ERRNO(EPERM), "Need to be root."); - SET_FLAG(arg_flags, DISSECT_IMAGE_ALLOW_INTERACTIVE_AUTH, isatty(STDIN_FILENO)); + SET_FLAG(arg_flags, DISSECT_IMAGE_ALLOW_INTERACTIVE_AUTH, isatty_safe(STDIN_FILENO)); return 1; } @@ -1639,7 +1639,7 @@ static int action_list_or_mtree_or_copy_or_make_archive(DissectedImage *m, LoopD r = sym_archive_write_open_FILE(a, f); } else { - if (isatty(STDOUT_FILENO)) + if (isatty_safe(STDOUT_FILENO)) return log_error_errno(SYNTHETIC_ERRNO(EBADF), "Refusing to write archive to TTY."); r = sym_archive_write_open_fd(a, STDOUT_FILENO); @@ -1999,12 +1999,12 @@ static int action_validate(void) { if (r < 0) return r; - if (isatty(STDOUT_FILENO) && emoji_enabled()) + if (isatty_safe(STDOUT_FILENO) && emoji_enabled()) printf("%s ", special_glyph(SPECIAL_GLYPH_SPARKLES)); printf("%sOK%s", ansi_highlight_green(), ansi_normal()); - if (isatty(STDOUT_FILENO) && emoji_enabled()) + if (isatty_safe(STDOUT_FILENO) && emoji_enabled()) printf(" %s", special_glyph(SPECIAL_GLYPH_SPARKLES)); putc('\n', stdout); diff --git a/src/journal/journald.c b/src/journal/journald.c index 87ed22e7fe..84f44c46b4 100644 --- a/src/journal/journald.c +++ b/src/journal/journald.c @@ -36,7 +36,7 @@ static int run(int argc, char *argv[]) { * daemon when it comes to logging hence LOG_TARGET_AUTO won't do the right thing for * us. Hence explicitly log to the console if we're started from a console or to kmsg * otherwise. */ - log_target = isatty(STDERR_FILENO) ? LOG_TARGET_CONSOLE : LOG_TARGET_KMSG; + log_target = isatty_safe(STDERR_FILENO) ? LOG_TARGET_CONSOLE : LOG_TARGET_KMSG; log_set_prohibit_ipc(true); /* better safe than sorry */ log_set_target(log_target); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 4a9693f3b7..8c68a2058c 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -290,7 +290,7 @@ static int handle_arg_console(const char *arg) { else if (streq(arg, "passive")) arg_console_mode = CONSOLE_PASSIVE; else if (streq(arg, "pipe")) { - if (isatty(STDIN_FILENO) && isatty(STDOUT_FILENO)) + if (isatty_safe(STDIN_FILENO) && isatty(STDOUT_FILENO)) log_full(arg_quiet ? LOG_DEBUG : LOG_NOTICE, "Console mode 'pipe' selected, but standard input/output are connected to an interactive TTY. " "Most likely you want to use 'interactive' console mode for proper interactivity and shell job control. " @@ -298,7 +298,7 @@ static int handle_arg_console(const char *arg) { arg_console_mode = CONSOLE_PIPE; } else if (streq(arg, "autopipe")) { - if (isatty(STDIN_FILENO) && isatty(STDOUT_FILENO)) + if (isatty_safe(STDIN_FILENO) && isatty(STDOUT_FILENO)) arg_console_mode = CONSOLE_INTERACTIVE; else arg_console_mode = CONSOLE_PIPE; @@ -5883,7 +5883,7 @@ static int run(int argc, char *argv[]) { umask(0022); if (arg_console_mode < 0) - arg_console_mode = isatty(STDIN_FILENO) && isatty(STDOUT_FILENO) ? + arg_console_mode = isatty_safe(STDIN_FILENO) && isatty(STDOUT_FILENO) ? CONSOLE_INTERACTIVE : CONSOLE_READ_ONLY; if (arg_console_mode == CONSOLE_PIPE) /* if we pass STDERR on to the container, don't add our own logs into it too */ diff --git a/src/partition/repart.c b/src/partition/repart.c index 7db493cafc..6f03161702 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -7701,7 +7701,7 @@ static int parse_argv(int argc, char *argv[]) { return log_oom(); } - if (arg_pretty < 0 && isatty(STDOUT_FILENO)) + if (arg_pretty < 0 && isatty_safe(STDOUT_FILENO)) arg_pretty = true; if (arg_architecture >= 0) { diff --git a/src/run/run.c b/src/run/run.c index 2db60e6bb8..7fd76ebced 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -674,7 +674,7 @@ static int parse_argv(int argc, char *argv[]) { /* If we both --pty and --pipe are specified we'll automatically pick --pty if we are connected fully * to a TTY and pick direct fd passing otherwise. This way, we automatically adapt to usage in a shell * pipeline, but we are neatly interactive with tty-level isolation otherwise. */ - arg_stdio = isatty(STDIN_FILENO) && isatty(STDOUT_FILENO) && isatty(STDERR_FILENO) ? + arg_stdio = isatty_safe(STDIN_FILENO) && isatty(STDOUT_FILENO) && isatty(STDERR_FILENO) ? ARG_STDIO_PTY : ARG_STDIO_DIRECT; @@ -908,7 +908,7 @@ static int parse_argv_sudo_mode(int argc, char *argv[]) { arg_wait = true; arg_aggressive_gc = true; - arg_stdio = isatty(STDIN_FILENO) && isatty(STDOUT_FILENO) && isatty(STDERR_FILENO) ? ARG_STDIO_PTY : ARG_STDIO_DIRECT; + arg_stdio = isatty_safe(STDIN_FILENO) && isatty(STDOUT_FILENO) && isatty(STDERR_FILENO) ? ARG_STDIO_PTY : ARG_STDIO_DIRECT; arg_expand_environment = false; arg_send_sighup = true; @@ -1176,7 +1176,7 @@ static int transient_service_set_properties(sd_bus_message *m, const char *pty_p if (r < 0) return bus_log_create_error(r); - send_term = isatty(STDIN_FILENO) || isatty(STDOUT_FILENO) || isatty(STDERR_FILENO); + send_term = isatty_safe(STDIN_FILENO) || isatty(STDOUT_FILENO) || isatty(STDERR_FILENO); } if (send_term) { diff --git a/src/shared/ask-password-agent.c b/src/shared/ask-password-agent.c index 60281d3ae9..e425f825db 100644 --- a/src/shared/ask-password-agent.c +++ b/src/shared/ask-password-agent.c @@ -8,6 +8,7 @@ #include "exec-util.h" #include "log.h" #include "process-util.h" +#include "terminal-util.h" static pid_t agent_pid = 0; @@ -17,9 +18,8 @@ int ask_password_agent_open(void) { if (agent_pid > 0) return 0; - /* We check STDIN here, not STDOUT, since this is about input, - * not output */ - if (!isatty(STDIN_FILENO)) + /* We check STDIN here, not STDOUT, since this is about input, not output */ + if (!isatty_safe(STDIN_FILENO)) return 0; if (!is_main_thread()) diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index 778b31274d..7706c3c271 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -988,14 +988,14 @@ int ask_password_auto( if (FLAGS_SET(flags, ASK_PASSWORD_ACCEPT_CACHED) && req && req->keyring && - (FLAGS_SET(flags, ASK_PASSWORD_NO_TTY) || !isatty(STDIN_FILENO)) && + (FLAGS_SET(flags, ASK_PASSWORD_NO_TTY) || !isatty_safe(STDIN_FILENO)) && FLAGS_SET(flags, ASK_PASSWORD_NO_AGENT)) { r = ask_password_keyring(req, flags, ret); if (r != -ENOKEY) return r; } - if (!FLAGS_SET(flags, ASK_PASSWORD_NO_TTY) && isatty(STDIN_FILENO)) + if (!FLAGS_SET(flags, ASK_PASSWORD_NO_TTY) && isatty_safe(STDIN_FILENO)) return ask_password_tty(-EBADF, req, until, flags, NULL, ret); if (!FLAGS_SET(flags, ASK_PASSWORD_NO_AGENT)) diff --git a/src/shared/exec-util.c b/src/shared/exec-util.c index 213d660a2e..870f8f66d8 100644 --- a/src/shared/exec-util.c +++ b/src/shared/exec-util.c @@ -555,8 +555,8 @@ int _fork_agent(const char *name, const int except[], size_t n_except, pid_t *re /* In the child: */ - stdout_is_tty = isatty(STDOUT_FILENO); - stderr_is_tty = isatty(STDERR_FILENO); + stdout_is_tty = isatty_safe(STDOUT_FILENO); + stderr_is_tty = isatty_safe(STDERR_FILENO); if (!stdout_is_tty || !stderr_is_tty) { int fd; diff --git a/src/shared/format-table.c b/src/shared/format-table.c index ee641d1144..4cf4b67f6d 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -2391,7 +2391,7 @@ int table_print(Table *t, FILE *f) { if (t->width != 0 && t->width != SIZE_MAX) table_effective_width = t->width; else if (t->width == 0 || - ((pass > 0 || !any_soft) && (pager_have() || !isatty(STDOUT_FILENO)))) + ((pass > 0 || !any_soft) && (pager_have() || !isatty_safe(STDOUT_FILENO)))) table_effective_width = table_requested_width; else table_effective_width = MIN(table_requested_width, columns()); diff --git a/src/shared/polkit-agent.c b/src/shared/polkit-agent.c index 2f38965cf2..92e576c9d2 100644 --- a/src/shared/polkit-agent.c +++ b/src/shared/polkit-agent.c @@ -14,6 +14,7 @@ #include "polkit-agent.h" #include "process-util.h" #include "stdio-util.h" +#include "terminal-util.h" #include "time-util.h" #if ENABLE_POLKIT @@ -31,7 +32,7 @@ int polkit_agent_open(void) { return 0; /* We check STDIN here, not STDOUT, since this is about input, not output */ - if (!isatty(STDIN_FILENO)) + if (!isatty_safe(STDIN_FILENO)) return 0; if (!is_main_thread()) diff --git a/src/storagetm/storagetm.c b/src/storagetm/storagetm.c index 910746dc34..ca8e886d37 100644 --- a/src/storagetm/storagetm.c +++ b/src/storagetm/storagetm.c @@ -1065,7 +1065,7 @@ static int on_display_refresh(sd_event_source *s, uint64_t usec, void *userdata) c->display_refresh_scheduled = false; - if (isatty(STDERR_FILENO)) + if (isatty_safe(STDERR_FILENO)) fputs(ANSI_HOME_CLEAR, stderr); /* If we have both IPv4 and IPv6, we display IPv4 info via Plymouth, since it doesn't have much @@ -1247,7 +1247,7 @@ static int run(int argc, char* argv[]) { if (r < 0) return log_error_errno(r, "Failed to subscribe to RTM_DELADDR events: %m"); - if (isatty(STDIN_FILENO)) + if (isatty_safe(STDIN_FILENO)) log_info("Hit Ctrl-C to exit target mode."); _unused_ _cleanup_(notify_on_cleanup) const char *notify_message = diff --git a/src/test/test-format-table.c b/src/test/test-format-table.c index 41621dbb5b..f074c5b6f7 100644 --- a/src/test/test-format-table.c +++ b/src/test/test-format-table.c @@ -7,6 +7,7 @@ #include "json-util.h" #include "string-util.h" #include "strv.h" +#include "terminal-util.h" #include "tests.h" #include "time-util.h" @@ -562,7 +563,7 @@ TEST(table) { assert_se(table_format(t, &formatted) >= 0); printf("%s\n", formatted); - if (isatty(STDOUT_FILENO)) + if (isatty_safe(STDOUT_FILENO)) assert_se(streq(formatted, "no a long f… no a long f… a long fi…\n" "no fäää no fäää fäää\n" diff --git a/src/varlinkctl/varlinkctl.c b/src/varlinkctl/varlinkctl.c index aae6032da5..77cecdbf4e 100644 --- a/src/varlinkctl/varlinkctl.c +++ b/src/varlinkctl/varlinkctl.c @@ -504,7 +504,7 @@ static int verb_call(int argc, char *argv[], void *userdata) { /* is correct, as dispatch_verb() shifts arguments by one for the verb. */ r = sd_json_parse_with_source(parameter, source, 0, &jp, &line, &column); } else { - if (isatty(STDIN_FILENO) > 0 && !arg_quiet) + if (isatty_safe(STDIN_FILENO) && !arg_quiet) log_notice("Expecting method call parameter JSON object on standard input. (Provide empty string or {} for no parameters.)"); source = ""; diff --git a/src/vmspawn/vmspawn-register.c b/src/vmspawn/vmspawn-register.c index 474cbd5df9..20e6def360 100644 --- a/src/vmspawn/vmspawn-register.c +++ b/src/vmspawn/vmspawn-register.c @@ -12,6 +12,7 @@ #include "process-util.h" #include "socket-util.h" #include "string-util.h" +#include "terminal-util.h" #include "vmspawn-register.h" #include "varlink-util.h" @@ -73,7 +74,7 @@ int register_machine( SD_JSON_BUILD_PAIR_CONDITION(!!directory, "rootDirectory", SD_JSON_BUILD_STRING(directory)), SD_JSON_BUILD_PAIR_CONDITION(!!address, "sshAddress", SD_JSON_BUILD_STRING(address)), SD_JSON_BUILD_PAIR_CONDITION(!!key_path, "sshPrivateKeyPath", SD_JSON_BUILD_STRING(key_path)), - SD_JSON_BUILD_PAIR_CONDITION(isatty(STDIN_FILENO), "allowInteractiveAuthentication", SD_JSON_BUILD_BOOLEAN(true)), + SD_JSON_BUILD_PAIR_CONDITION(isatty_safe(STDIN_FILENO), "allowInteractiveAuthentication", SD_JSON_BUILD_BOOLEAN(true)), SD_JSON_BUILD_PAIR_CONDITION(!keep_unit, "allocateUnit", SD_JSON_BUILD_BOOLEAN(true))); }