Since v1.2.0 was released, a number of users complained that the removal
of tun/tap device access from the default device ruleset is causing a
regression in their workloads.
Additionally, it seems that some upper-level orchestration tools
(Docker Swarm, Kubernetes) makes it either impossible or cumbersome
to supply additional device rules.
While it's probably not quite right to have /dev/net/tun in a default
device list, it was there from the very beginning, and users rely on it.
Let's keep it there for the sake of backward compatibility.
This reverts commit 2ce40b6ad7.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(Cherry-pick of commit 394f4c3b7012674ebe0232c560713e57cbd653e6.)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(This is a cherry-pick of c0044c7aa403ecf2d9172bd9386d05433b011076.)
If we get an unexpected error here, it is probably because of a library
or kernel change that could cause our detection logic to be invalid. As
a result, these warnings should be louder so users have a chance to tell
us about them sooner (or so we might notice them before doing a release,
as happened with the 1.2.0 regression).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(This is a cherry-pick of dea0e04dd93d3922083e68667d20aac532d31129.)
It is possible for LinkAttachProgram to return ErrNotSupported if
program attachment is not supported at all (which doesn't matter in this
case), but it seems possible that upstream will start returning
ErrNotSupported for BPF_F_REPLACE at some point so it's best to make
sure we don't cause additional regressions here.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(This is a cherry-pick of dea0e04dd93d3922083e68667d20aac532d31129.)
In v0.13.0, cilium/ebpf stopped supporting setting BPF_F_REPLACE as an
explicit flag and instead requires us to use link.Anchor to specify
where the program should be attached.
Commit 216175a9ca ("Upgrade Cilium's eBPF library version to 0.16")
did update this correctly for the actual attaching logic, but when
checking for kernel support we still passed BPF_F_REPLACE. This would
result in a generic error being returned, which our feature-support
checking logic would treat as being an error the indicates that
BPF_F_REPLACE *is* supported, resulting in a regression on pre-5.6
kernels.
It turns out that our debug logging saying that this unexpected error
was happening was being output as a result of this change, but nobody
noticed...
Fixes: 216175a9ca ("Upgrade Cilium's eBPF library version to 0.16")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
If the sub-cgroup RemovePath has failed for any reason, return the
error right away. This way, we don't have to check for err != nil
before retrying rmdir.
This is a cosmetic change and should not change any functionality.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 12e06a7c4f)
Signed-off-by: lfbzhm <lifubang@acmcoder.com>
An issue with runc 1.2.0 was reported to buildkit, in which
runc delete returns with an error, with the log saying:
> unable to destroy container: unable to remove container's cgroup: open /sys/fs/cgroup/snschvixiy3s74w74fjantrdg: no such file or directory
Apparently, what happens is runc is running with no cgroup access
(because /sys/fs/cgroup is mounted read-only). In this case error to
create a cgroup path (in runc create/run) is ignored, but cgroup removal
(in runc delete) is not.
This is caused by commit d3d7f7d, which changes the cgroup removal
logic in RemovePath. In the current code, if the initial rmdir has
failed (in this case with EROFS), but the subsequent os.ReadDir returns
ENOENT, it is returned (instead of being ignored -- as the path does not
exist and so there is nothing to remove).
Here is the minimal fix for the issue.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit db59489b68)
Signed-off-by: lfbzhm <lifubang@acmcoder.com>
This allows to do
runc update $ID --memory=-1 --memory-swap=$VAL
for cgroup v2, i.e. set memory to unlimited and swap to a specific
value.
This was not possible because ConvertMemorySwapToCgroupV2Value rejected
memory=-1 ("unlimited"). In a hindsight, it was a mistake, because if
memory limit is unlimited, we should treat memory+swap limit as just swap
limit.
Revise the unit test; add description to each case.
Fixes: c86be8a2 ("cgroupv2: fix setting MemorySwap")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 732806e24c)
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Improve readability of ConvertMemorySwapToCgroupV2Value by switching
from a bunch of if statements to a switch, and adding a comment
describing each case.
No functional change.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit cb9f3d6d14)
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
The userns package was moved to the moby/sys/userns module
at commit 3778ae603c.
This patch deprecates the old location, and adds it as an alias
for the moby/sys/userns package.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Initially, this was a commit to switch from strings.Fields to
strings.SplitN in getCpuUsageBreakdown, since strings.Fields
was probably slower than strings.SplitN in some old Go versions.
Afterwards, strings.Cut was also considered for potential
speed improvements.
After writing a benchmark test, we learned that:
- strings.Fields performance is now adequate;
- strings.SplitN is slower than strings.Fields;
- strings.Cut had <5% performance gain from strings.Fields;
So, remove the TODO and keep the benchmark test.
Signed-off-by: Stavros Panakakis <stavrospanakakis@gmail.com>
Shared pid namespace means `runc kill` (or `runc delete -f`) have to
kill all container processes, not just init. To do so, it needs a cgroup
to read the PIDs from.
If there is no cgroup, processes will be leaked, and so such
configuration is bad and should not be allowed. To keep backward
compatibility, though, let's merely warn about this for now.
Alas, the only way to know if cgroup access is available is by returning
an error from Manager.Apply. Amend fs cgroup managers to do so (systemd
doesn't need it, since v1 can't work with rootless, and cgroup v2 does
not have a special rootless case).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The warnings fixed were:
libcontainer/configs/config_test.go:205:12: printf: non-constant format string in call to (*testing.common).Errorf (govet)
t.Errorf(fmt.Sprintf("Expected error to not occur but it was %+v", err))
^
libcontainer/cgroups/fs/blkio_test.go:481:13: printf: non-constant format string in call to (*testing.common).Errorf (govet)
t.Errorf(fmt.Sprintf("test case '%s' failed unexpectedly: %s", testCase.desc, err))
^
libcontainer/cgroups/fs/blkio_test.go:595:13: printf: non-constant format string in call to (*testing.common).Errorf (govet)
t.Errorf(fmt.Sprintf("test case '%s' failed unexpectedly: %s", testCase.desc, err))
^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
There's too much logic here figuring out which CPUs to use. Runc is a
low level tool and is not supposed to be that "smart". What's worse,
this logic is executed on every exec, making it slower. Some of the
logic in (*setnsProcess).start is executed even if no annotation is set,
thus making ALL execs slow.
Also, this should be a property of a process, rather than annotation.
The plan is to rework this.
This reverts commit afc23e3397.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It has been pointed out that some controllers can not accept multiple
lines of output at once. In particular, io.max can only set one device
at a time.
Practically, the only multi-line resource values we can get come from
unified.* -- let's write those line by line.
Add a test case.
Reported-by: Tao Shen <shentaoskyking@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When swap is being disabled (as set to 0), or set to max, ignore
non-existent memory.swap.max cgroup file.
If swap is being set explicitly to some value, do return an error like
before.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This test panics if userns is detected (such as when run in a rootless
docker container) because SetV1 does nothing in this case.
We could fix the panic, but it doesn't make sense to run the test at
all.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The issue is the same as in commit 1b2adcf but for RT scheduler;
the fix is also the same.
Test case by ls-ggg.
Co-authored-by: ls-ggg <335814617@qq.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
If CPU burst knob is non-existent, the current implementation (added in
commit e1584831) still tries to set it again after setting the new CPU
quota, which is useless (and we have to ignore ENOENT again).
Fix this.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit b6967fa84c moved the functionality of managing cgroup devices
into a separate package, and decoupled libcontainer/cgroups from it.
Yet, some software (e.g. cadvisor) may need to use libcontainer package,
which imports libcontainer/cgroups/devices, thus making it impossible to
use libcontainer without bringing in cgroup/devices dependency.
In fact, we only need to manage devices in runc binary, so move the
import to main.go.
The need to import libct/cg/dev in order to manage devices is already
documented in libcontainer/cgroups, but let's
- update that documentation;
- add a similar note to libcontainer/cgroups/systemd;
- add a note to libct README.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This handles a corner case when joining a container having all
the processes running exclusively on isolated CPU cores to force
the kernel to schedule runc process on the first CPU core within the
cgroups cpuset.
The introduction of the kernel commit
46a87b3851f0d6eb05e6d83d5c5a30df0eca8f76 has affected this deterministic
scheduling behavior by distributing tasks across CPU cores within the
cgroups cpuset. Some intensive real-time application are relying on this
deterministic behavior and use the first CPU core to run a slow thread
while other CPU cores are fully used by real-time threads with SCHED_FIFO
policy. Such applications prevents runc process from joining a container
when the runc process is randomly scheduled on a CPU core owned by a
real-time thread.
Introduces isolated CPU affinity transition OCI runtime annotation
org.opencontainers.runc.exec.isolated-cpu-affinity-transition to restore
the behavior during runc exec.
Fix issue with kernel >= 6.2 not resetting CPU affinity for container processes.
Signed-off-by: Cédric Clerget <cedric.clerget@gmail.com>
We auto-close this file descriptor in the final exec step, but it's
probably a good idea to not possibly leak the file descriptor to "runc
init" (we've had issues like this in the past) especially since it is a
directory handle from the host mount namespace.
In practice, on runc 1.1 this does leak to "runc init" but on main the
handle has a low enough file descriptor that it gets clobbered by the
ForkExec of "runc init".
OPEN_TREE_CLONE would let us protect this handle even further, but the
performance impact of creating an anonymous mount namespace is probably
not worth it.
Also, switch to using an *os.File for the handle so if it goes out of
scope during setup (i.e. an error occurs during setup) it will get
cleaned up by the GC.
Fixes: GHSA-xr7r-f8xq-vfvv CVE-2024-21626
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
With the idmap work, we will have a tainted Go thread in our
thread-group that has a different mount namespace to the other threads.
It seems that (due to some bad luck) the Go scheduler tends to make this
thread the thread-group leader in our tests, which results in very
baffling failures where /proc/self/mountinfo produces gibberish results.
In order to avoid this, switch to using /proc/thread-self for everything
that is thread-local. This primarily includes switching all file
descriptor paths (CLONE_FS), all of the places that check the current
cgroup (technically we never will run a single runc thread in a separate
cgroup, but better to be safe than sorry), and the aforementioned
mountinfo code. We don't need to do anything for the following because
the results we need aren't thread-local:
* Checks that certain namespaces are supported by stat(2)ing
/proc/self/ns/...
* /proc/self/exe and /proc/self/cmdline are not thread-local.
* While threads can be in different cgroups, we do not do this for the
runc binary (or libcontainer) and thus we do not need to switch to
the thread-local version of /proc/self/cgroups.
* All of the CLONE_NEWUSER files are not thread-local because you
cannot set the usernamespace of a single thread (setns(CLONE_NEWUSER)
is blocked for multi-threaded programs).
Note that we have to use runtime.LockOSThread when we have an open
handle to a tid-specific procfs file that we are operating on multiple
times. Go can reschedule us such that we are running on a different
thread and then kill the original thread (causing -ENOENT or similarly
confusing errors). This is not strictly necessary for most usages of
/proc/thread-self (such as using /proc/thread-self/fd/$n directly) since
only operating on the actual inodes associated with the tid requires
this locking, but because of the pre-3.17 fallback for CentOS, we have
to do this in most cases.
In addition, CentOS's kernel is too old for /proc/thread-self, which
requires us to emulate it -- however in rootfs_linux.go, we are in the
container pid namespace but /proc is the host's procfs. This leads to
the incredibly frustrating situation where there is no way (on pre-4.1
Linux) to figure out which /proc/self/task/... entry refers to the
current tid. We can just use /proc/self in this case.
Yes this is all pretty ugly. I also wish it wasn't necessary.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The current code is only doing retries in RemovePaths, which is only
used for cgroup v1 (cgroup v2 uses RemovePath, which makes no retries).
Let's remove all retry logic and logging from RemovePaths, together
with:
- os.Stat check from RemovePaths (its usage probably made sense before
commit 19be8e5ba5 but not after);
- error/warning logging from RemovePaths (this was added by commit
19be8e5ba5 in 2020 and so far we've seen no errors other
than EBUSY, so reporting the actual error proved to be useless).
Add the retry logic to rmdir, and the second retry bool argument.
Decrease the initial delay and increase the number of retries from the
old implementation so it can take up to ~1 sec before returning EBUSY
(was about 0.3 sec).
Hopefully, as a result, we'll have less "failed to remove cgroup paths"
errors.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
CentOS 7 is showing its age and we'd rather skip some tests on it than
find out why they are flaky.
Add internal/testutil package, and move the generalized version of
SkipOnCentOS7 from libcontainer/cgroups/devices to there.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This prevents potential exploit of using "../" in cgroups.OpenFile
(as well as other methods that use OpenFile) to read or write to
other cgroups.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This field reports swap-only usage. For cgroupv1, `Usage` and `Failcnt`
are set by subtracting memory usage from memory+swap usage. For cgroupv2,
`Usage`, `Limit`, and `MaxUsage` are set. This commit also export `MaxUsage`
of memory under cgroupv2 mode, using `memory.peak` introduced in kernel 5.19.
Signed-off-by: Heran Yang <heran55@126.com>
There is no point in showing the underlying error when path == "",
because it is ENOENT.
Revert the change done in commit e1584831b6.
Fixes: e1584831b6
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit f34eb2c00 introduced a workaround to retry on EINTR due to changes in Go 1.14.
It was fixed in Go 1.15 [1], meaning a custom retry loop is no longer
necessary.
Keep the test case to avoid future regressions.
[1] https://github.com/golang/go/issues/38033
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This adds support for hugetlb.<pagesize>.rsvd limiting and accounting.
The previous non-rsvd max/limit_in_bytes does not account for reserved
huge page memory, making it possible for a processes to reserve all the
huge page memory, without being able to allocate it (due to cgroup
restrictions).
In practice this makes it possible to successfully mmap more huge page
memory than allowed via the cgroup settings, but when using the memory
the process will get a SIGBUS and crash. This is bad for applications
trying to mmap at startup (and it succeeds), but the program crashes
when starting to use the memory. eg. postgres is doing this by default.
This also keeps writing to the old max/limit_in_bytes, for backward
compatibility.
More info can be found here: https://lkml.org/lkml/2020/2/3/1153
(commit message mostly written by Odin Ugedal)
Co-authored-by: Odin Ugedal <odin@ugedal.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Burstable CFS controller is introduced in Linux 5.14. This helps with
parallel workloads that might be bursty. They can get throttled even
when their average utilization is under quota. And they may be latency
sensitive at the same time so that throttling them is undesired.
This feature borrows time now against the future underrun, at the cost
of increased interference against the other system users, by introducing
cfs_burst_us into CFS bandwidth control to enact the cap on unused
bandwidth accumulation, which will then used additionally for burst.
The patch adds the support/control for CFS bandwidth burst.
runtime-spec: https://github.com/opencontainers/runtime-spec/pull/1120
Co-authored-by: Akihiro Suda <suda.kyoto@gmail.com>
Co-authored-by: Nadeshiko Manju <me@manjusaka.me>
Signed-off-by: Kailun Qin <kailun.qin@intel.com>
golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains
the fix [1] whitelisting all errno comparisons for errors coming from
x/sys/unix.
Thus, these annotations are no longer necessary. Hooray!
[1] https://github.com/polyfloyd/go-errorlint/pull/47
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
A few cases relied on the fact that systemd is used, and thus
/sys/fs/cgroup/user.slice is available.
Guess what, in case of "make unittest" it might not be.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Split the test into two -- for fs and systemd cgroup managers, and only
run the second one if systemd is available.
Prevents the following failure during `make unittest`:
> === RUN TestNilResources
> manager_test.go:27: systemd not running on this host, cannot use systemd cgroups manager
> --- FAIL: TestNilResources (0.22s)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This aligns v2 usage calculations more closely with v1.
Current node-level reporting for v1 vs v2 on the same
machine under similar load may differ by ~250-750Mi.
Also return usage as combined swap + memory usage, aligned
with v1 and non-root v2 cgroups.
`mem_cgroup_usage` in the kernel counts NR_FILE_PAGES
+ NR_ANON_MAPPED + `nr_swap_pages` (if swap enabled) [^0].
Using total - free results in higher "usage" numbers.
This is likely due to various types of reclaimable
memory technically counted as in use (e.g. inactive anon).
See also https://github.com/kubernetes/kubernetes/issues/118916 for more context
[^0]: https://github.com/torvalds/linux/blob/06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5/mm/memcontrol.c#L3673-L3680
Signed-off-by: Alexander Eldeib <alexeldeib@gmail.com>
Kir Kolyshkin