[PATCH] script(1): nothing written to typescript or to stdout when stdin is not a tty

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] script(1): nothing written to typescript or to stdout when stdin is not a tty

Soumendra Ganguly-2
Two bugs:

1. When stdin is not a tty, tcgetattr and ioctl+TIOCGWINSZ on stdin
fail, leaving tt and win having all bytes set to 0. Next, when openpty
is called with &tt and &win as arguments, all termios and winsize
fields of the slave end of the pty are set to 0.
As a consequence, nothing is written to the stdout or to the typescript.

To reproduce issue, make a regular file "test" with the following contents

ls
exit
<newline here>

Then run "script < test".

Fix: call openpty with NULL, NULL instead of &tt, &win.

2. When stdin is not a tty, even after the above issue is fixed, the
parent process does not wait on the child process pid. While that
itself is a problem, it also leads to malformed output:

"Script started, ..."
"Script done, ..."
ALL OUTPUT HERE

It should instead be like this:

"Script started, ..."
ALL OUTPUT HERE
"Script done, ..."

Patch follows.

--- src/usr.bin/script/script.c 2020-08-04 02:41:13.226129480 -0500
+++ script.c 2020-08-25 15:40:57.475954507 -0500
@@ -132,13 +132,15 @@
  if ((fscript = fopen(fname, aflg ? "a" : "w")) == NULL)
  err(1, "%s", fname);

- if (isatty(0)) {
- if (tcgetattr(STDIN_FILENO, &tt) == 0 &&
-    ioctl(STDIN_FILENO, TIOCGWINSZ, &win) == 0)
- istty = 1;
+ if (tcgetattr(STDIN_FILENO, &tt) == 0 &&
+    ioctl(STDIN_FILENO, TIOCGWINSZ, &win) == 0) {
+ if (openpty(&master, &slave, NULL, &tt, &win) == -1)
+ err(EXIT_FAILURE, "openpty");
+ istty = 1;
+ } else {
+ if (openpty(&master, &slave, NULL, NULL, NULL) == -1)
+ err(EXIT_FAILURE, "openpty");
  }
- if (openpty(&master, &slave, NULL, &tt, &win) == -1)
- err(1, "openpty");

  (void)printf("Script started, output file is %s\n", fname);
  if (istty) {
@@ -147,13 +149,13 @@
  cfmakeraw(&rtt);
  rtt.c_lflag &= ~ECHO;
  (void)tcsetattr(STDIN_FILENO, TCSAFLUSH, &rtt);
- }

- bzero(&sa, sizeof sa);
- sigemptyset(&sa.sa_mask);
- sa.sa_handler = handlesigwinch;
- sa.sa_flags = SA_RESTART;
- (void)sigaction(SIGWINCH, &sa, NULL);
+ bzero(&sa, sizeof sa);
+ sigemptyset(&sa.sa_mask);
+ sa.sa_handler = handlesigwinch;
+ sa.sa_flags = SA_RESTART;
+ (void)sigaction(SIGWINCH, &sa, NULL);
+ }

  child = fork();
  if (child == -1) {
@@ -199,6 +201,8 @@
  off += n;
  }
  }
+ while (!dead) /* Wait for SIGCHLD. */
+ sleep(1);
  done(sigdeadstatus);
 }

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] script(1): nothing written to typescript or to stdout when stdin is not a tty

Soumendra Ganguly-2
The while+sleep wait for child has now been replaced with
sigemptyset+sigsuspend.



--- src/usr.bin/script/script.c 2020-08-04 02:41:13.226129480 -0500
+++ script.c 2020-08-30 19:59:40.852918727 -0500
@@ -105,6 +105,7 @@
  char *cmd;
  ssize_t cc, off;
  int aflg, ch;
+ sigset_t set;

  cmd = NULL;
  aflg = 0;
@@ -132,13 +133,15 @@
  if ((fscript = fopen(fname, aflg ? "a" : "w")) == NULL)
  err(1, "%s", fname);

