[Pkg-shadow-devel] Bug#663200: Bug#663200: Bug#628843: Bug#659878: cannot set terminal process group (-1): Inappropriate ioctl for device

Wolfgang Zarre lkdev at essax.com
Sun Sep 15 07:47:13 UTC 2013


Hi,

> There's been a switch to git in the mean time.

I prefer git.

> You can find the repo on github:
> https://github.com/shadow-maint/shadow
> 

I cloned and I tried to merge as good as possible and therefore
I did also some rework.

Now Ctrl-Z is working as well as expected.


Beside a working version now there might be still
some improvements necessary, corrections or fixes.


Maybe some thoughts and notes to the merge:

@@ -360,14 +402,6 @@:
    I removed the terminal setting of the child due the fact
    that actually the system settings should be set like the
    same as at a normal login including the personal settings
    of a user.


@@ -423,7 +457,7 @@:
    I case of failures it is IMHO better to set SIGHUP instead
    SIGTERM because according to the code the child might be
    SIGKILLed by kill_child() if the child is a shell which
    ignores SIGTERM.


@@ -434,31 +468,39 @@:
    Due the fact that we are not always retrieving a signal to
    be able to switch correct between the parents tty's raw and cooked
    mode I found the way just in using the process group id to differ
    between background and foreground operation but independent
    if the child process is stopped or running.

    Important is that the parents tty stays sane except the
    su session gets SIGKILLed.



@@ -491,76 +549,146 @@:

    I was letting the tty reset outside the loop because we would loose
    the reset if there would be an interrupt between setting to raw mode
    and running the main loop.



Sorry for the mess with spaces and tab's however, due the
fact that the source file was mixed already it would be
good to do a re-base either to spaces or tabs.





Based on:
    branch: su-c_tty
    commit ad1ecc897b4168f36ef0cb048ebea101015521c8

___BEGIN_PATCH___
diff --git a/src/su.c b/src/su.c
index 34f6771..63f239e 100644
--- a/src/su.c
+++ b/src/su.c
@@ -60,7 +60,6 @@
 #include <pwd.h>
 #include <signal.h>
 #include <stdio.h>
-#include <sys/time.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <sys/ioctl.h>
@@ -220,6 +219,46 @@ static /*@noreturn@*/void su_failure (const char *tty, bool su_to_root)
 	exit (1);
 }

+static bool term_setattr( int fd, const struct termios *termset, bool hndl_sig) {
+
+    struct termios termset_new;
+    struct termios termset_check;
+
+	termset_new = *termset;
+	/* Set RAW mode  */
+	cfmakeraw( &termset_new);
+
+	if( hndl_sig)
+	    termset_new.c_lflag = ISIG;
+
+    if( tcsetattr( fd, TCSANOW, &termset_new) == -1) {
+        fprintf( stderr,
+                 _("%s: Cannot set raw mode\n"),
+                 Prog);
+        return false;
+    }
+
+    if( tcgetattr( fd, &termset_check) == -1) {
+        fprintf( stderr,
+                 _("%s: Cannot get terminal attributes\n"),
+                 Prog);
+        return false;
+    }
+
+    if( termset_new.c_iflag != termset_check.c_iflag ||
+        termset_new.c_oflag != termset_check.c_oflag ||
+        termset_new.c_cflag != termset_check.c_cflag ||
+        termset_new.c_lflag != termset_check.c_lflag ||
+        memcmp( &termset_new.c_cc, &termset_check.c_cc, NCCS) != 0) {
+
+        fprintf( stderr,
+                 _("%s: Could not set terminal attributes correctly\n"),
+                 Prog);
+        return false;
+    }
+    return true;
+}
+
 /*
  * execve_shell - Execute a shell with execve, or interpret it with
  * /bin/sh
@@ -280,19 +319,22 @@ static void handle_session (const struct passwd *pw)
 #endif				/* USE_PAM */
 	int fd_ptmx = -1;
 	int fd_pts = -1;
-	char *pts_name = NULL;	
+	char *pts_name = NULL;
 	struct termios termset_save;
-	struct termios termset_new;
 	fd_set inp_fds;
 	struct timeval sel_to;
 	char trbuf[BUFSIZ];
 	ssize_t bytes_r;
 	struct winsize winsz;
 	bool winsz_set = false;
