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% ⚠️

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

❗ 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%)	⬇️
criu/files.c	80.83% <100.00%> (ø)

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Snorch]

I like the code. I think that there is no point to put an fd into fdstore if it would not be used from fdstore, as self_stdin_fdid is currently only used if stdin_isatty == true.

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

But, can you please explain how adding the fd to fdstore can results in the original fd been closed?

[nviennot]

When using --inherit-fd fd[0]:XXX (for example, to replace resource XXX with the file provided via stdin), the following gets invoked:

criu/criu/files.c

Line 1599 in f1cc40c

close_safe(&inh->inh_fd);

(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 #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]

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

[rst0git]

LGTM