Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make some no-op permission changes not requiring privilege. #2207

Closed
wants to merge 8 commits into from

Conversation

osctobe
Copy link
Contributor

@osctobe osctobe commented Jun 21, 2023

No description provided.

criu/util.c Outdated Show resolved Hide resolved
@osctobe osctobe force-pushed the less-privileged branch 4 times, most recently from 754abe3 to 6b78278 Compare June 26, 2023 21:01
@osctobe osctobe requested a review from avagin June 27, 2023 17:50
@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2023

Codecov Report

Patch coverage: 50.90% and project coverage change: +0.02 🎉

Comparison is base (2d6f04c) 70.35% compared to head (5600cd1) 70.38%.

❗ Current head 5600cd1 differs from pull request most recent head 9a99d21. Consider uploading reports for the commit 9a99d21 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##           criu-dev    #2207      +/-   ##
============================================
+ Coverage     70.35%   70.38%   +0.02%     
============================================
  Files           133      134       +1     
  Lines         34001    34024      +23     
============================================
+ Hits          23923    23948      +25     
+ Misses        10078    10076       -2     
Impacted Files Coverage Δ
criu/include/util.h 100.00% <ø> (ø)
criu/sk-unix.c 74.52% <ø> (ø)
criu/tty.c 77.44% <ø> (ø)
criu/util.c 63.38% <39.02%> (-1.01%) ⬇️
criu/files-reg.c 75.36% <77.77%> (+0.29%) ⬆️
criu/cgroup.c 74.94% <100.00%> (+0.37%) ⬆️
criu/memfd.c 81.81% <100.00%> (ø)

... and 13 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

criu/pie/restorer.c Outdated Show resolved Hide resolved
criu/util.c Outdated Show resolved Hide resolved
@osctobe osctobe force-pushed the less-privileged branch 2 times, most recently from 82a716e to e665279 Compare July 18, 2023 18:20
Add generic wrappers for fchown() and fchmod() that skip the calls if
no changes are needed. This will allow to unify places where we can
avoid errors when no-op requests are not permitted.

Signed-off-by: Michał Mirosław <[email protected]>
Note: This removes the difference in calling convention of
restore_file_perms() returning -errno that was the only call that did
this in the caller.

From: Radosław Burny <[email protected]>
Signed-off-by: Michał Mirosław <[email protected]>
Signed-off-by: Michał Mirosław <[email protected]>
Signed-off-by: Michał Mirosław <[email protected]>
When CRIU is run with the task's credentials on restore, don't set uids
and gids. This avoids the need to modify the SECURE_NO_SETUID_FIXUP flag
which requires CAP_SETPCAP.

From: Andy Tucker <[email protected]>
Signed-off-by: Michał Mirosław <[email protected]>
Skip calling setgroups() when the list of auxiliary groups already has
the values we want.  This allows restoring into an unprivileged user
namespace where setgroups() is disabled.

From: Ambrose Feinstein <[email protected]>
Signed-off-by: Michał Mirosław <[email protected]>
@avagin
Copy link
Member

avagin commented Jul 22, 2023

I have merged this series with one minor fix of the lint warning. We need to figure out how to test these new calls.

@osctobe osctobe closed this Jul 24, 2023
@osctobe osctobe deleted the less-privileged branch July 24, 2023 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants