#840586 more(1) may attempt to tcsetattr(3) despite lacking controlling terminal

Package:
util-linux
Source:
util-linux
Description:
miscellaneous system utilities
Submitter:
Jan Schaumann
Date:
2024-11-16 14:27:03 UTC
Severity:
normal
Tags:
#840586#5
Date:
2016-10-13 00:44:51 UTC
From:
To:
more(1) may attempt to call tsetattr(3) on stderr even if its process is
not in a process group with a controlling terminal.  As a result,
SIGTTOU will be generated, suspending the process.

The following example illustrates the issue:

$ /bin/sh -c "timeout 60 /bin/sh -c \"ls | more\""

This command will now hang until the timeout, since the shell, ls(1) and
more(1) commands invoked by timeout(1) will be suspended.

Note the difference to

$ timeout 60 /bin/sh -c "ls | more"

which behaves as expected, since here the entire shell command is in the
process group that has the controlling terminal.


The problem is in text-utils/more.c, where a call to tcsetattr(3) is
made on stderr.  The test for whether or not a tty is present is not
sufficient here, since a tty may be present but not be in the process
group of the controlling terminal.

A simplified PoC of the problematic code looks like this:
--- 8< --------- 8< --------- 8< ---------

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <termios.h>
#include <unistd.h>

int
main(int argc, char ** argv) {
        struct termios otty;
        int no_tty;

        if (setpgid(0, 0) == -1) {
                fprintf(stderr, "Unable to setpgid: %s\n", strerror(errno));
                exit(1);
        }

        no_tty = tcgetattr(fileno(stdout), &otty);

        if (!no_tty) {
#ifdef FIX
                if (tcgetpgrp(fileno(stderr)) == getpgrp()) {
#endif
                        if (tcsetattr(fileno(stderr), TCSANOW, &otty) == -1) {
                                fprintf(stderr, "Unable to tcsetattr: %s\n", strerror(errno));
                                exit(1);
                        }
#ifdef FIX
                }
#endif
        }
        return 0;
}
--- 8< --------- 8< --------- 8< --------- If compiled without '-DFIX', then we observe the following: $ ./a.out $ /bin/sh -c ./a.out [hangs, can't ^C, need to suspend and kill] If compiled with '-DFIX', we get the desired behaviour. The fix to text-utils/more.c should then be:
--- 8< --------- 8< --------- 8< ---------
--- more.c 2016-10-12 18:19:00.858761448 -0400 +++ more.c.orig 2011-09-26 05:50:25.000000000 -0400 @@ -1769,16 +1769,6 @@ retry: #endif /* do_SIGTTOU */ no_tty = tcgetattr(fileno(stdout), &otty); - no_intty = tcgetattr(fileno(stdin), &otty); - - /* If we are not in the process group with the controlling terminal, - * then we have on business trying to interact with it, so let's - * pretend there isn't a tty. */ - if (tcgetpgrp(fileno(stdout)) != getpgrp()) { - no_tty = 1; - no_intty = 1; - } - if (!no_tty) { docrterase = (otty.c_cc[VERASE] != 255); docrtkill = (otty.c_cc[VKILL] != 255); @@ -1880,7 +1870,7 @@ if ((shell = getenv("SHELL")) == NULL) shell = "/bin/sh"; } - + no_intty = tcgetattr(fileno(stdin), &otty); tcgetattr(fileno(stderr), &otty); savetty0 = otty; slow_tty = cfgetispeed(&otty) < B1200;
--- 8< --------- 8< --------- 8< --------- That is, if the current process group is not the same as the process group of the controlling terminal, then it's best to pretend that we don't have a terminal and cause more(1) to merely copy the input rather than attempt to paginate.
#840586#10
Date:
2016-10-13 11:33:31 UTC
From:
To:
Hello Jan Schaumann.

Thanks for your bug report.
[...]

If I'm not mistaken this testcase makes me think that the problem
is still there in util-linux 2.28.2-1.

Could you please contact upstream directly about this issue?

http://vger.kernel.org/vger-lists.html#util-linux

Regards,
Andreas Henriksson

#840586#15
Date:
2016-10-13 15:28:57 UTC
From:
To:
Andreas Henriksson <andreas@fatal.se> wrote:
https://marc.info/?l=util-linux-ng&m=147637020702238&w=2

#840586#22
Date:
2024-11-16 14:24:41 UTC
From:
To:
Control: tags -1 + upstream wontfix

Marking upstream wontfix, to clarify that I won't apply a
Debian-specific patch. If/when upstream fixes this, happy to apply
upstream's patch.

Chris