[PATCH 1/4] Avoid that agetty nor sulogin are fooled by a running plymouth

[PATCH 1/4] Avoid that agetty nor sulogin are fooled by a running plymouth

[Werner Fink]

The nowadays used plymouth locks the devices used for the system
console which causes that agetty as well as sulogin can not modify
the termios settings of e.g. the serial devices of the systenm console.

Signed-off-by: Werner Fink <werner@suse.de>
---
 login-utils/sulogin.c |   51 ++++++++++++++++++++++++++++++++-
 term-utils/agetty.c   |   75 ++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 118 insertions(+), 8 deletions(-)

diff --git login-utils/sulogin.c login-utils/sulogin.c
index 37d006d..1e227a8 100644
--- login-utils/sulogin.c
+++ login-utils/sulogin.c
@@ -77,13 +77,62 @@ static volatile sig_atomic_t sigchild;
 #endif
 
 /*
+ * For the case plymouth is found on this system
+ */
+static int plymouth_command(const char* arg)
+{
+	const char *cmd = "/usr/bin/plymouth";
+	static int has_plymouth = 1;
+	pid_t pid;
+
+	if (!has_plymouth)
+		return 127;
+
+	pid = fork();
+	if (!pid) {
+		int fd = open("/dev/null", O_RDWR);
+		dup2(fd, 0);
+		dup2(fd, 1);
+		dup2(fd, 2);
+		close(fd);
+		execl(cmd, cmd, arg, (char *) NULL);
+		exit(127);
+	} else if (pid > 0) {
+		int status;
+		waitpid(pid, &status, 0);
+		if (status == 127)
+			has_plymouth = 0;
+		return status;
+	}
+	return 1;
+}
+
+/*
  * Fix the tty modes and set reasonable defaults.
  */
 static void tcinit(struct console *con)
 {
 	int mode = 0, flags = 0;
 	struct termios *tio = &con->tio;
-	int fd = con->fd;
+	struct termios lock;
+	int fd = con->fd, i = (plymouth_command("--ping")) ? 20 : 0;
+
+	while (i-- > 0) {
+		/*
+		 * With plymouth the termios flags become changed after this
+		 * function had changed the termios.
+		 */
+		memset(&lock, 0, sizeof(struct termios));
+		if (ioctl(fd, TIOCGLCKTRMIOS, &lock) < 0)
+			break;
+		if (!lock.c_iflag && !lock.c_oflag && !lock.c_cflag && !lock.c_lflag)
+			break;
+		if (i == 15 && plymouth_command("quit") != 0)
+			break;
+		sleep(1);
+	}
+	memset(&lock, 0, sizeof(struct termios));
+	ioctl(fd, TIOCSLCKTRMIOS, &lock);
 
 	errno = 0;
 
diff --git term-utils/agetty.c term-utils/agetty.c
index 7c0579c..fe671a9 100644
--- term-utils/agetty.c
+++ term-utils/agetty.c
@@ -20,6 +20,7 @@
 #include <sys/ioctl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
 #include <fcntl.h>
 #include <stdarg.h>
 #include <ctype.h>
@@ -276,13 +277,15 @@ static void log_warn (const char *, ...)
 static ssize_t append(char *dest, size_t len, const char  *sep, const char *src);
 static void check_username (const char* nm);
 static void login_options_to_argv(char *argv[], int *argc, char *str, char *username);
+static int plymouth_command(const char* arg);
 
 /* Fake hostname for ut_host specified on command line. */
 static char *fakehost;
 
 #ifdef DEBUGGING
+# include "closestream.h"
 # ifndef DEBUG_OUTPUT
-#  define DEBUG_OUTPUT "/dev/ttyp0"
+#  define DEBUG_OUTPUT "/dev/tty10"
 # endif
 # define debug(s) do { fprintf(dbf,s); fflush(dbf); } while (0)
 FILE *dbf;
@@ -320,8 +323,12 @@ int main(int argc, char **argv)
 
 #ifdef DEBUGGING
 	dbf = fopen(DEBUG_OUTPUT, "w");
-	for (int i = 1; i < argc; i++)
+	for (int i = 1; i < argc; i++) {
+		if (i > 1)
+			debug(" ");
 		debug(argv[i]);
+	}
+	debug("\n");
 #endif				/* DEBUGGING */
 
 	/* Parse command-line arguments. */
@@ -472,7 +479,6 @@ int main(int argc, char **argv)
 	}
 	free(options.osrelease);
 #ifdef DEBUGGING
-	fprintf(dbf, "read %c\n", ch);
 	if (close_stream(dbf) != 0)
 		log_err("write failed: %s", DEBUG_OUTPUT);
 #endif
