From: Franck Bui <fbui@suse.com>
Date: Thu, 14 Dec 2023 11:07:46 +0000 (+0100)
Subject: vconsole-setup: handle the case where the vc is in KD_GRAPHICS mode more gracefully
X-Git-Tag: v256-rc1~1460^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=58161db61ba933f1e3e7395d1f50fe0213bb82f2;p=thirdparty%2Fsystemd.git

vconsole-setup: handle the case where the vc is in KD_GRAPHICS mode more gracefully

Regardless of whether a vc path is passed, the behavior of
systemd-vconsole-setup wasn't ideal when either the passed vc or /dev/tty1 was
in graphics mode.

When a vc in graphics mode was passed, no message was emitted despite the fact
that the font settings couldn't be applied. The previous code might have
assumed that setfont(8) would throw a warning but that's not case.

When no argument was passed, systemd-vconsole-setup was supposed to
automatically select a valid tty, init it and copy the font setting to the
remaining ttys. However if the selected virtual console was in KD_GRAPHICS mode
the initialization of the font failed not only for the selected source vc but
for all of them.
---

diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c
index 4d82c65f0af..4c28f43808b 100644
--- a/src/vconsole/vconsole-setup.c
+++ b/src/vconsole/vconsole-setup.c
@@ -217,6 +217,8 @@ static int verify_vc_allocation_byfd(int fd) {
 static int verify_vc_kbmode(int fd) {
         int curr_mode;
 
+        assert(fd >= 0);
+
         /*
          * Make sure we only adjust consoles in K_XLATE or K_UNICODE mode.
          * Otherwise we would (likely) interfere with X11's processing of the
@@ -231,6 +233,20 @@ static int verify_vc_kbmode(int fd) {
         return IN_SET(curr_mode, K_XLATE, K_UNICODE) ? 0 : -EBUSY;
 }
 
+static int verify_vc_display_mode(int fd) {
+        int mode;
+
+        assert(fd >= 0);
+
+        /* Similarly the vc is likely busy if it is in KD_GRAPHICS mode. If it's not the case and it's been
+         * left in graphics mode, the kernel will refuse to operate on the font settings anyway. */
+
+        if (ioctl(fd, KDGETMODE, &mode) < 0)
+                return -errno;
+
+        return mode != KD_TEXT ? -EBUSY : 0;
+}
+
 static int toggle_utf8_vc(const char *name, int fd, bool utf8) {
         int r;
         struct termios tc = {};
@@ -470,24 +486,14 @@ static void setup_remaining_vcs(int src_fd, unsigned src_idx, bool utf8) {
                 if (cfo.op != KD_FONT_OP_SET)
                         continue;
 
-                r = ioctl(fd_d, KDFONTOP, &cfo);
+                r = verify_vc_display_mode(fd_d);
                 if (r < 0) {
-                        int last_errno, mode;
-
-                        /* The fonts couldn't have been copied. It might be due to the
-                         * terminal being in graphical mode. In this case the kernel
-                         * returns -EINVAL which is too generic for distinguishing this
-                         * specific case. So we need to retrieve the terminal mode and if
-                         * the graphical mode is in used, let's assume that something else
-                         * is using the terminal and the failure was expected as we
-                         * shouldn't have tried to copy the fonts. */
-
-                        last_errno = errno;
-                        if (ioctl(fd_d, KDGETMODE, &mode) >= 0 && mode != KD_TEXT)
-                                log_debug("KD_FONT_OP_SET skipped: tty%u is not in text mode", i);
-                        else
-                                log_warning_errno(last_errno, "KD_FONT_OP_SET failed, fonts will not be copied to tty%u: %m", i);
+                        log_debug_errno(r, "KD_FONT_OP_SET skipped: tty%u is not in text mode", i);
+                        continue;
+                }
 
+                if (ioctl(fd_d, KDFONTOP, &cfo) < 0) {
+                        log_warning_errno(errno, "KD_FONT_OP_SET failed, fonts will not be copied to tty%u: %m", i);
                         continue;
                 }
 
@@ -535,6 +541,7 @@ static int find_source_vc(char **ret_path, unsigned *ret_idx) {
                         RET_GATHER(err, r);
                         continue;
                 }
+
                 r = verify_vc_kbmode(fd);
                 if (r < 0) {
                         log_debug_errno(r, "Failed to check VC %s keyboard mode: %m", path);
@@ -542,6 +549,13 @@ static int find_source_vc(char **ret_path, unsigned *ret_idx) {
                         continue;
                 }
 
+                r = verify_vc_display_mode(fd);
+                if (r < 0) {
+                        log_debug_errno(r, "Failed to check VC %s display mode: %m", path);
+                        RET_GATHER(err, r);
+                        continue;
+                }
+
                 /* all checks passed, return this one as a source console */
                 *ret_idx = i;
                 *ret_path = TAKE_PTR(path);
@@ -572,6 +586,13 @@ static int verify_source_vc(char **ret_path, const char *src_vc) {
         if (r < 0)
                 return log_error_errno(r, "Virtual console %s is not in K_XLATE or K_UNICODE: %m", src_vc);
 
+        /* setfont(8) silently ignores when the font can't be applied due to the vc being in
+         * KD_GRAPHICS. Hence we continue to accept this case however we now let the user know that the vc
+         * will be initialized only partially.*/
+        r = verify_vc_display_mode(fd);
+        if (r < 0)
+                log_notice_errno(r, "Virtual console %s is not in KD_TEXT, font settings likely won't be applied.", src_vc);
+
         path = strdup(src_vc);
         if (!path)
                 return log_oom();