Commit Graph

16 Commits

Author SHA1 Message Date
Kir Kolyshkin d84388ae10 libct/cg/sd: set the DeviceAllow property before DevicePolicy
Every unit created by runc need daemon reload since systemd v230.
This breaks support for NVIDIA GPUs, see
https://github.com/opencontainers/runc/issues/3708#issuecomment-2216967210

A workaround is to set DeviceAllow before DevicePolicy.

Also:
 - add a test case (which fails before the fix) by @kolyshkin
 - better explain why we need empty DeviceAllow (by @cyphar)

Fixes 4568.

Reported-by: Jian Wen <wenjianhn@gmail.com>
Co-authored-by: Jian Wen <wenjianhn@gmail.com>
Co-authored-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2025-02-05 12:16:43 -08:00
Kir Kolyshkin e965e10c32 tests/int: do not set inheritable capabilities
Amends commit 98fe566c52 ("runc: do not set inheritable
capabilities").

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-09-15 11:49:32 -07:00
Kir Kolyshkin 58b1374f0a Fix failed exec after systemctl daemon-reload
A regression reported for runc v1.1.3 says that "runc exec -t" fails
after doing "systemctl daemon-reload":

> exec failed: unable to start container process: open /dev/pts/0: operation not permitted: unknown

Apparently, with commit 7219387eb7 we are no longer adding
"DeviceAllow=char-pts rwm" rule (because os.Stat("char-pts") returns
ENOENT).

The bug can only be seen after "systemctl daemon-reload" because runc
also applies the same rules manually (by writing to devices.allow for
cgroup v1), and apparently reloading systemd leads to re-applying the
rules that systemd has (thus removing the char-pts access).

The fix is to do os.Stat only for "/dev" paths.

Also, emit a warning that the path was skipped. Since the original idea
was to emit less warnings, demote the level to debug.

Note this also fixes the issue of not adding "m" permission for block-*
and char-* devices.

A test case is added, which reliably fails before the fix
on both cgroup v1 and v2.

Fixes: https://github.com/opencontainers/runc/issues/3551
Fixes: 7219387eb7
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-08-18 14:41:16 -07:00
Kir Kolyshkin d620a401d7 tests/int: remove $ROOTLESS, use $EUID
The variable $ROOTLESS, as set by helpers.bash and used in many places,
provides the same value as $EUID which is always set by bash. Since we
are using bash, we can rely on $EUID being omnipresent.

Modify all uses accordingly, and since the value is known to be a
number, omit the quoting.

Similarly, replace all uses of $(id -u) to $EUID.

Do some trivial cleanups along the way, such as
 - simplify some if A; then B; to A && B;
 - do not use [[ instead of [ where not necessary.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-23 11:12:44 -07: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
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 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
Kir Kolyshkin 65116710dd Merge pull request #2757 from kolyshkin/test-int-cleanups
tests/int: cleanups
2021-02-23 20:50:17 -08:00
Kir Kolyshkin 41670e21f0 tests/int: rework/simplify setup and teardown
1. Get rid of fixed ROOT, *_BUNDLE, and CONSOLE_SOCKET dirs.
   Now they are temporary directories created in setup_bundle.

2. Automate containers cleanup: instead of having to specify all
   containers to be removed, list and destroy everything (which is
   now possible since every test case has its own unique root).

3. Randomize cgroup paths so two tests running in parallel won't
   use the same cgroup.

Now it's theoretically possible to execute tests in parallel.
Practically it's not possible yet because bats uses GNU parallel,
which do not provide a terminal for whatever it executes, and
many runc tests (all those that run containers with terminal:
true) needs a tty. This may possibly be addressed later.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-02-09 20:05:54 -08:00
Vasiliy Ulyanov efb8552b05 tests/int: add device access test
The test verifies if the device file can be queried using
'access(dev_name, F_OK)' when the permissions are set to 'rw'. The call
does not explicitly trigger read or write access but should succeed.

Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>
2021-02-08 14:14:14 +01:00
Kir Kolyshkin ecfad5a118 Merge pull request #2574 from EduardoVega/2249-add-integration-tests-resource-devices
Add integration tests for cgroup devices
2020-09-30 00:19:27 -07:00
Kir Kolyshkin 908b7076e9 tests/int/*.bats: make sure to delete containers
With some grep|awk help, found a few places where the containers
supposed to be removed in teardown() weren't.

To find container names:

	grep -E '^[[:space:]]*\<_*_*runc\>.*\<(create|run|restore)\>'

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-21 02:47:23 -07:00
Eduardo Vega ce24d60331 Add integration tests for cgroup devices
Signed-off-by: Eduardo Vega <edvegavalerio@gmail.com>
2020-09-09 16:49:57 -06:00
Kir Kolyshkin dd3e0da337 tests/int/dev.bats: fixes for new busybox
These tests expect group name to be "nogroup", while recent busybox
changed that to "nobody".

Use numeric uids/gids to fix.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-08 18:53:08 -07:00
Cesar Talledo 9a699e1a9f Skip redundant setup for /dev/ptmx when specified explicitly in the OCI spec.
Per the OCI spec, /dev/ptmx is always a symlink to /dev/pts/ptmx. As such, if
the OCI spec has an explicit entry for /dev/ptmx, runc shall ignore it.

This change ensures this is the case. A integration test was also added
(in tests/integration/dev.bats).

Signed-off-by: Cesar Talledo <ctalledo@nestybox.com>
2020-08-07 16:46:26 -07:00
Cesar Talledo 0709202da7 Remove runc default devices that overlap with spec devices.
Runc has a set of default devices that it includes in Linux containers
(e.g., /dev/null, /dev/random, /dev/tty, etc.)

However if the container's OCI spec includes all or a subset of those same devices,
runc is currently not detecting the redundancy, causing it to create a lib
container config that has redundant device configurations.

This causes a failure in rootless mode, in particular when the /dev/tty device
has a redundant config:

container_linux.go:370: starting container process caused: process_linux.go:459: container init caused: rootfs_linux.go:70: creating device nodes caused: open /tmp/busyboxtest/rootfs/dev/tty: no such device or address"

The reason this fails in rootless mode only is that in this case runc sets up
/dev/tty not by doing mknod (it's not allowed within a user-ns) but rather by
creating a regular file under /dev/tty and bind-mounting the host's /dev/tty to
the container's /dev/tty. When this operation is done redundantly, it fails the
second time.

This change fixes this problem by ensuring runc checks for redundant devices
between the OCI spec it receives and the default devices it configures. If
a redundant device is detected, the OCI spec takes priority.

The change adds both a unit test and an integration test to verify the
behavior. Without this fix, this new integration test fails as shown above.

Signed-off-by: Cesar Talledo <ctalledo@nestybox.com>
2020-08-07 16:46:15 -07:00