Kubernetes may add one sysfs thermal_throttle entry per CPU to
maskedPaths. On large Intel systems this can produce many directory
masks for a single container. runc currently handles each directory
mask with a separate read-only tmpfs mount, and therefore a separate
tmpfs superblock.
On Linux 4.18/RHEL 8 kernels, creating and tearing down many tmpfs
superblocks can contend on the global shrinker_rwsem when containers
start or stop concurrently.
Use one read-only tmpfs for directory masks and bind-mount it over the
remaining directory targets. The first non-procfs-fd directory mount is
reopened through the container root fd before it is reused. File masks
still bind /dev/null, and procfs fd targets keep the existing
one-tmpfs-per-target behaviour because they are fd aliases rather than
stable rootfs paths.
If the bind-mount of the shared source fails (e.g. due to kernel
restrictions), fall back to individual tmpfs mounts for all remaining
directories. Tmpfs mounts use nr_blocks=1,nr_inodes=1 to minimise
kernel resource usage.
The bind mounts do not create additional tmpfs superblocks. They also
retain the read-only mount flag inherited from the source vfsmount, so
the masking semantics remain unchanged.
xref: kubernetes/kubernetes#138512
xref: kubernetes/kubernetes#138388
xref: kubernetes/kubernetes#131018
Co-authored-by: Davanum Srinivas <davanum@gmail.com>
Refactored-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: lifubang <lifubang@acmcoder.com>
Previously, masked directories (e.g., /proc/acpi, /proc/scsi) were
mounted as read-only tmpfs without explicit size or inode limits.
Although these mounts are meant to be empty and unwritable, the lack
of resource constraints means that—should an attacker bypass the
read-only protection (e.g., via container escape, mount namespace
manipulation, or a kernel vulnerability)—the tmpfs could consume up
to 50% of system memory by default (the kernel's default tmpfs limit).
To mitigate this risk in high-density container environments and
adhere to the principle of least privilege, we now explicitly set:
- nr_blocks=1 (sufficient for at most one block size)
- nr_inodes=1 (sufficient for at most one inode)
Ref: https://man7.org/linux/man-pages/man5/tmpfs.5.html
These limits ensure that even if compromised, kernel memory usage
remains strictly bounded and negligible.
This change aligns with best practices used by other container
runtimes and strengthens defense-in-depth for sensitive masked paths.
Co-authored-by: Davanum Srinivas <davanum@gmail.com>
Refactored-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: lifubang <lifubang@acmcoder.com>
Before commit 7dc24868, when process.env was nil, prepareEnv
returned a flag telling HOME is not set, and it was added.
Commit 7dc24868 moved the functionality of adding HOME into
prepareEnv but did not properly handle nil case. As a result,
runc exec -p with process.json having no env set resulted in
an exec with no HOME set.
Fix this, and add unit and integration tests.
Fixes: 7dc24868 ("libct: switch to numeric UID/GID/groups")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since commit 3cdda46 the poststart hooks runs after the container
process start, and so they race.
Move the poststart hook check to a separate step after the container
process has exited.
Fixes 5245.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This test is racy for a long time now. All the logs I could find in CI
seem to be dangling symlinks, like the test shows "23 -> ". This means
the fd was closed before we did the call to readlink().
Let's try to disable the GC. This should get rid of the "fds are getting
closed before we read them" part.
Updates: #4297
Signed-off-by: Rodrigo Campos <rodrigo@amutable.com>
By default, readlink is silent about any errors. Make it verbose so we
can better interpret any test failures.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When running a process inside a container, make sure its stderr is not
nil (except for some trivial cases like cat). Modify waitProcess to show
failed command's stderr, if possible.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since Wait returns an ExitError if process' exit status is not 0,
checking process status is redundant and this code is never reached.
Remove it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When rootfsPropagation is set to rslave, prepareRoot() was forcing the
rootfs parent mount to MS_PRIVATE before bind-mounting and pivoting into
the rootfs. That breaks the slave relationship needed for HostToContainer
propagation, so later unmount/remount events on host mountpoints under
the rootfs are not reflected inside the running container.
Fix this by keeping the rootfs parent mount as MS_SLAVE for slave-like
rootfs propagation settings, while leaving the final root propagation
remount in place.
Signed-off-by: sean <xujihui1985@gmail.com>
These helpers all make more sense as a self-contained package and moving
them has the added benefit of removing an unneeded libpathrs dependency
(from libcontainer/utils's import of pathrs-lite) from several test
binaries.
Signed-off-by: Aleksa Sarai <aleksa@amutable.com>
This allows users to automaticaly migrate to the new location
using `go fix`. It has some limitations, but can help smoothen
the transition; for example, taking this file;
```
package main
import (
"github.com/opencontainers/runc/libcontainer/devices"
)
func main() {
_, _ = devices.DeviceFromPath("a", "b")
_, _ = devices.HostDevices()
_, _ = devices.GetDevices("a")
}
```
Running `go fix -mod=readonly ./...` will migrate the code;
```
package main
import (
devices0 "github.com/moby/sys/devices"
)
func main() {
_, _ = devices0.DeviceFromPath("a", "b")
_, _ = devices0.HostDevices()
_, _ = devices0.GetDevices("a")
}
```
updates b345c78dca
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The runtime-spec [1] currently says:
> 6. Runtime's start command is invoked with the unique identifier of
> the container.
> 7. The startContainer hooks MUST be invoked by the runtime. If any
> startContainer hook fails, the runtime MUST generate an error, stop
> the container, and continue the lifecycle at step 12.
> 8. The runtime MUST run the user-specified program, as specified by
> process.
> 9. The poststart hooks MUST be invoked by the runtime. If any
> poststart hook fails, the runtime MUST generate an error, stop the
> container, and continue the lifecycle at step 12.
> ...
> 11. Runtime's delete command is invoked with the unique identifier of
> the container.
> 12. The container MUST be destroyed by undoing the steps performed
> during create phase (step 2).
> 13. The poststop hooks MUST be invoked by the runtime. If any poststop
> hook fails, the runtime MUST log a warning, but the remaining hooks
> and lifecycle continue as if the hook had succeeded.
Currently, we do 9 before 8 (heck, even before 6), which is clearly
against the spec and results in issues like the one described in [2].
Let's move running poststart hook to after the user-specified process
has started.
NOTE this patch only fixes the order and does not implement removing
the container when the poststart hook failed (as this part of the spec
is controversial -- destroy et al and should probably be, and currently
are, part of "runc delete").
[1]: https://github.com/opencontainers/runtime-spec/blob/main/runtime.md#lifecycle
[2]: https://github.com/opencontainers/runc/issues/5182
Reported-by: ningmingxiao <ning.mingxiao@zte.com.cn>
Reported-by: Erik Sjölund <erik.sjolund@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Rename c.signal to c.signalInit, and add c.signal which is a lock-less
form of c.Signal.
To be used by the next patch.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The libcontainer/devices package has been moved to moby/sys/devices, so
we can just point users to that and keep some compatibility shims around
until runc 1.6. We don't use it at all so there are no other changes
needed.
Signed-off-by: Aleksa Sarai <aleksa@amutable.com>
This uses preopened rootfs in Chdir and pivotRoot.
While at it, add O_PATH when opening oldroot in pivotRoot.
Co-authored-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: lfbzhm <lifubang@acmcoder.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
A lot of filesystem-related stuff happens inside the container root
directory, and we have used its name before. It makes sense to pre-open
it and use a *os.File handle instead.
Function names in internal/pathrs are kept as is for simplicity (and it
is an internal package), but they now accept root as *os.File.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Indeed, it does not make sense to prepend c.root once we started using
MkdirAllInRoot in commit 63c29081.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Li Fubang (3):
test: check mount source fds are cleaned up with idmapped mounts
libct: close mount source fd as soon as possible
libct: add a nil check for mountError
LGTMs: kolyshkin rata cyphar
This commit factors out setupAndMountToRootfs without changing any
logic. Use "Hide whitespace changes" during review to focus on the
actual changes.
The refactor ensures the mount source file descriptor is closed via
defer in each loop iteration, reducing the total number of open FDs
in runc. This helps avoid hitting the file descriptor limit under
high concurrency or when handling many mounts.
Signed-off-by: lifubang <lifubang@acmcoder.com>
This adds support for WaitKillableRecv seccomp flag
(also known as SCMP_FLTATR_CTL_WAITKILL in libseccomp and
as SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV in the kernel).
This requires:
- libseccomp >= 2.6.0
- libseccomp-golang >= 0.11.0
- linux kernel >= 5.19
Note that this flag does not make sense without NEW_LISTENER, and
the kernel returns EINVAL when SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV
is set but SECCOMP_FILTER_FLAG_NEW_LISTENER is not set.
For runc this means that .linux.seccomp.listenerPath should also be set,
and some of the seccomp rules should have SCMP_ACT_NOTIFY action. This
is why the flag is tested separately in seccomp-notify.bats.
At the moment the only adequate CI environment for this functionality is
Fedora 43. On all other platforms (including CentOS 10 and Ubuntu 24.04)
it is skipped similar to this:
> ok 251 runc run [seccomp] (SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV) # skip requires libseccomp >= 2.6.0 and API level >= 7 (current version: 2.5.6, API level: 6)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When deprecating Relabel field, its json attributes were mistakenly
removed, so now it is:
- saved to JSON under "Relabel" (rather than "relabel");
- won't be ignored if empty.
Let's fix it before it's too late.
Fixes: 8b2b5e94 ("libct: remove relabeling dead code")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It makes more sense to save whether we should cleanup the directory
after it gets created (to avoid error cases deleting a different
directory) as well as tying this check to the existing os.ErrExist
check rather than doing an extra stat(2).
Fixes: e2baa3ad10 ("Intel RDT: update according to spec changes.")
Suggested-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This fixes random failures to start a container in conmon integration
tests (see issue 5151).
I guess we need to find another way to fix issue 4645.
This reverts commit 1b39997e73.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
There is no way to set Mount.Relabel field via OCI spec (config.json),
and so the relabeling code is never used.
My guess it's a leftover from times when runc used to be part of Docker.
Remove it, and mark Relabel field as deprecated.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
There is one proposed clarification to the OCI spec: the subdirectory
needs to be deleted. Runc already does that, but the clarification adds
for directory removal only if the directory was created by us.
Signed-off-by: Ismo Puustinen <ismo.puustinen@intel.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
unix.CPUSet is limited to 1024 CPUs. Calling
unix.SchedSetaffinity(pid, cpuset) removes all CPUs starting from 1024
from allowed CPUs of pid, even if cpuset is all ones. As a
consequence, when runc tries to reset CPU affinity to "allow all" by
default, it prevents all containers from CPUs 1024 onwards.
This change uses a huge CPU mask to play safe and get all possible
CPUs enabled with a single sched_setaffinity call.
Fixes: #5023
Signed-off-by: Antti Kervinen <antti.kervinen@intel.com>
These were all marked deprecated in commit a75076b4a4 ("Switch to
opencontainers/cgroups") when we switched maintenance of our cgroup code
to opencontainers/cgroups.
Users have had ample time to switch to opencontainers/cgroups
themselves, so we can finally remove this.
Note that the whole libcontainer/devices package will be moved to
moby/sys in the near future, so this whole package will be marked
deprecated soon.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
These were inadvertently added to our exported APIs by commit
eeda7bdf80cca ("Add memory policy support"). We couldn't remove them
from runc 1.4.x, but we deprecated them in commit 3741f9186d
("libct/configs: mark MPOL_* constants as deprecated") and marked them
for removal in runc 1.5. Users should never have used these in the first
place.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This was deprecated in commit e6a4870e4ac40 ("libct: better errors for
hooks"), and users have had ample time to migrate to Hooks.Run since.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
These were all marked deprecated in commit a75076b4a4 ("Switch to
opencontainers/cgroups") when we switched maintenance of our cgroup code
to opencontainers/cgroups.
Users have had ample time to switch to opencontainers/cgroups
themselves, so we can finally remove this.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
There is a chance of panic here -- eliminate it.
Add a test case (which panics before the fix).
Reported-by: Luke Hinds <luke@stacklok.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Example code in README is outdated (especially since cgroups is moved to
a separate repository) and lacks proper import statements. And, since it
is not code, it is hard to keep it up to date.
Let's move it out to the example_test.go file and refer to it. Note we
still don't run it, but it will be compiled and linted in CI.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Fix *some* of the prealloc linter warnings. While it does not make sense
to address all warnings (or add prealloc to the list of linters we run
in CI), some do make sense.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Previously, when prepareCgroupFD would not open container's cgroup
(as configured in config.json and saved to state.json), it returned
a fatal error, as we presumed a container can't exist without its own
cgroup.
Apparently, it can. In a case when container is configured without
cgroupns (i.e. it uses hosts cgroups), and /sys/fs/cgroup is mounted
read-write, a rootful container's init can move itself to an entirely
different cgroup (even a new one that it just created), and then the
original container cgroup is removed by the kernel (or systemd?) as
it has no processes left. By the way, from the systemd point of view
the container is gone. And yet it is still there, and users want
runc exec to work!
And it worked, thanks to the "let's try container init's cgroup"
fallback as added by commit c91fe9aeba ("cgroup2: exec: join the
cgroup of the init process on EBUSY"). The fallback was added for
the entirely different reason, but it happened to work in this very
case, too.
This behavior was broken with the introduction of CLONE_INTO_CGROUP
support.
While it is debatable whether this is a valid scenario when a container
moves itself into a different cgroup, this very setup is used by e.g.
buildkitd running in a privileged kubernetes container (see issue 5089).
To restore the way things are expected to work, add the same "try
container init's cgroup" fallback into prepareCgroupFD.
While at it, simplify the code flow.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
lifubang