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

[Wolfgang Zarre]

Hi again,

Sorry, but I was submitting the wrong patch by mistake but
here now the right one:

___BEGIN_PATCH___
diff --git a/src/su.c b/src/su.c
index 34f6771..8053225 100644
--- a/src/su.c
+++ b/src/su.c
@@ -60,7 +60,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -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) {
-

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

[Wolfgang Zarre]

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 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -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 m

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

[Nicolas François]

Hi,

On Tue, Aug 27, 2013 at 11:43:59AM +0200, lk...@essax.com wrote:
> 
> First, I'm sorry that it took me now nearly three month to finish because 
> always
> when I have the impression having time left it turns the opposite.

I can't blame you here...

> > I created a branch (su-C_tty) starting with the patch from Wolfgang Zarre
> > (comment 141 in #628843). Thanks!
> 
> You are welcome! The branch is on svn.debian.org ?

There's been a switch to git in the mean time.
You can find the repo on github:
https://github.com/shadow-maint/shadow

> > There is one thing I don't understand (let's start with this one):
> > When I execute a command with su –c , it does not react to
> > SIGTSTP (either from Ctrl-Z or kill -SIGTSTP). It works OK when I execute
> > a shell.
> > 
> > Is this behavior expected? What is the reason?
> 
> Actually it's not expected however, there where issues as mentioned in #156
> but was targeting more the issues as mentioned above to switch correct the
> parent's tty between cooked and raw mode.
> 
> A short test was showing now that this is really not working as expected and
> I'll try to investigate and rework the patch accordingly.


Thanks for your time.

Best Regards,
-- 
Nekral


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org