@@ -1089,6 +1095,27 @@ static void termio_init(struct options *op, struct termios *tp)
 {
 	speed_t ispeed, ospeed;
 	struct winsize ws;
+	struct termios lock;
+	int i =  (plymouth_command("--ping") == 0) ? 30 : 0;
+
+	while (i-- > 0) {
+		/*
+		 * Even with TTYReset=no it seems with systemd or plymouth
+		 * the termios flags become changed from under the first
+		 * agetty on a serial system console as the flags are locked.
+		 */
+		memset(&lock, 0, sizeof(struct termios));
+		if (ioctl(STDIN_FILENO, TIOCGLCKTRMIOS, &lock) < 0)
+			break;
+		if (!lock.c_iflag && !lock.c_oflag && !lock.c_cflag && !lock.c_lflag)
+			break;
+		debug("termios locked\n");
+		if (i == 15 && plymouth_command("quit") != 0)
+			break;
+		sleep(1);
+	}
+	memset(&lock, 0, sizeof(struct termios));
+	ioctl(STDIN_FILENO, TIOCSLCKTRMIOS, &lock);
 
 	if (op->flags & F_VCONSOLE) {
 #if defined(IUTF8) && defined(KDGKBMODE)
@@ -1153,9 +1180,6 @@ static void termio_init(struct options *op, struct termios *tp)
 	 * later on.
 	 */
 
-	 /* Flush input and output queues, important for modems! */
-	tcflush(STDIN_FILENO, TCIOFLUSH);
-
 #ifdef IUTF8
 	tp->c_iflag = tp->c_iflag & IUTF8;
 	if (tp->c_iflag & IUTF8)
@@ -1215,8 +1239,11 @@ static void termio_init(struct options *op, struct termios *tp)
 	if (op->flags & F_RTSCTS)
 		tp->c_cflag |= CRTSCTS;
 #endif
+	 /* Flush input and output queues, important for modems! */
+	tcflush(STDIN_FILENO, TCIOFLUSH);
 
-	tcsetattr(STDIN_FILENO, TCSANOW, tp);
+	if (tcsetattr(STDIN_FILENO, TCSANOW, tp))
+		log_warn(_("setting terminal attributes failed: %m"));
 
 	/* Go to blocking input even in local mode. */
 	fcntl(STDIN_FILENO, F_SETFL,
@@ -1237,6 +1264,10 @@ static void reset_vc(const struct options *op, struct termios *tp)
 
 	if (tcsetattr(STDIN_FILENO, TCSADRAIN, tp))
 		log_warn(_("setting terminal attributes failed: %m"));
+
+	/* Go to blocking input even in local mode. */
+	fcntl(STDIN_FILENO, F_SETFL,
+	      fcntl(STDIN_FILENO, F_GETFL, 0) & ~O_NONBLOCK);
 }
 
 /* Extract baud rate from modem status message. */
@@ -2257,3 +2288,33 @@ err:
 	log_err(_("checkname failed: %m"));
 }
 
+/*
+ * For the case plymouth is found on this system
+ */
+static int plymouth_command(const char* arg)
+{
+	const char *cmd = "/usr/bin/plymouth";
+	static int has_plymouth = 1;
+	pid_t pid;
+
+	if (!has_plymouth)
+		return 127;
+
+	pid = fork();
+	if (!pid) {
+		int fd = open("/dev/null", O_RDWR);
+		dup2(fd, 0);
+		dup2(fd, 1);
+		dup2(fd, 2);
+		close(fd);
+		execl(cmd, cmd, arg, (char *) NULL);
+		exit(127);
+	} else if (pid > 0) {
+		int status;
+		waitpid(pid, &status, 0);
+		if (status == 127)
+			has_plymouth = 0;
+		return status;
+	}
+	return 1;
+}
-- 
1.7.9.2

[PATCH 2/4] Enable sulogin to find a suitable console device even if the first line in /proc/consoles does not have any major and minor number.

[Werner Fink]

Enable sulogin to find a suitable console device even if the first line
in /proc/consoles does not have any major and minor number.

Signed-off-by: Werner Fink <werner@suse.de>
---
 login-utils/sulogin-consoles.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git login-utils/sulogin-consoles.c login-utils/sulogin-consoles.c
index 6ceea6b..817a882 100644
--- login-utils/sulogin-consoles.c
+++ login-utils/sulogin-consoles.c
@@ -324,7 +324,7 @@ static int detect_consoles_from_proc(struct list_head *consoles)
 	char fbuf[16 + 1];
 	DIR *dir = NULL;
 	FILE *fc = NULL;
-	int maj, min, rc = 1;
+	int maj, min, rc = 1, matches;
 
 	DBG(dbgprint("trying /proc"));
 
@@ -337,10 +337,12 @@ static int detect_consoles_from_proc(struct list_head *consoles)
 	if (!dir)
 		goto done;
 
-	while (fscanf(fc, "%*s %*s (%16[^)]) %d:%d", fbuf, &maj, &min) == 3) {
+	while ((matches = fscanf(fc, "%*s %*s (%16[^)]) %d:%d", fbuf, &maj, &min)) >= 1) {
 		char *name;
 		dev_t comparedev;
 
+		if (matches != 3)
+			continue;
 		if (!strchr(fbuf, 'E'))
 			continue;
 		comparedev = makedev(maj, min);
-- 
1.7.9.2

[PATCH 3/4] Better support of the special system consoles on S390

[Werner Fink]

That is that there are several consoles, the /dev/ttyS0 which
is type of ibm3215 and a dumb terminal, then there is the device
/dev/3270/tty1 which can handle ANSI color escape sequences and is
a ibm327x terminal, and the /dev/ttyS1 which is a vt220 terminal.

The macro is_speed() in agetty.c allows to distinguish between the
terminal line (/dev)3270/tty1 and the speed options on the command
line used in

/run/systemd/generator/getty.target.wants/serial-getty@3270-tty1.service

which is a symbolic link to /usr/lib/systemd/system/serial-getty@.service

Signed-off-by: Werner Fink <werner@suse.de>
---
 login-utils/sulogin.c |    8 +++++++-
 term-utils/agetty.c   |   15 ++++++++++-----
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git login-utils/sulogin.c login-utils/sulogin.c
index 1e227a8..147e633 100644
--- login-utils/sulogin.c
+++ login-utils/sulogin.c
@@ -231,10 +231,16 @@ static void tcfinal(struct console *con)
 		setenv("TERM", "linux", 1);
 		return;
 	}
-	if (con->flags & CON_NOTTY)
+	if (con->flags & CON_NOTTY) {
+		setenv("TERM", "dumb", 1);
 		return;
+	}
 
+#if defined (__s390__) || defined (__s390x__)
+	setenv("TERM", "dumb", 1);
+#else
 	setenv("TERM", "vt102", 1);
+#endif
 	tio = &con->tio;
 	fd = con->fd;
 
diff --git term-utils/agetty.c term-utils/agetty.c
index fe671a9..24c9366 100644
--- term-utils/agetty.c
+++ term-utils/agetty.c
@@ -51,7 +51,8 @@
 #    define DEFAULT_VCTERM "linux"
 #  endif
 #  if defined (__s390__) || defined (__s390x__)
-#    define DEFAULT_TTYS0  "ibm327x"
+#    define DEFAULT_TTYS0  "dumb"
+#    define DEFAULT_TTY32  "ibm327x"
 #    define DEFAULT_TTYS1  "vt220"
 #  endif
 #  ifndef DEFAULT_STERM
@@ -569,6 +570,8 @@ static void login_options_to_argv(char *argv[], int *argc,
 	*argc = i;
 }
 
+#define is_speed(str) (strlen((str)) == strspn((str), "0123456789,"))
+
 /* Parse command-line arguments. */
 static void parse_args(int argc, char **argv, struct options *op)
 {
@@ -747,7 +750,7 @@ static void parse_args(int argc, char **argv, struct options *op)
 	}
 
 	/* Accept "tty", "baudrate tty", and "tty baudrate". */
-	if ('0' <= argv[optind][0] && argv[optind][0] <= '9') {
+	if (is_speed(argv[optind])) {
 		/* Assume BSD style speed. */
 		parse_speeds(op, argv[optind++]);
 		if (argc < optind + 1) {
@@ -759,7 +762,7 @@ static void parse_args(int argc, char **argv, struct options *op)
 		op->tty = argv[optind++];
 		if (argc > optind) {
 			char *v = argv[optind++];
-			if ('0' <= *v && *v <= '9')
+			if (is_speed(v))
 				parse_speeds(op, v);
 			else
 				op->speeds[op->numspeed++] = bcode("9600");
@@ -1069,9 +1072,11 @@ static void open_tty(char *tty, struct termios *tp, struct options *op)
 		 * higher.  Whereas the second serial line on a S/390(x) is
 		 * a real character terminal which is compatible with VT220.
 		 */
-		if (strcmp(op->tty, "ttyS0") == 0)
+		if (strcmp(op->tty, "ttyS0") == 0)		/* linux/drivers/s390/char/con3215.c */
 			op->term = DEFAULT_TTYS0;
-		else if (strcmp(op->tty, "ttyS1") == 0)
+		else if (strncmp(op->tty, "3270/tty", 8) == 0)	/* linux/drivers/s390/char/con3270.c */
+			op->term = DEFAULT_TTY32;
+		else if (strcmp(op->tty, "ttyS1") == 0)		/* linux/drivers/s390/char/sclp_vt220.c */
 			op->term = DEFAULT_TTYS1;
 	}
 #endif
-- 
1.7.9.2

[PATCH 4/4] Beside virtual consoles support xvc and hvc device lines as well

[Werner Fink]

For this approach do not use the ioctl TIOCMGET anymore as this
is for real serial lines only. But switch over to use the ioctl
KDGKBMODE as this is unique to the virtual console lines only.

Signed-off-by: Werner Fink <werner@suse.de>
---
 login-utils/sulogin.c |   17 ++++++++++++-----
 term-utils/agetty.c   |   23 +++++++++++------------
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git login-utils/sulogin.c login-utils/sulogin.c
index 147e633..222d40e 100644
--- login-utils/sulogin.c
+++ login-utils/sulogin.c
@@ -49,6 +49,11 @@
 # include <selinux/get_context_list.h>
 #endif
 
+#ifdef __linux__
+# include <sys/kd.h>
+# include <sys/param.h>
+#endif
+
 #include "c.h"
 #include "closestream.h"
 #include "nls.h"
@@ -142,10 +147,14 @@ static void tcinit(struct console *con)
 		return;
 	}
 
-	/* Handle serial lines here */
-	if (ioctl(fd, TIOCMGET, (char *) &mode) == 0) {
+	/* Handle lines other than virtual consoles here */
+#if defined(KDGKBMODE)
+	if (ioctl(fd, KDGKBMODE, &mode) < 0)
+#endif
+	{
 		speed_t ispeed, ospeed;
 		struct winsize ws;
+		errno = 0;
 
 		/* this is a modem line */
 		con->flags |= CON_SERIAL;
@@ -191,9 +200,7 @@ static void tcinit(struct console *con)
 		goto setattr;
 	}
 #if defined(IUTF8) && defined(KDGKBMODE)
-	/* Detect mode of current keyboard setup, e.g. for UTF-8 */
-	if (ioctl(fd, KDGKBMODE, &mode) < 0)
-		mode = K_RAW;
+	/* Handle mode of current keyboard setup, e.g. for UTF-8 */
 	switch(mode) {
 	case K_UNICODE:
 		setlocale(LC_CTYPE, "C.UTF-8");
diff --git term-utils/agetty.c term-utils/agetty.c
index 24c9366..a706836 100644
--- term-utils/agetty.c
+++ term-utils/agetty.c
@@ -140,6 +140,7 @@ struct options {
 	int nice;			/* Run login with this priority */
 	int numspeed;			/* number of baud rates to try */
 	int clocal;			/* CLOCAL_MODE_* */
+	int kbmode;			/* Keyboard mode if virtual console */
 	speed_t speeds[MAX_SPEED];	/* baud rates to be tried */
 };
 
@@ -936,7 +937,7 @@ static void update_utmp(struct options *op)
 static void open_tty(char *tty, struct termios *tp, struct options *op)
 {
 	const pid_t pid = getpid();
-	int serial, closed = 0;
+	int closed = 0;
 
 	/* Set up new standard input, unless we are given an already opened port. */
 
@@ -1082,15 +1083,18 @@ static void open_tty(char *tty, struct termios *tp, struct options *op)
 #endif
 	/*
 	 * Detect if this is a virtual console or serial/modem line.
-	 * In case of a virtual console the ioctl TIOCMGET fails and
-	 * the error number will be set to EINVAL.
+	 * In case of a virtual console the ioctl KDGKBMODE succeeds
+	 * whereas on other lines it will fails.
 	 */
-	if (ioctl(STDIN_FILENO, TIOCMGET, &serial) < 0 && (errno == EINVAL)) {
+	if (ioctl(STDIN_FILENO, KDGKBMODE, &op->kbmode) == 0) {
 		op->flags |= F_VCONSOLE;
 		if (!op->term)
 			op->term = DEFAULT_VCTERM;
-	} else if (!op->term)
-		op->term = DEFAULT_STERM;
+	} else {
+		op->kbmode = K_RAW;
+		if (!op->term)
+			op->term = DEFAULT_STERM;
+	}
 
 	setenv("TERM", op->term, 1);
 }
@@ -1124,12 +1128,7 @@ static void termio_init(struct options *op, struct termios *tp)
 
 	if (op->flags & F_VCONSOLE) {
 #if defined(IUTF8) && defined(KDGKBMODE)
-		int mode;
-
-		/* Detect mode of current keyboard setup, e.g. for UTF-8 */
-		if (ioctl(STDIN_FILENO, KDGKBMODE, &mode) < 0)
-			mode = K_RAW;
-		switch(mode) {
+		switch(op->kbmode) {
 		case K_UNICODE:
 			setlocale(LC_CTYPE, "C.UTF-8");
 			op->flags |= F_UTF8;
-- 
1.7.9.2

Re: [PATCH 1/4] Avoid that agetty nor sulogin are fooled by a running plymouth

[Karel Zak]

On Thu, May 08, 2014 at 12:09:22PM +0200, Werner Fink wrote:
> The nowadays used plymouth locks the devices used for the system
> console which causes that agetty as well as sulogin can not modify
> the termios settings of e.g. the serial devices of the systenm console.

 All applied, thanks!

> +	int fd = con->fd, i = (plymouth_command("--ping")) ? 20 : 0;
> +
> +	while (i-- > 0) {
> +		/*
> +		 * With plymouth the termios flags become changed after this
> +		 * function had changed the termios.
> +		 */
> +		memset(&lock, 0, sizeof(struct termios));
> +		if (ioctl(fd, TIOCGLCKTRMIOS, &lock) < 0)
> +			break;
> +		if (!lock.c_iflag && !lock.c_oflag && !lock.c_cflag && !lock.c_lflag)
> +			break;
> +		if (i == 15 && plymouth_command("quit") != 0)
> +			break;
> +		sleep(1);
> +	}
> +	memset(&lock, 0, sizeof(struct termios));
> +	ioctl(fd, TIOCSLCKTRMIOS, &lock);

 I hope 20s sleep is the worst case and in normal situations we don't
 have to call plymouth "quit" from sulogin/agetty.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 1/4] Avoid that agetty nor sulogin are fooled by a running plymouth

[Dr. Werner Fink]

[-- Attachment #1: Type: text/plain, Size: 1871 bytes --]

On Tue, May 13, 2014 at 10:56:58AM +0200, Karel Zak wrote:
> On Thu, May 08, 2014 at 12:09:22PM +0200, Werner Fink wrote:
> > The nowadays used plymouth locks the devices used for the system
> > console which causes that agetty as well as sulogin can not modify
> > the termios settings of e.g. the serial devices of the systenm console.
> 
>  All applied, thanks!
> 
> > +	int fd = con->fd, i = (plymouth_command("--ping")) ? 20 : 0;
> > +
> > +	while (i-- > 0) {
> > +		/*
> > +		 * With plymouth the termios flags become changed after this
> > +		 * function had changed the termios.
> > +		 */
> > +		memset(&lock, 0, sizeof(struct termios));
> > +		if (ioctl(fd, TIOCGLCKTRMIOS, &lock) < 0)
> > +			break;
> > +		if (!lock.c_iflag && !lock.c_oflag && !lock.c_cflag && !lock.c_lflag)
> > +			break;
> > +		if (i == 15 && plymouth_command("quit") != 0)
> > +			break;
> > +		sleep(1);
> > +	}
> > +	memset(&lock, 0, sizeof(struct termios));
> > +	ioctl(fd, TIOCSLCKTRMIOS, &lock);
> 
>  I hope 20s sleep is the worst case and in normal situations we don't
>  have to call plymouth "quit" from sulogin/agetty.

Indeed the 20 seconds are the worst case, in normal case this is not hit but
during debugging a few service units I had run into a blocking plymouth, that
is that plymouth hold the terminal termios structure locked and this has
lead to that fact that no password could be specified.

In my experience plymouth has some more further side effects if on is using
a further serial console or if there is no virtual console (s390 and some
x86_64 server systems).  Maybe upstream of plymouth is not aware that in
server business a virtual console used for graphic is not that common.

Werner

-- 
  "Having a smoking section in a restaurant is like having
          a peeing section in a swimming pool." -- Edward Burr

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/4] Avoid that agetty nor sulogin are fooled by a running plymouth

[Ruediger Meier]

On Tuesday 13 May 2014, Dr. Werner Fink wrote:
> In my experience plymouth has some more further side effects if on is
> using a further serial console or if there is no virtual console
> (s390 and some x86_64 server systems).  Maybe upstream of plymouth is
> not aware that in server business a virtual console used for graphic
> is not that common.

I am more wondering why we are using plymouth in server business then. 
But that's an openSUSE issue ;)

cu,
Rudi