search

checkpoint-restore / criu

Checkpoint/Restore tool
criu.org
Other
2.86k stars 576 forks source link

tty: don't restore locking with --unprivileged #2238

Closed rst0git closed 1 year ago

rst0git commented 1 year ago

TIOCSLCKTRMIOS requires CAP_SYS_ADMIN. This causes criu restore to fail when used with --unprivileged.

codecov-commenter commented 1 year ago

Codecov Report

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

Comparison is base (72494ed) 70.65% compared to head (37b9396) 70.60%.

:exclamation: Current head 37b9396 differs from pull request most recent head 2e84b58. Consider uploading reports for the commit 2e84b58 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## criu-dev #2238 +/- ## ============================================ - Coverage 70.65% 70.60% -0.05% ============================================ Files 133 133 Lines 33317 33320 +3 ============================================ - Hits 23540 23527 -13 - Misses 9777 9793 +16 ``` | [Files Changed](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [criu/tty.c](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y3JpdS90dHkuYw==) | `77.46% <66.66%> (+0.02%)` | :arrow_up: | ... and [6 files with indirect coverage changes](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2238/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

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

avagin commented 1 year ago

Pls add a full description what this ioctl is about and why it is safe to do what you are doing in this patch.

avagin commented 1 year ago

@rst0git ping

rst0git commented 1 year ago

Pls add a full description what this ioctl is about and why it is safe to do what you are doing in this patch.

Thank you for the question. I investigated this further and while skipping TIOCSLCKTRMIOS for unprivileged restore seems to work, it might not be safe. Instead, it seems that allowing CAP_CHECKPOINT_RESTORE to be used with TIOCSLCKTRMIOS is more appropriate.