[Bug 3137] New: -f keeps stdin and stderr open

[bugzilla-daemon at mindrot.org]

https://bugzilla.mindrot.org/show_bug.cgi?id=3137

            Bug ID: 3137
           Summary: -f keeps stdin and stderr open
           Product: Portable OpenSSH
           Version: 8.2p1
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: ionic at ionic.de

As a more general case of #1988, the -f flag also seems to keep stdin
and stderr open in the general case:

% ssh ionic.de -f -T 'pv -qL 1 /dev/zero'
% ls -ldh /proc/7115/fd/*
lrwx------ 1 ionic ionic 64 Mar 16 09:21 /proc/7115/fd/0 -> /dev/pts/50
l-wx------ 1 ionic ionic 64 Mar 16 09:21 /proc/7115/fd/1 -> /dev/null
lrwx------ 1 ionic ionic 64 Mar 16 09:21 /proc/7115/fd/2 -> /dev/pts/50
lrwx------ 1 ionic ionic 64 Mar 16 09:21 /proc/7115/fd/3 ->
'socket:[1405985948]'
lrwx------ 1 ionic ionic 64 Mar 16 09:21 /proc/7115/fd/5 -> /dev/pts/50
lrwx------ 1 ionic ionic 64 Mar 16 09:21 /proc/7115/fd/6 -> /dev/pts/50


I'd argue that this is unnecessary and harmful because:
  - the channel will dup2() stdin and stderr anyway (see FD 5 and 6)
  - backgrounding usually means that the user intends to close a
controlling terminal anyway (or doesn't even have one to begin with),
severing the pipes uni-laterally.


Additionally, this effectively lets #1988 resurface when pairing
ControlMaster, ControlPersist and the -f flag.

Using the -f flag disables the "normal" ControlPersist code path that
closes stderr and daemonizes, and instead uses the other -f code path
which does not close stderr. This makes sense to not fork twice, but
also means that the fix from #1988 is outright worked around:

% ssh ionic.de -o ControlMaster=yes -o ControlPersist=yes -o
ControlPath=/home/ionic/.sshsock -f -N -T 'pv -qL 1 /dev/zero'
% ls -ldh /proc/22122/fd/*
lrwx------ 1 ionic ionic 64 Mar 16 09:45 /proc/22122/fd/0 ->
/dev/pts/50
l-wx------ 1 ionic ionic 64 Mar 16 09:45 /proc/22122/fd/1 -> /dev/null
lrwx------ 1 ionic ionic 64 Mar 16 09:45 /proc/22122/fd/2 ->
/dev/pts/50
lrwx------ 1 ionic ionic 64 Mar 16 09:45 /proc/22122/fd/3 ->
'socket:[1406165968]'
lrwx------ 1 ionic ionic 64 Mar 16 09:45 /proc/22122/fd/4 ->
'socket:[1406168946]'

Here, no command is actually executed. We're only interested in the
control socket, but the forked/backgrounded control socket process
retains stdin and stderr.


FWIW, keeping stdin connected/open might be okay, but I don't see any
benefit in doing so.

Keeping stderr open might be useful if -v is passed/debugging turned
on, but should otherwise be closed like in the "normal" ControlPersist
case.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.