When writing netlink messages, it is possible to have a byte array
larger than UINT16_MAX which would result in the length field
overflowing and allowing user-controlled data to be parsed as control
characters (such as creating custom mount points, changing which set of
namespaces to allow, and so on).
Co-authored-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Add Go 1.17, grop 1.15 from CI; since 1.17 release 1.15 is unsupported.
Keep 1.13 in 1.0 branch, since an older version of Docker/Moby might use
it.
Keep 1.16 in Dockerfile, since this is a stable branch and we'd rather
not swap horses in the middle of the stream.
This corresponds to commit a587180136 in main branch.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Currently, we can create subcgroup in a rootless container with systemd cgroupv2 on centos8.
But after the container exited, the container cgroup and its subcgroup will not be removed.
Fix this by removing all directories recursively.
Fixes: https://github.com/opencontainers/runc/issues/3225
Signed-off-by: Kang Chen <kongchen28@gmail.com>
[kolyshkin: cherry picked from commit 7758d3fb02,
changing the code to use cgroups.RemovePath().]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
openSUSE comes with site-config package, which makes configure select
${prefix}/lib64 as libdir on x86_64, unless explicitly specified.
Since release.sh relies on a particular libdir path (for pkgconfig), it
breaks things:
> + make -C /home/kir/git/runc PKG_CONFIG_PATH=/tmp/tmp.QgIJ1sR5c9/lib/pkgconfig COMMIT_NO= EXTRA_FLAGS=-a 'EXTRA_LDFLAGS=-w -s -buildid=' static
> make[1]: Entering directory '/home/kir/git/runc'
> CGO_ENABLED=1 go build -trimpath -a -tags "seccomp netgo osusergo" -ldflags "-extldflags -static -X main.gitCommit=v1.0.0-204-g963e0146 -X main.version=1.0.0+dev -w -s -buildid=" -o runc .
> Package libseccomp was not found in the pkg-config search path.
> Perhaps you should add the directory containing `libseccomp.pc'
To fix, we have to explicitly specify libdir.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 39d0ee18e9)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case hugetlb is not supported, GetStats() should not error out,
and yet it does.
Assume that if GetHugePageSize return an error, hugetlb is
not supported (this is what cgroup v1 manager do).
Fixes: 89a87adb
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 916c6a1539)
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
As reported in [1], in a case where read-only fuse (sshfs) mount
is used as a volume without specifying ro flag, the kernel fails
to remount it (when adding various flags such as nosuid and nodev),
returning EPERM.
Here's the relevant strace line:
> [pid 333966] mount("/tmp/bats-run-PRVfWc/runc.RbNv8g/bundle/mnt", "/proc/self/fd/7", 0xc0001e9164, MS_NOSUID|MS_NODEV|MS_REMOUNT|MS_BIND|MS_REC, NULL) = -1 EPERM (Operation not permitted)
I was not able to reproduce it with other read-only mounts as the source
(tried tmpfs, read-only bind mount, and an ext2 mount), so somehow this
might be specific to fuse.
The fix is to check whether the source has RDONLY flag, and retry the
remount with this flag added.
A test case (which was kind of hard to write) is added, and it fails
without the fix. Note that rootless user need to be able to ssh to
rootless@localhost in order to sshfs to work -- amend setup scripts
to make it work, and skip the test if the setup is not working.
[1] https://github.com/containers/podman/issues/12205
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 50105de1d8)
Conflicts:
- .cirrus.yml: trivial, due to missing commit f0dbefac.
- .github/workflows/test.yml: due to missing commits 120f74060 and
3fd1851ce9, resolved manually.
- Dockerfile: trivial, due to missing commit 24d318b8bb.
- libcontainer/rootfs_linux.go: due to missing commits 36aefad45d
and 9c444070ec, resolved manually.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit fb4c27c4b7 (went into v1.0.0-rc93) fixed a bug with
read-only tmpfs, but introduced a bug with read-only /dev.
This happens because /dev is a tmpfs mount and is therefore remounted
read-only a bit earlier than before.
To fix,
1. Revert the part of the above commit which remounts all tmpfs mounts
as read-only in mountToRootfs.
2. Reuse finalizeRootfs (which is already used to remount /dev
read-only) to also remount all ro tmpfs mounts that were previously
mounted rw in mountPropagate.
3. Remove the break in finalizeRootfs, as now we have more than one
mount to care about.
4. Reorder the if statements in finalizeRootfs to perform the fast check
(for ro flag) first, and compare the strings second. Since /dev is
most probably also a tmpfs mount, do the m.Device check first.
Add a test case to validate the fix and prevent future regressions;
make sure it fails before the fix:
✗ runc run [ro /dev mount]
(in test file tests/integration/mounts.bats, line 45)
`[ "$status" -eq 0 ]' failed
runc spec (status=0):
runc run test_busybox (status=1):
time="2021-11-12T12:19:48-08:00" level=error msg="runc run failed: unable to start container process: error during container init: error mounting \"devpts\" to rootfs at \"/dev/pts\": mkdir /tmp/bats-run-VJXQk7/runc.0Fj70w/bundle/rootfs/dev/pts: read-only file system"
Fixes: fb4c27c4b7
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit b247cd392a)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Add CAP_SYSLOG to ensure that /dev/kmsg can be accesses on systems where
the sysctl kernel.dmesg_restrict = 1.
Signed-off-by: Odin Ugedal <odin@uged.al>
(cherry picked from commit 6be088d69d)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
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>
As Cirrus CI does not provide a real terminal this uses the same
'ssh -tt' workaround as the Vagrant setup. This sets up the
CentOS 7 and 8 to allow SSH as root to localhost so that we can run
all the tests via 'ssh -tt'.
Not going through vagrant reduces CI times for CentOS 7 and 8 from 6
minutes to 4 minutes.
Signed-off-by: Adrian Reber <areber@redhat.com>
(cherry picked from commit 9f656dbb11)
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Without this, the test case fails with
> Writing 1000000 to /sys/fs/cgroup/cpu,cpuacct/runc-cgroups-integration-test/cpu.rt_period_us
> /tmp/bats-run-106836/bats.116418.src: line 548: /sys/fs/cgroup/cpu,cpuacct/runc-cgroups-integration-test/cpu.rt_period_us: Permission denied
Since we do not currently have a setup to test this, this went
unnoticed (can be seen in RHEL8 though).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
(cherry picked from commit d448016486)
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Since commit f09a3e1b8d, the value passed on to read starts with
a slash, resulting in the first element of the array to be empty.
As a result, the test tries to write to the top-level cgroup, which
fails when rootless:
> # Writing 1000000 to /sys/fs/cgroup/cpu,cpuacct//cpu.rt_period_us
> # /tmp/bats-run-106184/bats.115768.src: line 548: /sys/fs/cgroup/cpu,cpuacct//cpu.rt_period_us: Permission denied
To fix, remove the leading slash.
An alternative fix would be to do "for ((i = 1;" instead of "i = 0", but
that seems less readable.
Fixes: f09a3e1b8d
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 86af524866)
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
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>
Aleksa Sarai