Commit 5d0ffbf9c8 added OOM kill count checking and better container
start/run/exec error reporting in case we hit OOM.
It also introduced warnings like these:
> level=warning msg="unable to get oom kill count" error="openat2
> /sys/fs/cgroup/user.slice/user-1000.slice/user@1000.service/test_hello/memory.events:
> no such file or directory"
In case of rootless containers, unless cgroup is delegated or systemd is
used, runc can not create a cgroup and thus it fails to get OOM kill
count. This is expected, and the warning should not be shown in this
case.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For fs, commit fc620fdf81 made rootless field private,
and for fs2, it was always private, and yet comments in both
mention it as m.Rootless.
Fix it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
full diff: https://github.com/cilium/ebpf/compare/v0.4.0...v0.5.0
Breaking changes
------------------------------------------
All LoadPinned*() functions now take LoadPinOptions to control loader behaviour.
Simply pass nil to load with default options.
- LoadPinnedMap()
- LoadPinnedProgram()
- LoadPinnedCgroup()
- LoadPinnedIter()
- LoadPinnedRawLink()
- LoadPinnedNetNs()
Bug fixes
------------------------------------------
- Program.IsPinned() now behaves correctly on maps loaded from bpffs
- Map.Pin() no longer clobbers the destination file if it already exists
Features
------------------------------------------
- Attaching to k(ret)probes and tracepoints can now be done with link.Kprobe(),
link.Kretprobe() and link.Tracepoint()
- Programs of type Kprobe automatically get their KernelVersion fields populated
by detecting the kernel version at runtime
- MapOptions now contains a LoadPinOptions
- ProgSpec now contains a Flags field, adding support for BPF_F_SLEEPABLE
- Made BTF map loader more flexible by looping over Vars in a BTF data section
- Pinned Maps and Programs can now be loaded from bpffs in read-or write-only mode
- Added golangci-lint project configuration, running in CI
Examples
------------------------------------------
kprobe and tracepoint examples updated to use the new link.Kprobe() and link.Tracepoint() API
There is now an example for how to attach eBPF programs to uprobes
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Caught by golangci-lint when enabling golint:
libcontainer/cgroups/ebpf/ebpf.go:35:12: SA1019: prog.Attach is deprecated: use link.RawAttachProgram instead. (staticcheck)
if err := prog.Attach(dirFD, ebpf.AttachCGroupDevice, unix.BPF_F_ALLOW_MULTI); err != nil {
^
libcontainer/cgroups/ebpf/ebpf.go:39:13: SA1019: prog.Detach is deprecated: use link.RawDetachProgram instead. (staticcheck)
if err := prog.Detach(dirFD, ebpf.AttachCGroupDevice, unix.BPF_F_ALLOW_MULTI); err != nil {
^
Worth noting that we currently call prog.Detach() with unix.BPF_F_ALLOW_MULTI;
https://github.com/golang/sys/blob/22da62e12c0cd9c1da93581e1113ca4d82a5be14/unix/zerrors_linux.go#L178
BPF_F_ALLOW_MULTI = 0x2
Looking at the source code for prog.Detach(); https://github.com/cilium/ebpf/blob/v0.4.0/prog.go#L579-L581,
this would _always_ produce an error:
if flags != 0 {
return errors.New("flags must be zero")
}
Note that the flags parameter is not used (except for that validation)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Since the previous commit, some strings logged by write_log() contain a
literal newline, which leads to errors like this one:
> # time="2020-06-07T15:41:37Z" level=error msg="failed to decode \"{\\\"level\\\":\\\"debug\\\", \\\"msg\\\": \\\"nsexec-0[2265]: update /proc/2266/uid_map to '0 1000 1\\n\" to json: invalid character '\\n' in string literal"
The fix is to escape such characters.
Add a simple (as much as it can be) routine which implements JSON string
escaping as required by RFC4627, section 2.5, plus escaping of DEL (0x7f)
character (not required, but allowed by the standard, and usually done
by tools such as jq).
As much as I hate to code something like this, I was not able to find
a ready to consume and decent C implementation (not using glib).
Added a test case (and some additional asserts in C code, conditionally
enabled by the test case) to make sure the implementation is correct.
The test case have to live in a separate directory so we can use
different C flags to compile the test, and use C from go test.
[v2: try to simplify the code, add more tests]
[v3: don't do exit(1), try returning an error instead]
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>
In order to make 'runc --debug' actually useful for debugging nsexec
bugs, provide information about all the internal operations when in
debug mode.
[@kolyshkin: rebasing; fix formatting via indent for make validate to pass]
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Alas, the EPERM on chdir saga continues...
Unfortunately, the there were two releases between when https://github.com/opencontainers/runc/commit/5e0e67d76cc99d76c8228d48f38f37034503f315 was released
and when the workaround https://github.com/opencontainers/runc/pull/2712 was added.
Between this, folks started relying on the ability to have a workdir that the container user doesn't have access to.
Since this case was previously valid, we should continue support for it.
Now, we retry the chdir:
Once at the top of the function (to catch cases where the runc user has access, but container user does not)
and once after we setup user (to catch cases where the container user has access, and the runc user does not)
Add a test case for this as well.
Signed-off-by: Peter Hunt <pehunt@redhat.com>
Kir Kolyshkin (3):
make release: build/include libseccomp
script/release.sh: fix shellcheck warnings
ci: make static built binary available
LGTMs: @AkihiroSuda @cyphar
Closes#2770
Moving these utilities to a separate package, so that consumers of this
package don't have to pull in the whole "system" package.
Looking at uses of these utilities (outside of runc itself);
`RunningInUserNS()` is used by [various external consumers][1],
so adding a "Deprecated" alias for this.
[1]: https://grep.app/search?current=2&q=.RunningInUserNS
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Move the unix-specific code to a file that's not compiled on
Windows.
Some of the errors (ErrUnsupported, ErrNoPasswdEntries, ErrNoGroupEntries)
are used in other parts of the code, so are moved to a non-platform
specific file.
Most of "user" is probably not useful on Windows, although it's possible
that Windows code may have to parse a passwd file, so leaving that code
for now.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit bf749516 added these two functions, but they are only used from
Windows code. The v1 of this patch moved these functions to _windows.go
file, but after some discussion we decided to drop windows code
altogether, so this is what this patch now does.
This fixes
> libcontainer/user/user.go:64:6: func `groupFromOS` is unused (unused)
> libcontainer/user/user.go:35:6: func `userFromOS` is unused (unused)
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>
1. Move docs/systemd-properties.md to docs/systemd.md
2. Document the cgroupsPath to systemd unit name and slice conversion
rules, as well as mapping of OCI runtime spec resource limits to
systemd unit properties.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
c.Parent is only used by systemd cgroup drivers, and both v1 and v2
drivers do have code to set the default if it is empty, so setting
it here is redundant.
In addition, in case of cgroup v2 rootless container setting it here
is harmful as the default should be user.slice not system.slice.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
libseccomp is LGPL, meaning if we statically link it, we have to include
the source code of the library.
Amend "make release" to download and build libseccomp, build runc
against it, and include its sources into the release directory.
The only caveat is I found no way to stop go build from using the
stock (distro-provided) libseccomp.a, so the script checks that
the stock libseccomp.a is not available, and aborts otherwise.
While at it:
- enable shellcheck for script/release.sh
- remove libseccomp installation from the gha job
- add dependecies needed for libseccomp build to the gha job
[v2: also include libseccomp .asc file]
[v3: rebase]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This uploads the results of make release step (static binary and
a source tarball) so that they are available. I am not very sure if
it's of any use, but at least one can download and play with a static
binary from any PR.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Akihiro Suda