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>
These were deprecated and moved; the stubs were included in the
last two (rc94, rc95) releases, so external consumers would have
the chance to update their code.
Removing this so that this doesn't get into v1.0.0 GA
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The runc update CLI is not able to modify devices, so let's set SkipDevices
(so that a cgroup controller won't try to update devices cgroup).
This helps use cases when some other device management (NVIDIA GPUs)
applies its configuration on top of what runc does.
Make sure we do not save SkipDevices into state.json.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
gofumpt (mvdan.cc/gofumpt) is a fork of gofmt with stricter rules.
Brought to you by
git ls-files \*.go | grep -v ^vendor/ | xargs gofumpt -s -w
Looking at the diff, all these changes make sense.
Also, replace gofmt with gofumpt in golangci.yml.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is somewhat radical approach to deal with kernel memory.
Per-cgroup kernel memory limiting was always problematic. A few
examples:
- older kernels had bugs and were even oopsing sometimes (best example
is RHEL7 kernel);
- kernel is unable to reclaim the kernel memory so once the limit is
hit a cgroup is toasted;
- some kernel memory allocations don't allow failing.
In addition to that,
- users don't have a clue about how to set kernel memory limits
(as the concept is much more complicated than e.g. [user] memory);
- different kernels might have different kernel memory usage,
which is sort of unexpected;
- cgroup v2 do not have a [dedicated] kmem limit knob, and thus
runc silently ignores kernel memory limits for v2;
- kernel v5.4 made cgroup v1 kmem.limit obsoleted (see
https://github.com/torvalds/linux/commit/0158115f702b).
In view of all this, and as the runtime-spec lists memory.kernel
and memory.kernelTCP as OPTIONAL, let's ignore kernel memory
limits (for cgroup v1, same as we're already doing for v2).
This should result in less bugs and better user experience.
The only bad side effect from it might be that stat can show kernel
memory usage as 0 (since the accounting is not enabled).
[v2: add a warning in specconv that limits are ignored]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit ccdd75760c introduced the HookName type
for hooks, but only set this type on the Prestart const, but not for the
other hooks.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Add some minimal validation for cgroups. The following checks
are implemented:
- cgroup name and/or prefix (or path) is set;
- for cgroup v1, unified resources are not set;
- for cgroup v2, if memorySwap is set, memory is also set,
and memorySwap > memory.
This makes some invalid configurations fail earlier (before runc init
is started), which is better.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As reported by go test -race ./libcontainer/configs:
=== RUN TestCommandHookRunTimeout
==================
WARNING: DATA RACE
Read at 0x00c000202230 by goroutine 23:
os/exec.(*Cmd).Wait()
/usr/lib/golang/src/os/exec/exec.go:502 +0x91
github.com/opencontainers/runc/libcontainer/configs.Command.Run()
/home/kir/go/src/github.com/opencontainers/runc/libcontainer/configs/config.go:390 +0x58c
github.com/opencontainers/runc/libcontainer/configs_test.TestCommandHookRunTimeout()
/home/kir/go/src/github.com/opencontainers/runc/libcontainer/configs/config_test.go:223 +0x3ed
testing.tRunner()
/usr/lib/golang/src/testing/testing.go:1123 +0x202
Previous write at 0x00c000202230 by goroutine 27:
os/exec.(*Cmd).Wait()
/usr/lib/golang/src/os/exec/exec.go:505 +0xb4
github.com/opencontainers/runc/libcontainer/configs.Command.Run.func1()
/home/kir/go/src/github.com/opencontainers/runc/libcontainer/configs/config.go:373 +0x55
Goroutine 23 (running) created at:
testing.(*T).Run()
/usr/lib/golang/src/testing/testing.go:1168 +0x5bb
testing.runTests.func1()
/usr/lib/golang/src/testing/testing.go:1439 +0xa6
testing.tRunner()
/usr/lib/golang/src/testing/testing.go:1123 +0x202
testing.runTests()
/usr/lib/golang/src/testing/testing.go:1437 +0x612
testing.(*M).Run()
/usr/lib/golang/src/testing/testing.go:1345 +0x3b3
main.main()
_testmain.go:69 +0x236
Goroutine 27 (running) created at:
github.com/opencontainers/runc/libcontainer/configs.Command.Run()
/home/kir/go/src/github.com/opencontainers/runc/libcontainer/configs/config.go:372 +0x415
github.com/opencontainers/runc/libcontainer/configs_test.TestCommandHookRunTimeout()
/home/kir/go/src/github.com/opencontainers/runc/libcontainer/configs/config_test.go:223 +0x3ed
testing.tRunner()
/usr/lib/golang/src/testing/testing.go:1123 +0x202
==================
testing.go:1038: race detected during execution of test
--- FAIL: TestCommandHookRunTimeout (0.10s)
Apparently, the issue is we call two Wait()s for the same command
which can race internally.
Fix is easy -- since we already have a waiting goroutine,
wait for it to return instead of calling a second Wait().
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Test that CommandHook actually executes a new process with the given env
variables, parameters and json state.
This commit also solves an issue with the previous approach that was calling
'os.Exit(0)' failing to signal test failures.
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
In case many net.* sysctls are provided, and we're not running
in the host netns, the function keep repeating isNetNS check
for every such sysctl. This is a waste of resources.
Do the isNetNS check only once, and only if needed.
Note that using sync.Once() is not really needed here; we could
have used a boolean variable to skip the repeated check, but
it looks more idiomatic that way.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case nsfs mount (such as /run/docker/netns/xxxx) is provided as
the netns path, the current way of determining whether path is of
host netns or not is not working.
The proper way to check is to do stat(2) and compare dev_t and
inode fields, which is what this commit does.
This is a minimal fix which does not try to optimize repeated
check in case more than one net.* sysctl is given and there is
no error.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Move the Device-related types to libcontainer/devices, so that
the package can be used in isolation. Aliases have been created
in libcontainer/configs for backward compatibility.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The kubelet uses libct/cgroups code to set up cgroups. It creates a
parent cgroup (kubepods) to put the containers into.
The problem (for cgroupv2 that uses eBPF for device configuration) is
the hard requirement to have devices cgroup configured results in
leaking an eBPF program upon every kubelet restart. program. If kubelet
is restarted 64+ times, the cgroup can't be configured anymore.
Work around this by adding a SkipDevices flag to Resources.
A check was added so that if SkipDevices is set, such a "container"
can't be started (to make sure it is only used for non-containers).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This patch adds a test based on real world usage of runc hooks
(libnvidia-container). We verify that mounting a library inside
a container and running ldconfig succeeds.
Signed-off-by: Renaud Gaubert <rgaubert@nvidia.com>
This (and the converting function) is only used by one of the four
cgroup drivers. The other three do some checking and conversion in
place, so let the fs2 do the same.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Making them the same type is simply confusing, but also means that you
could accidentally use one in the wrong context. This eliminates that
problem. This also includes a whole bunch of cleanups for the types
within DeviceRule, so that they can be used more ergonomically.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
These lists have been in the codebase for a very long time, and have
been unused for a large portion of that time -- specconv doesn't
generate them and the only user of these flags has been tests (which
doesn't inspire much confidence).
In addition, we had an incorrect implementation of a white-list policy.
This wasn't exploitable because all of our users explicitly specify
"deny all" as the first rule, but it was a pretty glaring issue that
came from the "feature" that users can select whether they prefer a
white- or black- list. Fix this by always writing a deny-all rule (which
is what our users were doing anyway, to work around this bug).
This is one of many changes needed to clean up the devices cgroup code.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
In case systemd is used to set cgroups for the container,
it creates a scope unit dedicated to it (usually named
`runc-$ID.scope`).
This patch adds an ability to set arbitrary systemd properties
for the systemd unit via runtime spec annotations.
Initially this was developed as an ability to specify the
`TimeoutStopUSec` property, but later generalized to work with
arbitrary ones.
Example usage: add the following to runtime spec (config.json):
```
"annotations": {
"org.systemd.property.TimeoutStopUSec": "uint64 123456789",
"org.systemd.property.CollectMode":"'inactive-or-failed'"
},
```
and start the container (e.g. `runc --systemd-cgroup run $ID`).
The above will set the following systemd parameters:
* `TimeoutStopSec` to 2 minutes and 3 seconds,
* `CollectMode` to "inactive-or-failed".
The values are in the gvariant format (see [1]). To figure out
which type systemd expects for a particular parameter, see
systemd sources.
In particular, parameters with `USec` suffix require an `uint64`
typed argument, while gvariant assumes int32 for a numeric values,
therefore the explicit type is required.
NOTE that systemd receives the time-typed parameters as *USec
but shows them (in `systemctl show`) as *Sec. For example,
the stop timeout should be set as `TimeoutStopUSec` but
is shown as `TimeoutStopSec`.
[1] https://developer.gnome.org/glib/stable/gvariant-text.html
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
allow to set what subsystems are used by
libcontainer/cgroups/fs.Manager.
subsystemsUnified is used on a system running with cgroups v2 unified
mode.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Sebastiaan van Stijn