The ParentImage set by the test should be a path relative to
ImagesDirectory, pointing to a parent images directory (created
by pre-dump).
The parent directory is created by TempDir and so its name is not
constant but has a variable suffix. So, the config was pointing to
a non-existent directory.
This left unnoticed by criu as it assumed the parent image does not
exist, and performed a full dump.
Since criu PR 1403 (will be a part of criu 3.16) that is no longer the
case -- the invalid parent path is treated as an error, and so our
test fails like this:
== RUN TestCheckpoint
checkpoint_test.go:145: === /tmp/criu070876105/dump.log ===
checkpoint_test.go:145: open /tmp/criu070876105/dump.log: no such file or directory
checkpoint_test.go:146: criu failed: type DUMP errno 56
log file: /tmp/criu070876105/dump.log
--- FAIL: TestCheckpoint (0.26s)
Fix this by using the actual name of the parent image dir.
Fixes: 98f004182b
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is helpful to kubernetes in cases it knows for sure that the freeze
is not required (since it created the systemd unit with no device
restrictions).
As the code is trivial, no tests are required.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 9a095e44db)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Add a test for freezeBeforeSet, checking various scenarios including
those that were failing before the fix in the previous commit.
[v2: add more cases, add a check before creating a unit.]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit fec49f2a6c)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes the behavior intended to avoid freezing containers/control
groups without it being necessary. This is important for end users of
libcontainer who rely on the behavior of no freeze.
The previous implementation would always get error trying to get
DevicePolicy from the Unit via dbus, since the Unit interface doesn't
contain DevicePolicy.
Signed-off-by: Odin Ugedal <odin@uged.al>
(cherry picked from commit 41043673b7)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 2bab4a5 resulted in a warning from gcc:
nsexec.c: In function ‘write_log’:
nsexec.c:171:2: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
171 | write(logfd, json, ret);
| ^~~~~~~~~~~~~~~~~~~~~~~
As there's nothing we can or want to do in case write fails,
let's just tell the compiler we're not going to use it.
Fixes: 2bab4a5
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit db8330c9e5)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As reported in issue 3119, there is a race in nsexec logging
that can lead to garbled json received by log forwarder, which
complains about it with a "failed to decode" error.
This happens because dprintf (used since the very beginning of nsexec
logging introduced in commit ba3cabf932) relies on multiple write(2)
calls, and with additional logging added by 64bb59f592 a race is
possible between runc init parent and its children.
The fix is to prepare a string and write it using a single call to
write(2).
[v2: NULLify json on error from asprintf]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 2bab4a56f1)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes using runc with podman on my system (Fedora 34).
> $ podman --runtime `pwd`/runc run --rm --memory 4M fedora echo it works
> Error: unable to start container process: error adding seccomp filter rule for syscall bdflush: permission denied: OCI permission denied
The problem is, libseccomp returns EPERM when a redundant rule (i.e. the
rule with the same action as the default one) is added, and podman (on
my machine) sets the following rules in config.json:
<....>
"seccomp": {
"defaultAction": "SCMP_ACT_ERRNO",
"architectures": [
"SCMP_ARCH_X86_64",
"SCMP_ARCH_X86",
"SCMP_ARCH_X32"
],
"syscalls": [
{
"names": [
"bdflush",
"io_pgetevents",
<....>
],
"action": "SCMP_ACT_ERRNO",
"errnoRet": 1
},
<....>
(Note that defaultErrnoRet is not set, but it defaults to 1).
With this commit, it works:
> $ podman --runtime `pwd`/runc run --memory 4M fedora echo it works
> it works
Add an integration test (that fails without the fix).
Similar crun commit:
* https://github.com/containers/crun/commit/08229f3fb904c5ea19a7d9
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Cherry picked from commit 5dd92fd9b4.
Minor conflict in libcontainer/seccomp/seccomp_linux.go due to
missing commit e44bee1026.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As reported in issue 3084, sometimes setting CPU quota period fails
when a new period is lower and a parent cgroup has CPU quota limit set.
This happens as in cgroup v1 the quota and the period can not be set
together (this is fixed in v2), and since the period is being set first,
new_limit = old_quota/new_period may be higher than the parent cgroup
limit.
The fix is to retry setting the period after the quota, to cover all
possible scenarios.
Add a test case to cover a regression caused by an earlier version of
this patch (ignoring a failure of setting invalid period when quota is
not set).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 1b77ebe2c4fa0c6d576dc587aa69e05f6bafd898)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
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>
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>
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>
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>
Commits 1f1e91b1a0 and 2192670a24
added validation for mountpoints to be an absolute path, to match the OCI
specs.
Unfortunately, the old behavior (accepting the path to be a relative path)
has been around for a long time, and although "not according to the spec",
various higher level runtimes rely on this behavior.
While higher level runtime have been updated to address this requirement,
there will be a transition period before all runtimes are updated to carry
these fixes.
This patch relaxes the validation, to generate a WARNING instead of failing,
allowing runtimes to update (but allowing them to update runc to the current
version, which includes security fixes).
We can remove this exception in a future patch release.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It seems that we are triggering the mutli-attach fallback in the fedora
CI, but we don't have enough debugging information to really know what's
going on, so add some. Unfortunately the amount of information we have
available with eBPF programs in general is fairly limited (we can't get
their bytecode for instance).
We also demote the "more than one filter" warning to an info message
because it happens very often under the systemd cgroup driver (likely
when systemd configures the cgroup it isn't deleting our old program, so
when our apply code runs after the systemd one there are two running
programs).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
It turns out that the cilium eBPF library doesn't degrade gracefully if
BPF_F_REPLACE is not supported, so we need to work around it by treating
that case as we treat the more-than-one program case.
It also turns out that we weren't passing BPF_F_REPLACE explicitly, but
this is required by the cilium library (causing EINVALs).
Fixes: d0f2c25f52 ("cgroup2: devices: replace all existing filters when attaching")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Noticed that the check of trying to use both rootful and rootless
in NewDbusConnManager never worked, as we never set dbusInited to true.
Do that. While at it, protect this with the mutex (against the
case of two goroutines simultaneously calling NewDbusConnManager).
This is a rare call, so taking read-only then read-write mutex does not
make sense.
Fixes: c7f847ed3a
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Restoring an SELinux enabled container with Podman will result in
a container with the exactly same SELinux process labels as during
checkpointing. CRIU takes care of all the process labels.
Restoring multiple copies of a checkpointed container will result in all
containers having the same SELinux process labels, which might be
undesired.
When looking at Pods all container in a Pod share the process label
of the infrastructure container. To restore a container into and
existing Pod it is necessary to tell CRIU to restore the container
with the infrastructure container process label.
CRIU supports setting different process labels using --lsm-profile for a
long time and this just passes the process label information from runc
to CRIU.
Unfortunately CRIU has a bug as no one was using the --lsm-profile
option so this changes requires the upcoming CRIU version 3.16.
Signed-off-by: Adrian Reber <areber@redhat.com>
... and remove the one from tests/integration.
The idea is similar to the one for the test case being removed -- try
updating device rules many times to make sure we are not leaking eBPF
programs after every update/Set(). This is better though as we can
really change the device rules every time (which "runc update" can't)
and check that the rule is applied.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin