This is a follow-up to #5275. That change reused a single tmpfs mount
to mask multiple directories, which is efficient when masking more than
one path. However, it introduced unnecessary overhead when only one
directory is masked. This commit restores the original behavior for the
single-path case while preserving shared tmpfs logic for multiple paths.
Signed-off-by: lifubang <lifubang@acmcoder.com>
Switch from sync.Once to sync.OnceFunc and sync.OnceValues.
Keep Root a function (rather than a variable) because godoc
renders function doc better than a variable doc.
Switch to using internal function root internally.
Modify tests accordingly (and simplify NewIntelRdtTestUtil to
fakeRoot).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The whole struct intelRdtStatus with its methods and a sync.Once is not
needed, since intelrtd.Is*Enabled methods are already run-once (or use
run-once and a simple comparison).
Yet it is still needed for the test to fake values returned by *Enabled.
Simplify to use func pointers which a test case overwrites.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It is not doing anything, and tests can just instantiate the &Manager{}.
Suggested-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Use sync.OnceValue.
2. Fix the len(buf) check -- we only need 1 byte. Real kernel output
is "Y\n" so practically this change is a no-op.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As runc binary grows in size over time (new features, more
dependencies) some tests start to flake because of low memory limits.
One such test is "runc run (cgroup v2 resources.unified override)";
it obviously fails because of 1M memory limit:
> runc run failed: unable to start container process: container init was OOM-killed (memory limit too low?)
Increase the limits 4x. Do the same for the "unified only" test.
Fixes issue 5264.
Reported-by: Kevin Berry <kpberry11@gmail.com>
Reported-by: Ricardo Branco <rbranco@suse.de>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
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>
Apparently, lima's experimental/fedora-rawhide image does not include
which rpm, and we don't really want to bother installing it.
Replace "which" with "command -v". Looks like this was the only place;
we already use "command -v" everywhere else.
This should fix lima (experimental/fedora-rawhide) CI.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin (3):
runc exec -p: fix adding HOME to nil env
tests/int/env.bats: add test for runc exec -p
tests/int: amend runc exec --env test
LGTMs: rata cyphar
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>
All existing tests check runc run, and there is no single runc exec
environment test except for one in exec.bats.
Add it (no new issues found).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This tests checks that "runc exec --env VAR=VAR ..." actually appends
VAR=VAL to the exec's environment.
Add additional checks that:
- process.env from config.json is also inherited;
- HOME is set.
Those checks do not reveal any new issues.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
runc's build scripts and CI setup used a mix of wget (4 sites) and
curl (8 sites) for HTTPS fetches. Standardise the three script sites on
curl, which is already required by the majority of call sites.
* script/build-libpathrs.sh and script/build-seccomp.sh: replace
wget "<url>"{,.asc} with curl -fsSLO "<url>"{,.asc}. Bash brace
expansion still yields two separate downloads (the tarball and its
.asc signature).
* script/setup_host.sh: replace wget with curl in the Fedora RPM
install list.
Leaving the single wget call in Dockerfile untouched per review
feedback -- the trixie base image already ships wget, and switching
would add an extra apt-get update && install step before the existing
single-pass package install.
Refs #5240
Signed-off-by: Ali <alliasgher123@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>
add bat integration test for rootfs propagation test, expect to
see the mount propagation is slave, the test will create a isolate mntns
to run the test as the test will mutate the rootfs propagation
Signed-off-by: sean <xujihui1985@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>
lifubang