+	pid_t pg_pid = 0;
+	pid_t pg_pid_cmp = 0;
+	pid_t pg_pid_tmp = 0;


+	pg_pid = getpid();

-	if (isatty (0) == 1) {
+	if (isatty ( STDIN_FILENO) == 1) {
 		have_tty = true;

 		if (tcgetattr (STDIN_FILENO, &termset_save) == -1) {
@@ -360,14 +402,6 @@ static void handle_session (const struct passwd *pw)
 		if (have_tty) {
 			close (fd_ptmx);

-			if (tcsetattr (fd_pts, TCSANOW, &termset_save) == -1) {
-				fprintf (stderr,
-				         _("%s: Cannot set termios attributes of session\n"),
-				         Prog);
-				(void) close (fd_pts);
-				exit (1);
-			}
-
 			if (   winsz_set
 			    && (ioctl (fd_pts, TIOCSWINSZ, &winsz) == -1)) {
 				fprintf (stderr,
@@ -423,7 +457,7 @@ static void handle_session (const struct passwd *pw)
 		(void) fprintf (stderr,
 		                _("%s: signal malfunction\n"),
 		                Prog);
-		caught = SIGTERM;
+		caught = SIGHUP;
 	}
 	if (0 == caught) {
 		struct sigaction action;
@@ -434,31 +468,39 @@ static void handle_session (const struct passwd *pw)
 		sigemptyset (&ourset);

 		if (   (sigaddset (&ourset, SIGTERM) != 0)
+		    || (sigaddset (&ourset, SIGINT) != 0)
 		    || (sigaddset (&ourset, SIGALRM) != 0)
 		    || (sigaddset (&ourset, SIGWINCH) != 0)
+		    || (sigaddset (&ourset, SIGCONT) != 0)
+		    || (sigaddset (&ourset, SIGTSTP) != 0)
 		    || (sigaction (SIGTERM, &action, NULL) != 0)
+		    || (sigaction (SIGINT, &action, NULL) != 0)
 		    || (sigaction (SIGWINCH, &action, NULL) != 0)
-		    || (sigprocmask (SIG_UNBLOCK, &ourset, NULL) != 0)) {
+		    || (sigaction (SIGCONT, &action, NULL) != 0)
+		    || (sigaction (SIGTSTP, &action, NULL) != 0)
+		    || (sigprocmask (SIG_UNBLOCK, &ourset, NULL) != 0)
+		    ) {
 			fprintf (stderr,
 			         _("%s: signal masking malfunction\n"),
 			         Prog);
-			caught = SIGTERM;
+			caught = SIGHUP;
 		}
 	}

 	if ((0 == caught) && have_tty) {
-		/* Set RAW mode  */
-		termset_new = termset_save;
-		cfmakeraw (&termset_new);
-		if (tcsetattr (STDIN_FILENO, TCSANOW, &termset_new) != 0) {
-			/* FIXME: At least one change was successful.
-			 * Success should be checked with tcsetattr */
-			fprintf (stderr,
-			         _("%s: Cannot set terminal attributes: %s\n"),
-			         Prog, strerror (errno));
-			caught = -1;
-		}
-	}
+		if( (pg_pid_tmp = tcgetpgrp( STDIN_FILENO)) == -1) {
+			fprintf( stderr, _("%s: Cannot get process group id\n"), Prog);
+			caught = SIGHUP;
+		} else {
+			/* Set raw mode if running in foreground */
+			if( pg_pid_tmp == pg_pid) {
+				/* Set RAW mode  */
+                if( term_setattr( STDIN_FILENO, &termset_save, !doshell) == false)
+                    caught = SIGHUP;
+            }
+            pg_pid_cmp = pg_pid_tmp;
+        }
+    }

 	if (0 == caught) {
 		bool stop = true;
@@ -466,6 +508,7 @@ static void handle_session (const struct passwd *pw)
 		do {
 			pid_t pid;
 			stop = true;
+			errno = 0;

 			if (have_tty) {
 				pid = waitpid (-1, &status, WUNTRACED | WNOHANG);
@@ -473,16 +516,31 @@ static void handle_session (const struct passwd *pw)
 				pid = waitpid (-1, &status, WUNTRACED);
 			}

-			if (   ((pid_t)-1 != pid && !have_tty)
+			/* When interrupted by signal, the signal will be
+			 * forwarded to the child, and termination will be
+			 * forced later.
+			 */
+			if (   (((pid_t)-1 == pid) && (EINTR == errno) && (SIGTSTP == caught)) ||
+			        ((pid == (pid_t)0) && (SIGTSTP == caught))) {
+				/* Except for SIGTSTP, which request to
+				 * stop the child.
+				 * We will SIGSTOP ourself on the next
+				 * waitpid round.
+				 */
+				kill (pid_child, SIGSTOP);
+				stop = false;
+			} else if ( ((pid_t)0 < pid)
 			           && (0 != WIFSTOPPED (status))) {
 				/* The child (shell) was suspended.
 				 * Suspend su. */
-				kill (getpid (), WSTOPSIG (status));
+				kill (pg_pid, WSTOPSIG (status));
 				/* wake child when resumed */
 				kill (pid, SIGCONT);
 				stop = false;
-			} else if (pid == (pid_t)0 && have_tty) {
-				stop = false;
+			} else if ( pid >= (pid_t)0 && have_tty) {
+
+				if( pid == (pid_t)0)
+					stop = false;

 				if (caught == SIGWINCH) {
 					caught = 0;
@@ -491,76 +549,146 @@ static void handle_session (const struct passwd *pw)
 					}
 				}

+				/* Reset 'process group pid compare' to set STDIN to 'raw' again */
+				if( caught == SIGCONT) {
+					caught = 0;
+					pg_pid_cmp = 0;
+				}
+
+                /* Terminate the child with SIGHUP to be able
+                 * to terminate a shell running as command
+                 */
+				if( caught == SIGINT)
+					kill( -pid_child, SIGHUP);
+
+				/* If caught by any other signal then stop */
+				if( caught != 0)
+					stop = true;
+
 				FD_ZERO (&inp_fds);
 				FD_SET (STDIN_FILENO, &inp_fds);
 				FD_SET (fd_ptmx, &inp_fds);
 				sel_to = (struct timeval){ 0, 10000};

 				if (select (fd_ptmx + 1, &inp_fds, NULL, NULL, &sel_to) == -1) {
-					if (errno == EINTR) {
+					if (errno == EINTR && stop == false) {
 						continue;
 					}
 					stop = true;
+					errno = 0;
 				}
 				if (FD_ISSET (STDIN_FILENO, &inp_fds)) {
-					bytes_r = read (STDIN_FILENO, trbuf, BUFSIZ);
-					if (bytes_r <= 0) {
-						if (errno == EINTR) {
-							continue;
-						}
-						fprintf (stderr,
-						         _("%s: Failure in reading from stdin\r\n"),
-						         Prog);
+					/* Get process group pid to compare with last run */
+					if ( (pg_pid_tmp = tcgetpgrp( STDIN_FILENO)) == -1) {
+						fprintf( stderr,
+						         _("%s: Cannot get process group id: %s\n"),
+						         Prog, strerror( errno));
 						stop = true;
+                        errno = 0;
 					}
-
-					if (   (bytes_r > 0)
-					    && (write (fd_ptmx, trbuf, bytes_r) != bytes_r)) {
-						if (errno == EINTR || errno == EIO) {
-							/* FIXME: are we
-							 * loosing some
-							 * bytes here? */
-							continue;
+					else {
+						
+						/* Running in foreground  if equal */
+						if( pg_pid_tmp == pg_pid) {
+
+							/* Set raw mode again if last run was in background */
+							if( pg_pid_cmp != pg_pid_tmp) {
+
+								/* Fetch term settings again because parent might have changed settings */
+								if( tcgetattr( STDIN_FILENO, &termset_save) == -1) {
+									fprintf( stderr,
+                                             _("%s: Cannot get termios attributes: %s\n"),
+                                             Prog, strerror( errno));
+									stop = true;
+                                    errno = 0;
+								}
+								else {
+									/* Set RAW mode  */
+                                    if( term_setattr( STDIN_FILENO, &termset_save, !doshell) ==
false) {
+                                        stop = true;
+                                        errno = 0;
+                                    }
+								}
+								pg_pid_cmp = pg_pid_tmp;
+							}
 						}
-						fprintf (stderr, _("%s: Failure in writing to session\r\n"), Prog);
-						stop = true;
 					}
+
+
+					bytes_r = read( STDIN_FILENO, trbuf, BUFSIZ);
+					if( bytes_r <= 0) {
+						if( errno != EINTR && errno != EIO) {
+							fprintf( stderr,
+                                     _("%s: Failure in reading from stdin: %s\r\n"),
+                                     Prog, strerror( errno));
+						    stop = true;
+                            errno = 0;
+                        }
+					}
+
+                    while( bytes_r > 0) {
+					    ret = write( fd_ptmx, trbuf, bytes_r);
+                        if( ret < 0) {
+						    if( errno != EINTR ) {
+						        fprintf( stderr,
+                                         _("%s: Failure in writing to session: %s\r\n"),
+                                        Prog, strerror( errno));
+						        stop = true;
+                                bytes_r = 0;
+                            }
+                            errno = 0;
+                        }
+                        else
+                            bytes_r -= ret;
+                    }
 				}

 				if (FD_ISSET (fd_ptmx, &inp_fds)) {
 					bytes_r = read (fd_ptmx, trbuf, BUFSIZ);
 					if (bytes_r <= 0) {
-						if (errno == EINTR || errno == EIO) {
-							continue;
-						}
-						fprintf (stderr,
-						         _("%s: Failure in reading from session: %s\r\n"),
-						         Prog, strerror (errno));
-						stop = true;
+						if (errno != EINTR && errno != EIO) {
+						    fprintf (stderr,
+						             _("%s: Failure in reading from session: %s\r\n"),
+						             Prog, strerror (errno));
+						    stop = true;
+                            errno = 0;
+                        }
 					}

-					if (bytes_r > 0 && write (STDOUT_FILENO, trbuf, bytes_r) != bytes_r) {
-						fprintf (stderr,
-						         _("%s: Failure in writing to stdout\r\n"),
-						         Prog);
-						stop = true;
-					}
+                    while( bytes_r > 0) {
+					    ret = write( STDOUT_FILENO, trbuf, bytes_r);
+                        if( ret < 0) {
+						    if( errno != EINTR ) {
+						        fprintf( stderr,
+                                         _("%s: Failure in writing to stdout: %s\r\n"),
+                                        Prog, strerror( errno));
+						        stop = true;
+                                bytes_r = 0;
+                            }
+                            errno = 0;
+                        }
+                        else
+                            bytes_r -= ret;
+                    }
 				}
-			}
-		} while (!stop);

-		if (have_tty) {
-			(void) close (fd_pts);
-			/* Reset RAW mode  */
-			if (tcsetattr (STDIN_FILENO, TCSANOW, &termset_save) == -1) {
-				fprintf (stderr,
-				         _("%s: Cannot reset termios attributes\n"),
-				         Prog);
-				/* caught not set */
+				if( pid == (pid_t)0 && caught == 0 && stop == true)
+					caught = SIGHUP;
 			}
-		}
+		} while (!stop);
 	}

+
+    if (have_tty) {
+        (void) close (fd_pts);
+        /* Reset RAW mode  */
+        if (tcsetattr (STDIN_FILENO, TCSANOW, &termset_save) == -1) {
+            fprintf (stderr,
+                     _("%s: Cannot reset termios attributes\n"),
+                     Prog);
+        }
+    }
+
 	if (0 != caught) {
 		(void) fputs ("\n", stderr);
 		(void) fputs (_("Session terminated, terminating shell..."),
@@ -1326,7 +1454,7 @@ int main (int argc, char **argv)
 		 * Use the shell and create an argv
 		 * with the rest of the command line included.
 		 */
-		argv[-1] = cp;
+		argv[-1] = (char *)cp;
 		execve_shell (shellstr, &argv[-1], environ);
 		err = errno;
 		(void) fprintf (stderr,
___END_PATCH___



Best regards
Wolf



More information about the Pkg-shadow-devel mailing list