tty: Only store the stdin fd when it corresponds to a tty

[nviennot]

Doing otherwise can lead to problems when using inherit-fd on stdin as it gets closed.

[codecov-commenter]

Codecov Report

Patch coverage: 66.66% and project coverage change: -0.01% :warning:

Comparison is base (2df6ec5) 70.68% compared to head (5195d92) 70.67%.

:exclamation: Current head 5195d92 differs from pull request most recent head 2b71063. Consider uploading reports for the commit 2b71063 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##           criu-dev    #1455      +/-   ##
============================================
- Coverage     70.68%   70.67%   -0.01%     
============================================
  Files           133      133              
  Lines         33321    33317       -4     
============================================
- Hits          23552    23548       -4     
  Misses         9769     9769              

Files Changed	Coverage Δ
criu/tty.c	77.41% <60.00%> (-0.03%)	:arrow_down:
criu/files.c	80.83% <100.00%> (ø)

... and 5 files with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[nviennot]

When using --inherit-fd fd[0]:XXX (for example, to replace resource XXX with the file provided via stdin), the following gets invoked: https://github.com/checkpoint-restore/criu/blob/f1cc40c5123880c86768645c76935300a06fa792/criu/files.c#L1599 (upd: permalink) This sends stdin into the fdstore, and closes stdin. From that point, fd=0 is now closed, and the tty code tried to use it again.

[Snorch]

When using --inherit-fd fd[0]:XXX (for example, to replace resource XXX with the file provided via stdin), the following gets invoked: https://github.com/checkpoint-restore/criu/blob/criu-dev/criu/files.c#L1599 This sends stdin into the fdstore, and closes stdin. From that point, fd=0 is now closed, and the tty code tried to use it again.

Probably we can instead don't allow --inherit-fd on 0-2 standard io fds?

upd: not instead but also

[rst0git]

Probably we can instead don't allow --inherit-fd on 0-2 standard io fds?

I remember having a similar discussion in https://github.com/checkpoint-restore/criu/pull/681

[Snorch]

Probably we can instead don't allow --inherit-fd on 0-2 standard io fds?

I remember having a similar discussion in #681

Nice note. Seems like the problem of reusing 0-2 fds was already there with opts.ps_socket, and we've had "keep_fd" logic to protect from it, but we've removed it, relying that opts.ps_socket just would not use 0-2.

[github-actions[bot]]

A friendly reminder that this PR had no activity for 30 days.

[Snorch]

@avagin I reworked it:

• removed stdin_isatty
• added patch to prohibit --inherit-fd on std fds