TestPodSkipDevicesUpdate checks that updating a pod having SkipDevices: true
does not result in spurious "permission denied" errors in a container
running under the pod. The test is somewhat similar in nature to the
@test "update devices [minimal transition rules]" in tests/integration,
but uses a pod.
This tests the validity of freezeBeforeSet in v1.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit a71102624d)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This was initially added by commit 3e5c199708 because Set (with
r.Freezer = Frozen) was not able to freeze a container.
Now (see a few previous commits) Set can do the freeze, so the explicit
Freeze is no longer needed.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 52dd96db6b)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Introduce freezeBeforeSet, which contains the logic of figuring out
whether we need to freeze/thaw around setting systemd unit properties.
In particular, if SkipDevices is set, and the current unit properties
allow all devices, there is no need to freeze and thaw, as systemd
won't write any device rules in this case.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit f2db87986c,
minor conflict in include() due to missing commit
b60e2edf75)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In addition to freezing and thawing a container via Pause/Resume,
there is a way to also do so via Set.
This way was broken though and is being fixed by a few preceding
commits. The test is added to make sure this is fixed and won't regress.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 5dc3260431)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The t.Name() usage in libcontainer/integration prevented subtests
to be used, since in such case it returns a string containing "/",
and thus it can't be used to name a container.
Fix this by replacing slashes with underscores where appropriate.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit af1688a544)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
m.Freeze method changes m.cgroups.Resources.Freezer field, which should
not be done while we're temporarily freezing the cgroup in Set. If this
field is changed, and r == m.cgroups.Resources (as it often happens),
this results in inability to freeze the container using Set().
To fix, add and use a method which does not change r.Freezer field.
A test case for the bug will be added separately.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 67cfd3d400)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since device updates in cgroup v2 are atomic for systemd, there is no
need to freeze the processes before running the updates.
Signed-off-by: Odin Ugedal <odin@uged.al>
(cherry picked from commit f33be7cc98, trivial conflict
due to missing commit b60e2edf75)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Run device update tests on cgroup v2, and add a test verifying that we
don't allow access to devices when we don't intend to.
Signed-off-by: Odin Ugedal <odin@uged.al>
(cherry picked from commit d41a273dae)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is necessary in order for runc to be able to configure device
cgroups with --systemd-cgroup on distributions that have very strict
SELinux policies such as openSUSE MicroOS[1].
The core issue here is that systemd is adding its own BPF policy that
has an SELinux label such that runc cannot interact with it. In order to
work around this, we can just ignore the policy -- in theory this
behaviour is not correct but given that the most obvious case
(--systemd-cgroup) will still handle updates correctly, this logic is
reasonable.
[1]: https://bugzilla.suse.com/show_bug.cgi?id=1182428
Fixes: d0f2c25f52 ("cgroup2: devices: replace all existing filters when attaching")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit 57e3c54182)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
We need to update the eBPF library so that we can get the raw syscall
errors from bpf(2) syscalls using errors.Is.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit fe518a0678)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
If a control group is frozen, all its descendants will report FROZEN
in freezer.state cgroup file.
OTOH cgroup v2 cgroup.freeze is not reporting the cgroup as frozen
unless it is frozen directly (i.e. not via an ancestor).
Fix the discrepancy between v1 and v2 drivers behavior by
looking into freezer.self_freezing cgroup file, which, according
to kernel documentation, will show 1 iff the cgroup was frozen directly.
Co-authored-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Odin Ugedal <odin@uged.al>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 294c4866ea)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Lines in /etc/group longer than 64 characters breaks the current
implementation of group parser. This is caused by bufio.Scanner
buffer limit.
Fix by re-using the fix for a similar problem in golang os/user,
namely https://go-review.googlesource.com/c/go/+/283601.
Add some tests.
Co-authored-by: Andrey Bokhanko <andreybokhanko@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 24d5daf54d)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Same as in other places (other parsers here, as well as golang os/user
parser and glibc parser all tolerate extra space at BOL and EOL).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 226dfab0bc)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Every []byte to string conversion results in a new allocation.
Avoid some by using []byte more.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 120e3a77d8)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When running a script from an azure file share interrupted syscall
occurs quite frequently, to remedy this add retries around execve
syscall, when EINTR is returned.
Signed-off-by: Maksim An <maksiman@microsoft.com>
(cherry picked from commit e39ad65059)
[Minor conflict in libcontainer/standard_init_linux.go due to missing
commit e918d02139 -- resolved manually]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The test is failing like this:
not ok 70 runc run --no-pivot must not expose bare /proc
# (in test file tests/integration/no_pivot.bats, line 20)
# `[[ "$output" == *"mount: permission denied"* ]]' failed
# runc spec (status=0):
#
# runc run --no-pivot test_no_pivot (status=1):
# unshare: write error: Operation not permitted
Apparently, a recent kernel commit db2e718a47984b9d prevents
root from doing unshare -r unless it has CAP_SETFPCAP.
Add the capability for this specific test.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 1bbeadae72)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin (3):
libct/cg/fs/blkio: do not set weight == 0
libct/cg/fs2: set per-device io weight if available
tests/int/cgroups: add test for bfq per-device weight
LGTMs: AkihiroSuda mrunalp cyphar
Closes#3022
Per-device weight is supported since kernel v5.4 (kernel commit
795fe54c2a8), so let's set those if supplied.
[v2: implement a more relaxed check in bfqDeviceWeightSupported]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For per-device weight, you can set weight and/or leaf weight.
The problem is, with the recent fix to use BFQ on cgroup v1,
if per-device weights are set, the code tries to set device
weight to blkio.bfq.weight, and the leaf weight to
blkio.leaf_weight_device. The latter file does not exist on
kernels v5.0, meaning one can not set any per-device weights
at all.
The fix is to only set weights if they are non-zero (i.e. set).
The test case will come in a following commit.
Fixes: 6339d8a0dd
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is a better place as cgroups itself is using these.
Should help with moving more stuff common in between fs and fs2 to
fscommon.
Looks big, but this is just moving the code around:
fscommon/{fscommon,open}.go -> cgroups/file.go
fscommon/fscommon_test.go -> cgroups/file_test.go
and fixes for TestMode moved to a different package.
There's no functional change.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The `errors.Is(err, unix.EINVAL)` check in `haveBpfProgReplace()` was
broken because the `cilium/ebpf` library did not "wrap" errors.
https://github.com/cilium/ebpf/blob/v0.6.0/link/program.go#L72
So the eBPF support of runc was broken for kernel prior to 5.6.
This commit bumps up cilium/ebpf to contain cilium/ebpf PR 320.
Fix opencontainers/runc issue 3008
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
- Update the blkio cgroup to support the BFQ I/O Scheduler, that has
replaced CFQ in the Linux kernel.
- BFQ is controlled through blkio.bfq.weight[_device] instead of
CFQ's blkio.weight[_device] in cgroups v1.
- BFQ does not support blkio.leaf_weight[_device], so that behavior
remains untouched.
- Do not change behavior on legacy CFQ systems.
- Enable using blkio weights on BFQ systems.
Signed-off-by: Antti Kervinen <antti.kervinen@intel.com>
1. The meaning of SkipDevices is what it is -- do not set any
device-related options.
2. Reverts the part of commit 108ee85b82 which skipped the freeze
when the SkipDevices is set. Apparently, the freeze is needed on
update even if no Device* properties are being set.
3. Add "runc update" to "runc run [device cgroup deny]" test.
Fixes: 752e7a8249
Fixes: 108ee85b82
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sebastiaan van Stijn (2):
libcontainer: relax validation for absolute paths
configs/validator: move cgroup validation to the list of checks
LGTMs: kolyshkin cyphar
Closes#3004
Aleksa Sarai