- if (isatty(0)) {
- if (tcgetattr(STDIN_FILENO, &tt) == 0 &&
-    ioctl(STDIN_FILENO, TIOCGWINSZ, &win) == 0)
- istty = 1;
+ if (tcgetattr(STDIN_FILENO, &tt) == 0 &&
+    ioctl(STDIN_FILENO, TIOCGWINSZ, &win) == 0) {
+ if (openpty(&master, &slave, NULL, &tt, &win) == -1)
+ err(EXIT_FAILURE, "openpty");
+ istty = 1;
+ } else {
+ if (openpty(&master, &slave, NULL, NULL, NULL) == -1)
+ err(EXIT_FAILURE, "openpty");
  }
- if (openpty(&master, &slave, NULL, &tt, &win) == -1)
- err(1, "openpty");

  (void)printf("Script started, output file is %s\n", fname);
  if (istty) {
@@ -147,13 +150,13 @@
  cfmakeraw(&rtt);
  rtt.c_lflag &= ~ECHO;
  (void)tcsetattr(STDIN_FILENO, TCSAFLUSH, &rtt);
- }

- bzero(&sa, sizeof sa);
- sigemptyset(&sa.sa_mask);
- sa.sa_handler = handlesigwinch;
- sa.sa_flags = SA_RESTART;
- (void)sigaction(SIGWINCH, &sa, NULL);
+ bzero(&sa, sizeof sa);
+ sigemptyset(&sa.sa_mask);
+ sa.sa_handler = handlesigwinch;
+ sa.sa_flags = SA_RESTART;
+ (void)sigaction(SIGWINCH, &sa, NULL);
+ }

  child = fork();
  if (child == -1) {
@@ -199,6 +202,12 @@
  off += n;
  }
  }
+ if (!istty) {
+ sigemptyset(&set);
+ sigsuspend(&set);
+ if (!dead)
+ done(1);
+ }
  done(sigdeadstatus);
 }



On 8/25/20, Soumendra Ganguly <[hidden email]> wrote:

> Two bugs:
>
> 1. When stdin is not a tty, tcgetattr and ioctl+TIOCGWINSZ on stdin
> fail, leaving tt and win having all bytes set to 0. Next, when openpty
> is called with &tt and &win as arguments, all termios and winsize
> fields of the slave end of the pty are set to 0.
> As a consequence, nothing is written to the stdout or to the typescript.
>
> To reproduce issue, make a regular file "test" with the following contents
>
> ls
> exit
> <newline here>
>
> Then run "script < test".
>
> Fix: call openpty with NULL, NULL instead of &tt, &win.
>
> 2. When stdin is not a tty, even after the above issue is fixed, the
> parent process does not wait on the child process pid. While that
> itself is a problem, it also leads to malformed output:
>
> "Script started, ..."
> "Script done, ..."
> ALL OUTPUT HERE
>
> It should instead be like this:
>
> "Script started, ..."
> ALL OUTPUT HERE
> "Script done, ..."
>
> Patch follows.
>
> --- src/usr.bin/script/script.c 2020-08-04 02:41:13.226129480 -0500
> +++ script.c 2020-08-25 15:40:57.475954507 -0500
> @@ -132,13 +132,15 @@
>   if ((fscript = fopen(fname, aflg ? "a" : "w")) == NULL)
>   err(1, "%s", fname);
>
> - if (isatty(0)) {
> - if (tcgetattr(STDIN_FILENO, &tt) == 0 &&
> -    ioctl(STDIN_FILENO, TIOCGWINSZ, &win) == 0)
> - istty = 1;
> + if (tcgetattr(STDIN_FILENO, &tt) == 0 &&
> +    ioctl(STDIN_FILENO, TIOCGWINSZ, &win) == 0) {
> + if (openpty(&master, &slave, NULL, &tt, &win) == -1)
> + err(EXIT_FAILURE, "openpty");
> + istty = 1;
> + } else {
> + if (openpty(&master, &slave, NULL, NULL, NULL) == -1)
> + err(EXIT_FAILURE, "openpty");
>   }
> - if (openpty(&master, &slave, NULL, &tt, &win) == -1)
> - err(1, "openpty");
>
>   (void)printf("Script started, output file is %s\n", fname);
>   if (istty) {
> @@ -147,13 +149,13 @@
>   cfmakeraw(&rtt);
>   rtt.c_lflag &= ~ECHO;
>   (void)tcsetattr(STDIN_FILENO, TCSAFLUSH, &rtt);
> - }
>
> - bzero(&sa, sizeof sa);
> - sigemptyset(&sa.sa_mask);
> - sa.sa_handler = handlesigwinch;
> - sa.sa_flags = SA_RESTART;
> - (void)sigaction(SIGWINCH, &sa, NULL);
> + bzero(&sa, sizeof sa);
> + sigemptyset(&sa.sa_mask);
> + sa.sa_handler = handlesigwinch;
> + sa.sa_flags = SA_RESTART;
> + (void)sigaction(SIGWINCH, &sa, NULL);
> + }
>
>   child = fork();
>   if (child == -1) {
> @@ -199,6 +201,8 @@
>   off += n;
>   }
>   }
> + while (!dead) /* Wait for SIGCHLD. */
> + sleep(1);
>   done(sigdeadstatus);
>  }
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] script(1): nothing written to typescript or to stdout when stdin is not a tty

