bin/51556: less(1) generates SIGTTOU if lacking a controlling terminal

[jschauma%netmeister.org@localhost]

>Number:         51556
>Category:       bin
>Synopsis:       less(1) generates SIGTTOU if lacking a controlling terminal
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Oct 13 02:10:00 +0000 2016
>Originator:     Jan Schaumann
>Release:        NetBSD 7.0.1
>Organization:
	
>Environment:
Architecture: x86_64
Machine: amd64
>Description:

I just chased down a bug in Debian's more(1) where more(1) called
tcsetattr(3) despire not having a controlling terminal:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=840586

The same issue appears to exist in our less(1), but I haven't had the
time to pinpoint it in the code (which is notably more complex than
more(1)'s).

To summarize:

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

will hang, since the second sh(1) is suspended after receiving SIGTTOU
generated by more(1) (which, in our case, is less(1)).

This is due to timeout(1) creating a new process group, leading to the
processes it invokes not having a controlling terminal.

$ ps x -o pid,ppid,pgid,tpgid,stat,comm | egrep "(/bin/sh|timeout|more)"
 4345 25691  4345 25691 S    timeout
 6811  4345  4345 25691 T    /bin/sh
13202  6811  4345 25691 T    more
25691 14973 25691 25691 S+   /bin/sh

Note that PIDs 4345, 6811, and 13202 are not in the process group 25691,
which has the controlling terminal.

If somebody more familiar with the less(1) codebase and terminal IO
could take a look where we might be issuing writes to the controlling
terminal (via ioctl(2), I suppose), that'd be great.


>How-To-Repeat:

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

>Fix:

Only perform write actions on stdout/stderr if

tcgetpgrp(fileno(stderr)) == getpgrp()

I suspect this check needs to be added in a few places in extern/bsd/less/dist/screen.c ?

>Unformatted: