Commit Graph

407 Commits

Author SHA1 Message Date
Kir Kolyshkin be00ae07c3 ci: shellcheck: update to 0.8.0, fix/suppress new warnings
1. This valid warning is reported by shellcheck v0.8.0:

	In tests/integration/helpers.bash line 38:
	KERNEL_MINOR="${KERNEL_VERSION#$KERNEL_MAJOR.}"
				       ^-----------^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.

	Did you mean:
	KERNEL_MINOR="${KERNEL_VERSION#"$KERNEL_MAJOR".}"

Fix this.

2. These (invalid) warnings are also reported by the new version:

	In tests/integration/events.bats line 13:
	@test "events --stats" {
	^-- SC2030 (info): Modification of status is local (to subshell caused by @bats test).

	In tests/integration/events.bats line 41:
		[ "$status" -eq 0 ]
		   ^-----^ SC2031 (info): status was modified in a subshell. That change might be lost.

Basically, this is happening because shellcheck do not really track
the call tree and/or local variables. This is a known (and reported)
deficiency, and the alternative to disabling these warnings is moving
the code around, which is worse due to more changes in git history.

So we have to silence/disable these.

3. Update shellcheck to 0.8.0.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-10 17:24:04 -08:00
Kir Kolyshkin 6e1d476aad runc: remove --criu option
This was introduced in an initial commit, back in the day when criu was
a highly experimental thing. Today it's not; most users who need it have
it packaged by their distro vendor.

The usual way to run a binary is to look it up in directories listed in
$PATH. This is flexible enough and allows for multiple scenarios (custom
binaries, extra binaries, etc.). This is the way criu should be run.

Make --criu a hidden option (thus removing it from help). Remove the
option from man pages, integration tests, etc. Remove all traces of
CriuPath from data structures.

Add a warning that --criu is ignored and will be removed.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-01-26 20:25:56 -08:00
Kir Kolyshkin 5d77962099 tests/int: use update_config in hooks test
Using "$@" instead of $1 in update_config() allows us to use it from
hooks.bats, where jq is used with more options than usual.

We need to disable SC2016 as otherwise shellcheck sees $something inside
single quotes and think we are losing the shell expansion (we are not).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-12-08 21:36:27 -08:00
Kir Kolyshkin 9e798e26cb tests/int: ability to specify binary
This can be used to specify a different runc binary, for example:

	sudo -E RUNC=$PWD/runc.mine tests/integration/cwd.bats

A different (but compatible enough) runtime also works:

	sudo -E RUNC=/usr/local/bin/crun tests/integration/cwd.bats

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-12-08 21:25:05 -08:00
Akihiro Suda 382eba4354 Support recursive mount attrs ("rro", "rnosuid", "rnodev", ...)
The new mount option "rro" makes the mount point recursively read-only,
by calling `mount_setattr(2)` with `MOUNT_ATTR_RDONLY` and `AT_RECURSIVE`.
https://man7.org/linux/man-pages/man2/mount_setattr.2.html

Requires kernel >= 5.12.

The "rro" option string conforms to the proposal in util-linux/util-linux Issue 1501.

Fix issue 2823

Similary, this commit also adds the following mount options:
- rrw
- r[no]{suid,dev,exec,relatime,atime,strictatime,diratime,symfollow}

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
2021-12-07 17:39:57 +09:00
Fraser Tweedale 35d20c4e0b chown cgroup to process uid in container namespace
Delegating cgroups to the container enables more complex workloads,
including systemd-based workloads.  The OCI runtime-spec was
recently updated to explicitly admit such delegation, through
specification of cgroup ownership semantics:

  https://github.com/opencontainers/runtime-spec/pull/1123

Pursuant to the updated OCI runtime-spec, change the ownership of
the container's cgroup directory and particular files therein, when
using cgroups v2 and when the cgroupfs is to be mounted read/write.

As a result of this change, systemd workloads can run in isolated
user namespaces on OpenShift when the sandbox's cgroupfs is mounted
read/write.

It might be possible to implement this feature in other cgroup
managers, but that work is deferred.

Signed-off-by: Fraser Tweedale <ftweedal@redhat.com>
2021-11-30 08:52:59 +10:00
Aleksa Sarai 19d696ec29 merge branch 'pr-3276'
Kir Kolyshkin (2):
  runc run: fix ro /dev
  test/int/mount.bats: refer to github issue

LGTMs: thaJeztah cyphar
Closes #3276
2021-11-26 09:37:43 +11:00
Kir Kolyshkin 50105de1d8 Fix failure with rw bind mount of a ro fuse
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>
2021-11-18 13:09:41 -08:00
Kir Kolyshkin b247cd392a runc run: fix ro /dev
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>
2021-11-15 10:37:16 -08:00
Kir Kolyshkin f252eb5436 test/int/mount.bats: refer to github issue
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-12 12:10:46 -08:00
Kir Kolyshkin 68c2b6a7d9 runc run: refuse a frozen cgroup
Sometimes a container cgroup already exists but is frozen.
When this happens, runc init hangs, and it's not clear what is going on.

Refuse to run in a frozen cgroup; add a test case.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-03 20:36:27 -07:00
Kir Kolyshkin d08bc0c1b3 runc run: warn on non-empty cgroup
Currently runc allows multiple containers to share the same cgroup (for
example, by having the same cgroupPath in config.json). While such
shared configuration might be OK, there are some issues:

 - When each container has its own resource limits, the order of
   containers start determines whose limits will be effectively applied.

 - When one of containers is paused, all others are paused, too.

 - When a container is paused, any attempt to do runc create/run/exec
   end up with runc init stuck inside a frozen cgroup.

 - When a systemd cgroup manager is used, this becomes even worse -- such
   as, stop (or even failed start) of any container results in
   "stopTransientUnit" command being sent to systemd, and so (depending on
   unit properties) other containers can receive SIGTERM, be killed after a
   timeout etc.

Any of the above may lead to various hard-to-debug situations in production
(runc init stuck, cgroup removal error, wrong resource limits, init not
reaping zombies etc.).

One obvious solution is to refuse a non-empty cgroup when starting a new
container. This would be a breaking change though, so let's make it in
steps, with the first step is issue a warning and a deprecated notice
about a non-empty cgroup.

Later (in runc 1.2) we will replace this warning with an error.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-03 20:36:27 -07:00
Kir Kolyshkin dd696235a4 runc exec: reject paused container unless --ignore-paused
Currently, if a container is paused (i.e. its cgroup is frozen), runc exec
just hangs, and it is not obvious why.

Refuse to exec in a paused container. Add a test case.

In case runc exec in a paused container is a legit use case,
add --ignore-paused option to override the check. Document it,
add a test case.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-03 20:36:27 -07:00
Kir Kolyshkin 7324496f1a tests/int: fix userns for Fedora 35
Some test directories are created using mktemp -d, and so they have
permissions set to 0700 and are thus inaccessible to a user inside
userns. This was workarounded for $ROOT in userns.bats before.

Now, when we have updated Cirrus CI config to use Fedora 35 (rather than
34), userns tests fail:

> runc run failed: unable to start container process: error during
> container init: error preparing rootfs: mount
> /tmp/bats-run-4pCERd/runc.f66gCC/bundle/rootfs:/tmp/bats-run-4pCERd/runc.f66gCC/bundle/rootfs,
> flags: 0x5000: permission denied

Fedora 34 image used kernel v5.11, while Fedora 35 has v5.15.
Apparently, the newer kernel also checks that the parent directories
are accessible by the user before doing mount.

Move the old workaround from userns.bats to helpers.bats, drop the r bit
(not needed), and add $BATS_RUN_TMPDIR (also created by mktemp -d) to
fix userns.bats test failures under Fedora 35.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-03 19:07:37 -07:00
Kir Kolyshkin 05272718f4 tests/int/cgroups: fix for misc controller
The misc cgroup controller, introduced in Linux 5.13, is still unknown
to systemd, and thus it cannot delegate it. Add an appropriate fixup
to the test case, similar to an earlier commit 601cf5825f.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-03 18:54:16 -07:00
Rodrigo Campos 81fdc8ce1c New integration tests for user namespaces bind sources
The previous commit fixed an issue opening bind mount sources. This
commit just adds integration tests to make sure we don't regress on this
in the future.

Signed-off-by: Alban Crequy <alban@kinvolk.io>
Signed-off-by: Rodrigo Campos <rodrigo@kinvolk.io>
Co-authored-by: Rodrigo Campos <rodrigo@kinvolk.io>
2021-10-12 15:13:45 +02:00
Kir Kolyshkin 0202c398ff runc exec: implement --cgroup
In some setups, multiple cgroups are used inside a container,
and sometime there is a need to execute a process in a particular
sub-cgroup (in case of cgroup v1, for a particular controller).
This is what this commit implements.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-09-27 10:25:42 -07:00
Mauricio Vásquez cc15b887a0 tests: add integration test for cgroups hybrid
Check that runc run and runc exec put the process on the same cgroups v2
when using hybrid mode.

Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-09-27 10:25:12 -07:00
Kir Kolyshkin 6806b2c1c4 runc delete -f: fix for cg v1 + paused container
runc delete -f is not working for a paused container, since in cgroup v1
SIGKILL does nothing if a process is frozen (unlike cgroup v2, in which
you can kill a frozen process with a fatal signal).

Theoretically, we only need this for v1, but doing it for v2 as well is
OK.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-09-15 14:55:14 -07:00
Rodrigo Campos af641cd587 seccomp: Add test using the seccomp agent example
This commit adds the config.json as generated by the script. Note that
the diff is minimal if you see this commit with "git show -w". The
differences are mostly whitespaces and some ordering.

We add a simple test that runs this and expects sucess.

Signed-off-by: Rodrigo Campos <rodrigo@kinvolk.io>
2021-09-10 12:44:43 +02:00
Aleksa Sarai 8bf032602a merge branch 'pr-3047'
Liu Hua (1):
  checkpoint: resolve symlink for external bind mount(fix ci broken)

LGTMs: kolyshkin cyphar
2021-09-09 14:24:26 +10:00
Mauricio Vásquez 00772caec7 tests: add functional tests for seccomp notify
Add functional test to check seccomp notify end-to-end. This test uses the
sample seccomp agent from the contrib/cmd folder.

Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rodrigo Campos <rodrigo@kinvolk.io>
Co-authored-by: Rodrigo Campos <rodrigo@kinvolk.io>
2021-09-07 13:04:24 +02:00
Mauricio Vásquez 5ae831d9b3 tests: add functional tests for seccomp
Test KILL and ERRNO actions.

Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
2021-09-07 13:04:24 +02:00
Sebastiaan van Stijn b144f3d382 Merge pull request #2825 from lifubang/nodelete
proposal: add --keep to runc run
2021-08-31 10:04:58 +02:00
Kir Kolyshkin 639445789d tests/int: add a "update cpu period with pod limit set" test
Add a test case for an issue fixed by the previous commit.

Unfortunately, this is somewhat complicated as there's no easy way to
create a transient unit, so a binary, sd-helper, had to be added. On top
of that, an ability to create a parent/pod cgroup is added to
helpers.bash, which might be useful for future integration tests.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-08-26 13:11:22 -07:00
Kir Kolyshkin 1b2adcfe56 libct/cg/v1: workaround CPU quota period set failure
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>
2021-08-26 13:11:22 -07:00
Kir Kolyshkin fb629db693 tests/int/helpers: fix shellcheck warnings
... and add the file to be checked by shellcheck.

The warnings fixed are:

In tests/integration/helpers.bash line 10:
INTEGRATION_ROOT=$(dirname "$(readlink -f "$BASH_SOURCE")")
                                           ^----------^ SC2128: Expanding an array without an index only gives the first element.

In tests/integration/helpers.bash line 22:
TESTDATA="${INTEGRATION_ROOT}/testdata"
^------^ SC2034: TESTDATA appears unused. Verify use (or export if used externally).

In tests/integration/helpers.bash line 42:
	echo "runc $@ (status=$status):" >&2
                   ^-- SC2145: Argument mixes string and array. Use * or separate argument.
                              ^-----^ SC2154: status is referenced but not assigned.

In tests/integration/helpers.bash line 43:
	echo "$output" >&2
              ^-----^ SC2154: output is referenced but not assigned.

In tests/integration/helpers.bash line 77:
			| .linux.gidMappings += [{"hostID": '"$(($ROOTLESS_GIDMAP_START + 10))"', "containerID": 1, "size": 20}]
                                                                 ^--------------------^ SC2004: $/${} is unnecessary on arithmetic variables.

In tests/integration/helpers.bash line 78:
			| .linux.gidMappings += [{"hostID": '"$(($ROOTLESS_GIDMAP_START + 100))"', "containerID": 1000, "size": '"$(($ROOTLESS_GIDMAP_LENGTH - 1000))"'}]'
                                                                 ^--------------------^ SC2004: $/${} is unnecessary on arithmetic variables.
                                                                                                                                     ^---------------------^ SC2004: $/${} is unnecessary on arithmetic variables.

In tests/integration/helpers.bash line 125:
			base_path=$(gawk '$(NF-2) == "cgroup" && $NF ~ /\<'${g}'\>/ { print $5; exit }' /proc/self/mountinfo)
                                                                           ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
			base_path=$(gawk '$(NF-2) == "cgroup" && $NF ~ /\<'"${g}"'\>/ { print $5; exit }' /proc/self/mountinfo)

In tests/integration/helpers.bash line 127:
			eval CGROUP_${g^^}_BASE_PATH="${base_path}"
                                    ^----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
			eval CGROUP_"${g^^}"_BASE_PATH="${base_path}"

In tests/integration/helpers.bash line 229:
	if [ "x$CGROUP_UNIFIED" = "xyes" ]; then
             ^----------------^ SC2268: Avoid x-prefix in comparisons as it no longer serves a purpose.

Did you mean:
	if [ "$CGROUP_UNIFIED" = "yes" ]; then

In tests/integration/helpers.bash line 234:
		eval cgroup=\$${var}${REL_CGROUPS_PATH}
                              ^----^ SC2086: Double quote to prevent globbing and word splitting.
                                    ^-----------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
		eval cgroup=\$"${var}""${REL_CGROUPS_PATH}"

In tests/integration/helpers.bash line 236:
	cat $cgroup/$source
            ^-----^ SC2086: Double quote to prevent globbing and word splitting.
                    ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	cat "$cgroup"/"$source"

In tests/integration/helpers.bash line 242:
	current="$(get_cgroup_value $1)"
                                    ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	current="$(get_cgroup_value "$1")"

In tests/integration/helpers.bash line 245:
	echo "current" $current "!?" "$expected"
                       ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	echo "current" "$current" "!?" "$expected"

In tests/integration/helpers.bash line 257:
	[ $(id -u) != "0" ] && user="--user"
          ^------^ SC2046: Quote this to prevent word splitting.

In tests/integration/helpers.bash line 259:
	current=$(systemctl show $user --property $source $SD_UNIT_NAME | awk -F= '{print $2}')
                                                  ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	current=$(systemctl show $user --property "$source" $SD_UNIT_NAME | awk -F= '{print $2}')

In tests/integration/helpers.bash line 261:
	[ "$current" = "$expected" ] || [ -n "$expected2" -a "$current" = "$expected2" ]
                                                          ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

In tests/integration/helpers.bash line 309:
	check_cgroup_value "cpu.weight" $weight
                                        ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	check_cgroup_value "cpu.weight" "$weight"

In tests/integration/helpers.bash line 310:
	check_systemd_value "CPUWeight" $weight
                                        ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	check_systemd_value "CPUWeight" "$weight"

In tests/integration/helpers.bash line 383:
			if [ $CGROUP_UNIFIED = "no" -a ! -e "${CGROUP_MEMORY_BASE_PATH}/memory.memsw.limit_in_bytes" ]; then
                                                    ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

In tests/integration/helpers.bash line 412:
			local cpu_count=$(grep -c '^processor' /proc/cpuinfo)
                              ^-------^ SC2155: Declare and assign separately to avoid masking return values.

In tests/integration/helpers.bash line 450:
		sleep $delay
                      ^----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
		sleep "$delay"

In tests/integration/helpers.bash line 453:
	echo "Command \"$@\" failed $attempts times. Output: $output"
                        ^-- SC2145: Argument mixes string and array. Use * or separate argument.

In tests/integration/helpers.bash line 471:
	runc state $1
                   ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	runc state "$1"

In tests/integration/helpers.bash line 472:
	if [ $2 == "checkpointed" ]; then
             ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	if [ "$2" == "checkpointed" ]; then

In tests/integration/helpers.bash line 484:
	mkdir $dir
              ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	mkdir "$dir"

In tests/integration/helpers.bash line 497:
		kill -9 $(cat "$dir/pid")
                        ^---------------^ SC2046: Quote this to prevent word splitting.

In tests/integration/helpers.bash line 508:
	export ROOT=$(mktemp -d "$BATS_RUN_TMPDIR/runc.XXXXXX")
               ^--^ SC2155: Declare and assign separately to avoid masking return values.

In tests/integration/helpers.bash line 512:
	cd "$ROOT/bundle"
        ^---------------^ SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

Did you mean:
	cd "$ROOT/bundle" || exit

In tests/integration/helpers.bash line 535:
	cd "$INTEGRATION_ROOT"
        ^--------------------^ SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

Did you mean:
	cd "$INTEGRATION_ROOT" || exit

For more information:
  https://www.shellcheck.net/wiki/SC2145 -- Argument mixes string and array. ...
  https://www.shellcheck.net/wiki/SC2034 -- TESTDATA appears unused. Verify u...
  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-08-23 15:53:26 -07:00
Kir Kolyshkin f65276db35 tests/int/helpers: rm $bundle handling
It is not used since PR 2757, as all tests are run with cd to bundle
directory.

runc_spec argument count checking is removed since otherwise shellcheck
complains:

> SC2120: runc_spec references arguments, but none are ever passed.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-08-23 15:49:45 -07:00
Liu Hua 74ae9e0fc9 checkpoint: resolve symlink for external bind mount(fix ci broken)
runc resolves symlink before doing bind mount. So
we should save original path while formatting CriuReq for
dump and restore.

"checkpoint: resolve symlink for external bind mount" is merged as
da22625f6986f0ef196eaa1f8bb6adce098f0fb7(PR 2902) previously. And reverted
in commit 70fdc0573dced3464e9c31d674559f77c1de3973(PR 3043) duo to behavior changes
caused by commit 0ca91f44f1664da834bc61115a849b56d22f595f(Fixes: CVE-2021-30465)

Signed-off-by: Liu Hua <weldonliu@tencent.com>
2021-08-19 18:42:42 +08:00
Kir Kolyshkin 75761bccf7 Fix codespell warnings, add codespell to ci
The two exceptions I had to add to codespellrc are:
 - CLOS (used by intelrtd);
 - creat (syscall name used in tests/integration/testdata/seccomp_*.json).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-08-17 16:12:35 -07:00
Mrunal Patel 8772c4dd2f Merge pull request #3109 from kolyshkin/seccomp
seccomp: skip redundant rules
2021-08-03 23:09:00 -04:00
lifubang cb824629ba proposal: add --keep to runc run
Signed-off-by: lifubang <lifubang@acmcoder.com>
2021-08-02 12:51:36 -07:00
Kir Kolyshkin 60e02b4b25 runc exec: fail with exit code of 255
Currently there's no way to distinguish between the two cases:
 - runc exec failed;
 - the command executed returned 1.

This was possible before commit 8477638aab, as runc exec exited with
the code of 255 if exec itself has failed. The code of 255 is the same
convention as used by e.g. ssh.

Re-introduce the feature, document it, and add some tests so it won't be
broken again.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-07-28 13:46:13 -07:00
Kir Kolyshkin 5dd92fd9b4 libct/seccomp: skip redundant rules
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>
2021-07-27 00:04:59 -07:00
Adrian Reber 9f656dbb11 Do not use Vagrant for CentOS 7/8
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>
2021-07-23 09:23:23 +02:00
Kir Kolyshkin d448016486 tests/rootless.sh: fixup for "update rt" test
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>
2021-07-23 09:23:23 +02:00
Kir Kolyshkin 86af524866 tests/int: fix "update rt period and runtime" for rootless
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>
2021-07-23 09:23:23 +02:00
Aleksa Sarai 2c01cec8ac merge branch 'pr-3067'
Odin Ugedal (2):
  libct/cg/sd: Don't freeze cgroup on cgroup v2 Set
  Update device update tests

LGTMs: kolyshkin mrunalp cyphar
Closes #3067
2021-07-13 12:59:29 +10:00
Odin Ugedal d41a273dae Update device update tests
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>
2021-07-07 22:44:08 +02:00
Odin Ugedal 6be088d69d tests/int/dev: add CAP_SYSLOG to /dev/kmsg tests
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>
2021-07-07 15:44:16 +02:00
Akihiro Suda 5547b5774f Merge pull request #3033 from kolyshkin/rm-own-errors
libcontainer: rm own error system
2021-07-01 13:47:27 +09:00
Kir Kolyshkin 1bbeadae72 tests/int/no_pivot: fix for new kernels
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>
2021-06-29 13:31:54 -07:00
Kir Kolyshkin 70fdc0573d Revert "checkpoint: resolve symlink for external bind mount"
This reverts commit da22625f69
(PR 2902).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-06-24 20:52:43 -07:00
Mrunal Patel 1079288bef Merge pull request #2902 from liusdu/checkpoint
checkpoint: resolve symlink for external bind mount
2021-06-24 22:52:02 -04:00
Kir Kolyshkin e918d02139 libcontainer: rm own error system
This removes libcontainer's own error wrapping system, consisting of a
few types and functions, aimed at typization, wrapping and unwrapping
of errors, as well as saving error stack traces.

Since Go 1.13 now provides its own error wrapping mechanism and a few
related functions, it makes sense to switch to it.

While doing that, improve some error messages so that they start
with "error", "unable to", or "can't".

A few things that are worth mentioning:

1. We lose stack traces (which were never shown anyway).

2. Users of libcontainer that relied on particular errors (like
   ContainerNotExists) need to switch to using errors.Is with
   the new errors defined in error.go.

3. encoding/json is unable to unmarshal the built-in error type,
   so we have to introduce initError and wrap the errors into it
   (basically passing the error as a string). This is the same
   as it was before, just a tad simpler (actually the initError
   is a type that got removed in commit afa844311; also suddenly
   ierr variable name makes sense now).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-06-24 10:21:04 -07:00
Kir Kolyshkin 2916817278 tests/int/cgroups: add test for bfq per-device weight
This works for both cgroup v1 and v2.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-06-16 04:51:40 -07:00
Mrunal Patel a3813ca249 Merge pull request #3000 from kolyshkin/test-dev-update
libct/int: add device update test
2021-06-11 14:36:49 -04:00
Kir Kolyshkin bd8e070109 libct/cg/sd: fix "SkipDevices" handling
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>
2021-06-10 12:01:21 -07:00
Kir Kolyshkin 508f5bf665 libct/int: add device update test
... 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>
2021-06-05 16:11:29 -07:00
Kir Kolyshkin f5a2c9cced tests/int/dev: only call lsblk once
The "runc run [device cgroup allow rm block device]" test calls lsblk
three times to get device name, minor and major number. This creates a
potential problem when the devices are changed between the calls.

Simplify the code by using bash read together with IFS (as there's no
way to have lsblk output MAJOR:MINOR pair without a semicolon).

Note that head -n 1 is not needed as read already reads a single line.

[v2: don't use PATH as CentOS7's lsblk does not support it.]

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-05-25 12:21:42 -07:00