Soumendra Ganguly-2
Anyone interested in this?

On 8/30/20, Soumendra Ganguly <[hidden email]> wrote:

> The while+sleep wait for child has now been replaced with
> sigemptyset+sigsuspend.
>
>
>
> --- src/usr.bin/script/script.c 2020-08-04 02:41:13.226129480 -0500
> +++ script.c 2020-08-30 19:59:40.852918727 -0500
> @@ -105,6 +105,7 @@
>   char *cmd;
>   ssize_t cc, off;
>   int aflg, ch;
> + sigset_t set;
>
>   cmd = NULL;
>   aflg = 0;
> @@ -132,13 +133,15 @@
>   if ((fscript = fopen(fname, aflg ? "a" : "w")) == NULL)
>   err(1, "%s", fname);
>
> - if (isatty(0)) {
> - if (tcgetattr(STDIN_FILENO, &tt) == 0 &&
> -    ioctl(STDIN_FILENO, TIOCGWINSZ, &win) == 0)
> - istty = 1;
> + if (tcgetattr(STDIN_FILENO, &tt) == 0 &&
> +    ioctl(STDIN_FILENO, TIOCGWINSZ, &win) == 0) {
> + if (openpty(&master, &slave, NULL, &tt, &win) == -1)
> + err(EXIT_FAILURE, "openpty");
> + istty = 1;
> + } else {
> + if (openpty(&master, &slave, NULL, NULL, NULL) == -1)
> + err(EXIT_FAILURE, "openpty");
>   }
> - if (openpty(&master, &slave, NULL, &tt, &win) == -1)
> - err(1, "openpty");
>
>   (void)printf("Script started, output file is %s\n", fname);
>   if (istty) {
> @@ -147,13 +150,13 @@
>   cfmakeraw(&rtt);
>   rtt.c_lflag &= ~ECHO;
>   (void)tcsetattr(STDIN_FILENO, TCSAFLUSH, &rtt);
> - }
>
> - bzero(&sa, sizeof sa);
> - sigemptyset(&sa.sa_mask);
> - sa.sa_handler = handlesigwinch;
> - sa.sa_flags = SA_RESTART;
> - (void)sigaction(SIGWINCH, &sa, NULL);
> + bzero(&sa, sizeof sa);
> + sigemptyset(&sa.sa_mask);
> + sa.sa_handler = handlesigwinch;
> + sa.sa_flags = SA_RESTART;
> + (void)sigaction(SIGWINCH, &sa, NULL);
> + }
>
>   child = fork();
>   if (child == -1) {
> @@ -199,6 +202,12 @@
>   off += n;
>   }
>   }
> + if (!istty) {
> + sigemptyset(&set);
> + sigsuspend(&set);
> + if (!dead)
> + done(1);
> + }
>   done(sigdeadstatus);
>  }
>
>
>
> On 8/25/20, Soumendra Ganguly <[hidden email]> wrote:
>> Two bugs:
>>
>> 1. When stdin is not a tty, tcgetattr and ioctl+TIOCGWINSZ on stdin
>> fail, leaving tt and win having all bytes set to 0. Next, when openpty
>> is called with &tt and &win as arguments, all termios and winsize
>> fields of the slave end of the pty are set to 0.
>> As a consequence, nothing is written to the stdout or to the typescript.
>>
>> To reproduce issue, make a regular file "test" with the following
>> contents
>>
>> ls
>> exit
>> <newline here>
>>
>> Then run "script < test".
>>
>> Fix: call openpty with NULL, NULL instead of &tt, &win.
>>
>> 2. When stdin is not a tty, even after the above issue is fixed, the
>> parent process does not wait on the child process pid. While that
>> itself is a problem, it also leads to malformed output:
>>
>> "Script started, ..."
>> "Script done, ..."
>> ALL OUTPUT HERE
>>
>> It should instead be like this:
>>
>> "Script started, ..."
>> ALL OUTPUT HERE
>> "Script done, ..."
>>
>> Patch follows.
>>
>> --- src/usr.bin/script/script.c 2020-08-04 02:41:13.226129480 -0500
>> +++ script.c 2020-08-25 15:40:57.475954507 -0500
>> @@ -132,13 +132,15 @@
>>   if ((fscript = fopen(fname, aflg ? "a" : "w")) == NULL)
>>   err(1, "%s", fname);
>>
>> - if (isatty(0)) {
>> - if (tcgetattr(STDIN_FILENO, &tt) == 0 &&
>> -    ioctl(STDIN_FILENO, TIOCGWINSZ, &win) == 0)
>> - istty = 1;
>> + if (tcgetattr(STDIN_FILENO, &tt) == 0 &&
>> +    ioctl(STDIN_FILENO, TIOCGWINSZ, &win) == 0) {
>> + if (openpty(&master, &slave, NULL, &tt, &win) == -1)
>> + err(EXIT_FAILURE, "openpty");
>> + istty = 1;
>> + } else {
>> + if (openpty(&master, &slave, NULL, NULL, NULL) == -1)
>> + err(EXIT_FAILURE, "openpty");
>>   }
>> - if (openpty(&master, &slave, NULL, &tt, &win) == -1)
>> - err(1, "openpty");
>>
>>   (void)printf("Script started, output file is %s\n", fname);
>>   if (istty) {
>> @@ -147,13 +149,13 @@
>>   cfmakeraw(&rtt);
>>   rtt.c_lflag &= ~ECHO;
>>   (void)tcsetattr(STDIN_FILENO, TCSAFLUSH, &rtt);
>> - }
>>
>> - bzero(&sa, sizeof sa);
>> - sigemptyset(&sa.sa_mask);
>> - sa.sa_handler = handlesigwinch;
>> - sa.sa_flags = SA_RESTART;
>> - (void)sigaction(SIGWINCH, &sa, NULL);
>> + bzero(&sa, sizeof sa);
>> + sigemptyset(&sa.sa_mask);
>> + sa.sa_handler = handlesigwinch;
>> + sa.sa_flags = SA_RESTART;
>> + (void)sigaction(SIGWINCH, &sa, NULL);
>> + }
>>
>>   child = fork();
>>   if (child == -1) {
>> @@ -199,6 +201,8 @@
>>   off += n;
>>   }
>>   }
>> + while (!dead) /* Wait for SIGCHLD. */
>> + sleep(1);
>>   done(sigdeadstatus);
>>  }
>>
>