Skip to content

libcontainer/cgroups/fscommon: add openat2 support#2668

Merged
AkihiroSuda merged 2 commits intoopencontainers:masterfrom
kolyshkin:openat2
Dec 7, 2020
Merged

libcontainer/cgroups/fscommon: add openat2 support#2668
AkihiroSuda merged 2 commits intoopencontainers:masterfrom
kolyshkin:openat2

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

(based on previous work in PRs #2604, #2605, #2635; originally part of #2598)

In case openat2 is available, it will be used to guarantee
that we're not accessing anything other than cgroupfs[2] files.

In cases when openat2 is not available, or when cgroup has a
non-standard prefix (not "/sys/fs/cgroup", which might theoretically
be the case on some very old installs and/or very custom systems),
fall back to using securejoin + os.Open like we did before.

@kolyshkin
Copy link
Copy Markdown
Contributor Author

Rebased on top of #2666 to fix CI

Comment thread libcontainer/cgroups/fscommon/open.go
Comment thread libcontainer/cgroups/fscommon/open.go Outdated
@kolyshkin
Copy link
Copy Markdown
Contributor Author

CI failure is shfmt that went crazy; caused by mvdan/sh#628.

Restarted travis.

@kolyshkin
Copy link
Copy Markdown
Contributor Author

CI failure is shfmt that went crazy; caused by mvdan/sh#628.

Should be fixed by #2671

Comment thread libcontainer/cgroups/fscommon/open.go
Comment thread libcontainer/cgroups/fscommon/open.go
Comment thread libcontainer/cgroups/fscommon/open.go
@kolyshkin
Copy link
Copy Markdown
Contributor Author

@AkihiroSuda @cyphar @mrunalp can you please re-review? I think I have addressed all concerns and review comments

@kolyshkin
Copy link
Copy Markdown
Contributor Author

@AkihiroSuda @cyphar @mrunalp PTAL

In case openat2 is available, it will be used to guarantee
that we're not accessing anything other than cgroupfs[2] files.

In cases when openat2 is not available, or when cgroup has a
non-standard prefix (not "/sys/fs/cgroup", which might theoretically
be the case on some very old installs and/or very custom systems),
fall back to using securejoin + os.Open like we did before.

Signed-off-by: Kir Kolyshkin <[email protected]>
In case we get ENOSYS from openat2(2), this is expected, so log that
we're falling back to using securejoin as debug.

Otherwise, log it as a warning (as the error is unexpected, but we're
still good to go).

Signed-off-by: Kir Kolyshkin <[email protected]>
Copy link
Copy Markdown
Member

@cyphar cyphar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@AkihiroSuda AkihiroSuda merged commit 4d8d989 into opencontainers:master Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants