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

[1.1] Fix working with read-only /dev #3355

Merged
merged 3 commits into from Jan 28, 2022

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Jan 27, 2022

This is a backport of #3345 to 1.1 branch, needed for newer Podman (see containers/podman#12954).

See original PR for details.

kolyshkin added 3 commits Jan 27, 2022
Use os/file Chown method instead of bare unix.Fchown as it already have
access to underlying fd, and produces nice-looking errors. This allows
us to remove our error wrapping and some linter annotations.

We still use unix.Fstat since os.Stat access to os-specific fields
like uid/gid is not very straightforward. The only change here is to use
file name (rather than fd) in the error text.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit b7fdb68)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since we already called fstat, we know the current file uid. In case it
is the same as the one we want it to be, there's no point in trying
chown.

Remove the specific /dev/null check, as the above also covers it
(comparing /dev/null uid with itself is true).

This also fixes runc exec with read-only /dev for root user.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 18c4760)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case of a read-only /dev, it's better to move on and let whatever is
run in a container to handle any possible errors.

This solves runc exec for a user with read-only /dev.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 146c8c0)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor Author

kolyshkin commented Jan 28, 2022

OK, it seems we have wrong branch protection rules for release-1.1 (that have test (1.13.x included, but no 1.17 ones).

Will fix.

@kolyshkin
Copy link
Contributor Author

kolyshkin commented Jan 28, 2022

I have created branch protection rules for release-1.1, and also enabled the following (as an experiment):

  • Require branches to be up to date before merging
    This ensures pull requests targeting a matching branch have been tested with the latest code.

Perhaps it makes sense to enable it for main as well, since I've seen a few times that independent merges result in broken code.

@mrunalp mrunalp merged commit b9460f2 into opencontainers:release-1.1 Jan 28, 2022
24 checks passed
@kolyshkin kolyshkin mentioned this pull request Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.1-pr A backport to 1.1.x release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants