This field reports swap-only usage. For cgroupv1, `Usage` and `Failcnt`
are set by subtracting memory usage from memory+swap usage. For cgroupv2,
`Usage`, `Limit`, and `MaxUsage` are set. This commit also export `MaxUsage`
of memory under cgroupv2 mode, using `memory.peak` introduced in kernel 5.19.
Signed-off-by: Heran Yang <heran55@126.com>
The original reasoning for this option was to avoid having mount options
be overwritten by runc. However, adding command-line arguments has
historically been a bad idea because it forces strict-runc-compatible
OCI runtimes to copy out-of-spec features directly from runc and these
flags are usually quite difficult to enable by users when using runc
through several layers of engines and orchestrators.
A far more preferable solution is to have a heuristic which detects
whether copying the original mount's mount options would override an
explicit mount option specified by the user. In this case, we should
return an error. You only end up in this path in the userns case, if you
have a bind-mount source with locked flags.
During the course of writing this patch, I discovered that several
aspects of our handling of flags for bind-mounts left much to be
desired. We have completely botched the handling of explicitly cleared
flags since commit 97f5ee4e6a ("Only remount if requested flags differ
from current"), with our behaviour only becoming increasingly more weird
with 50105de1d8 ("Fix failure with rw bind mount of a ro fuse") and
da780e4d27 ("Fix bind mounts of filesystems with certain options
set"). In short, we would only clear flags explicitly request by the
user purely by chance, in ways that it really should've been reported to
us by now. The most egregious is that mounts explicitly marked "rw" were
actually mounted "ro" if the bind-mount source was "ro" and no other
special flags were included. In addition, our handling of atime was
completely broken -- mostly due to how subtle the semantics of atime are
on Linux.
Unfortunately, while the runtime-spec requires us to implement
mount(8)'s behaviour, several aspects of the util-linux mount(8)'s
behaviour are broken and thus copying them makes little sense. Since the
runtime-spec behaviour for this case (should mount options for a "bind"
mount use the "mount --bind -o ..." or "mount --bind -o remount,..."
semantics? Is the fallback code we have for userns actually
spec-compliant?) and the mount(8) behaviour (see [1]) are not
well-defined, this commit simply fixes the most obvious aspects of the
behaviour that are broken while keeping the current spirit of the
implementation.
NOTE: The handling of atime in the base case is left for a future PR to
deal with. This means that the atime of the source mount will be
silently left alone unless the fallback path needs to be taken, and any
flags not explicitly set will be cleared in the base case. Whether we
should always be operating as "mount --bind -o remount,..." (where we
default to the original mount source flags) is a topic for a separate PR
and (probably) associated runtime-spec PR.
So, to resolve this:
* We store which flags were explicitly requested to be cleared by the
user, so that we can detect whether the userns fallback path would end
up setting a flag the user explicitly wished to clear. If so, we
return an error because we couldn't fulfil the configuration settings.
* Revert 97f5ee4e6a ("Only remount if requested flags differ from
current"), as missing flags do not mean we can skip MS_REMOUNT (in
fact, missing flags are how you indicate a flag needs to be cleared
with mount(2)). The original purpose of the patch was to fix the
userns issue, but as mentioned above the correct mechanism is to do a
fallback mount that copies the lockable flags from statfs(2).
* Improve handling of atime in the fallback case by:
- Correctly handling the returned flags in statfs(2).
- Implement the MNT_LOCK_ATIME checks in our code to ensure we
produce errors rather than silently producing incorrect atime
mounts.
* Improve the tests so we correctly detect all of these contingencies,
including a general "bind-mount atime handling" test to ensure that
the behaviour described here is accurate.
This change also inlines the remount() function -- it was only ever used
for the bind-mount remount case, and its behaviour is very bind-mount
specific.
[1]: https://github.com/util-linux/util-linux/issues/2433
Reverts: 97f5ee4e6a ("Only remount if requested flags differ from current")
Fixes: 50105de1d8 ("Fix failure with rw bind mount of a ro fuse")
Fixes: da780e4d27 ("Fix bind mounts of filesystems with certain options set")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
There is no point in showing the underlying error when path == "",
because it is ENOENT.
Revert the change done in commit e1584831b6.
Fixes: e1584831b6
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit f34eb2c00 introduced a workaround to retry on EINTR due to changes in Go 1.14.
It was fixed in Go 1.15 [1], meaning a custom retry loop is no longer
necessary.
Keep the test case to avoid future regressions.
[1] https://github.com/golang/go/issues/38033
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When we added nolibc, we started using it unconditionally. But runc is
currently being compiled on more arches than supported by nolibc, like
MIPS.
Let's compile using stdlib if the arch we are compiling on is not
supported by nolibc.
If compilation is broken in some arch, just removing it from the
NOLIBC_GOARCHES variable should fix the compilation, as it will fallback
to use the C stdlib.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
This adds support for hugetlb.<pagesize>.rsvd limiting and accounting.
The previous non-rsvd max/limit_in_bytes does not account for reserved
huge page memory, making it possible for a processes to reserve all the
huge page memory, without being able to allocate it (due to cgroup
restrictions).
In practice this makes it possible to successfully mmap more huge page
memory than allowed via the cgroup settings, but when using the memory
the process will get a SIGBUS and crash. This is bad for applications
trying to mmap at startup (and it succeeds), but the program crashes
when starting to use the memory. eg. postgres is doing this by default.
This also keeps writing to the old max/limit_in_bytes, for backward
compatibility.
More info can be found here: https://lkml.org/lkml/2020/2/3/1153
(commit message mostly written by Odin Ugedal)
Co-authored-by: Odin Ugedal <odin@ugedal.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since Go 1.16, [Go issue 1435][1] is solved, and the stdlib syscall
implementations work on Linux. While they are a bit more
flexible/heavier-weight than the implementations that were copied to
libcontainer/system (working across all threads), we compile with Cgo,
and using the libc wrappers should be just as suitable.
[1]: https://github.com/golang/go/issues/1435
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
The name "root" (or "containerRoot") is confusing; one might think it is
the root of container's file system (the directory we chroot into).
Rename to stateDir for clarity.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Umask is problematic for Go programs as it affects other goroutines
(see [1] for more details).
Instead of using it, let's just prop up with Chmod.
Note this patch misses the MkdirAll call in createDeviceNode. Since the
runtime spec does not say anything about creating intermediary
directories for device nodes, let's assume that doing it via mkdir with
the current umask set is sufficient (if not, we have to reimplement
MkdirAll from scratch, with added call to os.Chmod).
[1] https://github.com/opencontainers/runc/pull/3563#discussion_r990293788
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Otherwise it is shown when compiling, like this:
# We use the flags suggested in nolibc/nolibc.h, it makes the binary very small.
gcc -fno-asynchronous-unwind-tables -fno-ident -s -Os -nostdlib -lgcc -static -o runc-dmz _dmz.c
strip -gs runc-dmz
Having it before the target is equally clear and will not be shown while
compiling.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Linux repo has under `tools/include/nolibc` very simple include files
that we can use to generate very small binaries that don't depend on
libc.
To make things even better, since Linux 6.6 it supports all the
architectures we support in runc, which is just beautiful.
The runc-dmz binary on x86_64 before this patch (on my debian host) was
taking 636K, with this patch it takes only 8K.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
We have different requirements for the initial configuration and
initWaiter pipe (just send netlink and JSON blobs with no complicated
handling needed for message coalescing) and the packet-based
synchronisation pipe.
Tests with switching everything to SOCK_SEQPACKET lead to endless issues
with runc hanging on start-up because random things would try to do
short reads (which SOCK_SEQPACKET will not allow and the Go stdlib
explicitly treats as a streaming source), so splitting it was the only
reasonable solution. Even doing somewhat dodgy tricks such as adding a
Read() wrapper which actually calls ReadPacket() and makes it seem like
a stream source doesn't work -- and is a bit too magical.
One upside is that doing it this way makes the difference between the
modes clearer -- INITPIPE is still used for initWaiter syncrhonisation
but aside from that all other synchronisation is done by SYNCPIPE.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The old name was quite confusing, and with the addition of the
procMountPlease sync message there are now multiple sync messages that
are related to "resuming" runc-init.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The idea is to remove the need for cloning the entire runc binary by
replacing the final execve() call of the container process with an
execve() call to a clone of a small C binary which just does an execve()
of its arguments.
This provides similar protection against CVE-2019-5736 but without
requiring a >10MB binary copy for each "runc init". When compiled with
musl, runc-dmz is 13kB (though unfortunately with glibc, it is 1.1MB
which is still quite large).
It should be noted that there is still a window where the container
processes could get access to the host runc binary, but because we set
ourselves as non-dumpable the container would need CAP_SYS_PTRACE (which
is not enabled by default in Docker) in order to get around the
proc_fd_access_allowed() checks. In addition, since Linux 4.10[1] the
kernel blocks access entirely for user namespaced containers in this
scenario. For those cases we cannot use runc-dmz, but most containers
won't have this issue.
This new runc-dmz binary can be opted out of at compile time by setting
the "runc_nodmz" buildtag, and at runtime by setting the RUNC_DMZ=legacy
environment variable. In both cases, runc will fall back to the classic
/proc/self/exe-based cloning trick. If /proc/self/exe is already a
sealed memfd (namely if the user is using contrib/cmd/memfd-bind to
create a persistent sealed memfd for runc), neither runc-dmz nor
/proc/self/exe cloning will be used because they are not necessary.
[1]: https://github.com/torvalds/linux/commit/bfedb589252c01fa505ac9f6f2a3d5d68d707ef4
Co-authored-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: lifubang <lifubang@acmcoder.com>
[cyphar: address various review nits]
[cyphar: fix runc-dmz cross-compilation]
[cyphar: embed runc-dmz into runc binary and clone in Go code]
[cyphar: make runc-dmz optional, with fallback to /proc/self/exe cloning]
[cyphar: do not use runc-dmz when the container has certain privs]
Co-authored-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Previously, if /var/run was mounted noexec, our cloned binary logic
would not work if memfd_create(2) was not available because we would try
to exec a binary that is on a noexec filesystem.
We cannot guarantee there will be an executable filesystem on the system
(other than mounting one ourselves, which would cause a bunch of other
headaches) but we can at least try the obvious options (/tmp, /bin, and
/). If none of these work, we will have to fail.
Reported-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This allow us to remove the amount of C code in runc quite
substantially, as well as removing a whole execve(2) from the nsexec
path because we no longer spawn "runc init" only to re-exec "runc init"
after doing the clone.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The runtime spec now allows relative mount dst paths, so remove the
comment saying we will switch this to an error later and change the
error messages to reflect that.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
From the Go specification:
"1. For a nil slice, the number of iterations is 0." [1]
Therefore, an additional nil check for before the loop is unnecessary.
[1]: https://go.dev/ref/spec#For_range
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Burstable CFS controller is introduced in Linux 5.14. This helps with
parallel workloads that might be bursty. They can get throttled even
when their average utilization is under quota. And they may be latency
sensitive at the same time so that throttling them is undesired.
This feature borrows time now against the future underrun, at the cost
of increased interference against the other system users, by introducing
cfs_burst_us into CFS bandwidth control to enact the cap on unused
bandwidth accumulation, which will then used additionally for burst.
The patch adds the support/control for CFS bandwidth burst.
runtime-spec: https://github.com/opencontainers/runtime-spec/pull/1120
Co-authored-by: Akihiro Suda <suda.kyoto@gmail.com>
Co-authored-by: Nadeshiko Manju <me@manjusaka.me>
Signed-off-by: Kailun Qin <kailun.qin@intel.com>
This makes libcontainer/userns self-dependent, largely returning to
the original implementation from lxc. The `uiMapInUserNS` is kept as
a separate function for unit-testing and fuzzing.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
If the container does not have own mount namespace configured (i.e. it
shares the mount namespace with the host), its "prestart" (obsoleted)
and "createRuntime" hooks are called twice, and its cgroups and Intel
RDT settings are also applied twice.
The code being removed was originally added by commit 2f2764984 ("Move
pre-start hooks after container mounts", Feb 17 2016). At that time,
the syncParentHooks() was called from setupRootfs(), which was only
used when the container config has mount namespace (NEWNS) enabled.
Later, commit 244c9fc426 ("*: console rewrite", Jun 4 2016) spli
the relevant part of setupRootfs() into prepareRootfs(). It was still
called conditionally (only if mount namespace was enabled).
Finally, commit 91ca331474 ("chroot when no mount namespaces is
provided", Jan 25 2018) removed the above condition, meaning
prepareRootfs(), and thus syncParentHooks(), is now called for any
container.
Meaning, the special case for when mount namespace is not enabled is no
longer needed.
Remove it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When a hook has failed, the error message looks like this:
> error running hook: error running hook #1: exit status 1, stdout: ...
The two problems here are:
1. it is impossible to know what kind of hook it was;
2. "error running hook" stuttering;
Change that to
> error running createContainer hook #1: exit status 1, stdout: ...
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains
the fix [1] whitelisting all errno comparisons for errors coming from
x/sys/unix.
Thus, these annotations are no longer necessary. Hooray!
[1] https://github.com/polyfloyd/go-errorlint/pull/47
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In the runc state JSON we always use snake_case. This is a no-op change,
but it will cause any existing container state files to be incorrectly
parsed. Luckily, commit fbf183c6f8 ("Add uid and gid mappings to
mounts") has never been in a runc release so we can change this before a
1.2.z release.
Fixes: fbf183c6f8 ("Add uid and gid mappings to mounts")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The original implementation of cgroupns had additional synchronisation
to "ensure" that the process is in the correct cgroup before unsharing
the cgroupns. This behaviour was actually never necessary, and after
commit 5110bd2fc0 ("nsenter: remove cgroupns sync mechanism") there is
no synchronisation at all, meaning that CLONE_NEWCGROUP should not get
any special treatment.
Fixes: 5110bd2fc0 ("nsenter: remove cgroupns sync mechanism")
Fixes: df3fa115f9 ("Add support for cgroup namespace")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The kernel ignores these arguments, and passing them can lead to
confusing error messages (the old source is irrelevant for MS_REMOUNT),
as well as causing issues for a future patch where we switch to
move_mount(2).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
*os.File is correctly tracked by the garbage collector, and there's no
need to use raw file descriptors for this code.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This includes quite a few cleanups and improvements to the way we do
synchronisation. The core behaviour is unchanged, but switching to
embedding json.RawMessage into the synchronisation structure will allow
us to do more complicated synchronisation operations in future patches.
The file descriptor passing through the synchronisation system feature
will be used as part of the idmapped-mount and bind-mount-source
features when switching that code to use the new mount API outside of
nsexec.c.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The code in this function became quite complicated and not entirely
correct over time. As a result, if an error is returned from parseSync,
it might end up stuck waiting for the child to finish.
1. Let's not wait() for the child twice. We already do it in the
defer statement (call p.terminate()) when we are returning an error.
2. Remove sentResume and sentRun since we do not want to check if
these were sent or not. Instead, introduce and check seenProcReady, as
procReady is always expected from runc init.
3. Eliminate the possibility to wrap nil as an error.
4. Make sure we always call shutdown on the sync socket, and do not let
shutdown error shadow the ierr.
This fixes the issue of stuck `runc runc` with the optimization patch
(sending procSeccompDone earlier) applied.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
These are not exhaustive, but at least confirm that the feature is not
obviously broken (we correctly set the time offsets).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Fix up a few things that were flagged in the review of the original
timens PR, namely around error handling and validation.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Heran Yang