Arguably these docs should live elsewhere (especially if we plan to
remove memfd-bind in the future), but for now this is the only place
that fully explains this issue.
Suggested-by: Rodrigo Campos <rodrigoca@microsoft.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit ac435895b9)
Signed-off-by: lfbzhm <lifubang@acmcoder.com>
If /run/runc and /usr/bin are on different filesystems, overlayfs may
enable the xino feature which results in the following log message:
kernel: overlayfs: "xino" feature enabled using 3 upper inode bits.
Each time we have to protect /proc/self/exe. So disable xino to remove
the log message (we don't care about the inode numbers of the files
anyway).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit 9bc42d61bb)
Signed-off-by: lfbzhm <lifubang@acmcoder.com>
If the sub-cgroup RemovePath has failed for any reason, return the
error right away. This way, we don't have to check for err != nil
before retrying rmdir.
This is a cosmetic change and should not change any functionality.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 12e06a7c4f)
Signed-off-by: lfbzhm <lifubang@acmcoder.com>
An issue with runc 1.2.0 was reported to buildkit, in which
runc delete returns with an error, with the log saying:
> unable to destroy container: unable to remove container's cgroup: open /sys/fs/cgroup/snschvixiy3s74w74fjantrdg: no such file or directory
Apparently, what happens is runc is running with no cgroup access
(because /sys/fs/cgroup is mounted read-only). In this case error to
create a cgroup path (in runc create/run) is ignored, but cgroup removal
(in runc delete) is not.
This is caused by commit d3d7f7d, which changes the cgroup removal
logic in RemovePath. In the current code, if the initial rmdir has
failed (in this case with EROFS), but the subsequent os.ReadDir returns
ENOENT, it is returned (instead of being ignored -- as the path does not
exist and so there is nothing to remove).
Here is the minimal fix for the issue.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit db59489b68)
Signed-off-by: lfbzhm <lifubang@acmcoder.com>
In Fedora 41, dnf5 is used and it does not have dnf shell. Let's use
old dnf update; dnf install instead. It is two transactions instead
of one, but dnf5 is faster.
While at it:
- add `--setopt=tsflags=nodocs` as we don't need docs in CI;
- change golang-go to golang as this is a new rpm name;
- remove gcc as it is now required by golang-bin;
- remove container-selinux, criu, fuse-sshfs, iptables from rpms
as they are already installed.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 609e9a5134)
Signed-off-by: lfbzhm <lifubang@acmcoder.com>
There is a typo in the comment (ClonedBinary should be CloneBinary), and
the code has changed a bit since then, and it makes more sense to refer
to CloneSelfExe now.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 8cc7375447)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
While this is used by the majority of upper container runtimes, it was
not needed for runc itself. Since commit 515f09f7 runc uses overlay,
too, so let's add a check for this.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit ee1bced18c)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This allows to do
runc update $ID --memory=-1 --memory-swap=$VAL
for cgroup v2, i.e. set memory to unlimited and swap to a specific
value.
This was not possible because ConvertMemorySwapToCgroupV2Value rejected
memory=-1 ("unlimited"). In a hindsight, it was a mistake, because if
memory limit is unlimited, we should treat memory+swap limit as just swap
limit.
Revise the unit test; add description to each case.
Fixes: c86be8a2 ("cgroupv2: fix setting MemorySwap")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 732806e24c)
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Improve readability of ConvertMemorySwapToCgroupV2Value by switching
from a bunch of if statements to a switch, and adding a comment
describing each case.
No functional change.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit cb9f3d6d14)
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Because we have the overlay solution, we can drop runc-dmz binary
solution since it has too many limitations.
Signed-off-by: lifubang <lifubang@acmcoder.com>
(cherry picked from commit 871057d863)
Signed-off-by: lifubang <lifubang@acmcoder.com>
Since Go 1.19, the same functionality is there in os/exec package.
As we require go 1.22 now, there's no need to have this.
This basically reverts commit 9258eac0 ("libct/start: use execabs for
newuidmap lookup").
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit eb2ff52ace)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
Containerd pre-creates userns and netns before calling runc, which
results in the current code not working when SELinux is enabled,
resulting in the following error:
> runc create failed: unable to start container process: error during
container init: error mounting "mqueue" to rootfs at "/dev/mqueue":
setxattr /path/to/rootfs/dev/mqueue: operation not permitted
The solution is to become root in the user namespace right after
we join it.
Fixes#4466.
Co-authored-by: Wei Fu <fuweid89@gmail.com>
Co-authored-by: Kir Kolyshkin <kolyshkin@gmail.com>
Co-authored-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: lifubang <lifubang@acmcoder.com>
(cherry picked from commit c78f3f2ea0)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This reverts commit 65a1074c75.
We needed [1] because when we removed the bindfd logic in [2] we had not
yet moved the binary cloning logic to Go and thus it was necessary to
increase the memory limit in CI because the clone was happening after
joining the cgroup. However, [3] finally moved that code to Go and thus
the cloning is now done outside of the container's cgroup and thus is no
longer accounted as part of the container's memory usage at any point.
Now we can properly support running a simple container with lower memory
usage as we did before.
[1]: commit 65a1074c75 ("increase memory.max in cgroups.bats")
[2]: commit b999376fb2 ("nsenter: cloned_binary: remove bindfd logic entirely")
[3]: commit 0e9a3358f8 ("nsexec: migrate memfd /proc/self/exe logic to Go code")
Signed-off-by: lfbzhm <lifubang@acmcoder.com>
[cyphar: fixed commit messages]
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
There is a race situation when we are opening a file, if there is a
small fd was closed at that time, maybe it will be reused by safeExe.
Because of Go stdlib fds shuffling bug, if the fd of safeExe is too
small, go stdlib will dup3 it to another fd, or dup3 a other fd to this
fd, then it will cause the fd type cmd.Path refers to a random path,
and it can lead to an error "permission denied" when starting the process.
Please see #4294 and <https://github.com/golang/go/issues/61751>.
So we should not use the original fd of safeExe, but use the fd after
shuffled by Go stdlib. Because Go stdlib will guarantee this fd refers to
the correct file.
Signed-off-by: lfbzhm <lifubang@acmcoder.com>
Commit b999376fb2 ("nsenter: cloned_binary: remove bindfd logic
entirely") removed the read-only bind-mount logic from our cloned binary
code because it wasn't really safe because a container with
CAP_SYS_ADMIN could remove the MS_RDONLY bit and get write access to
/proc/self/exe (even with user namespaces this could've been an issue
because it's not clear if the flags are locked).
However, copying a binary does seem to have a minor performance impact.
The only way to have no performance impact would be for the kernel to
block these write attempts, but barring that we could try to reduce the
overhead by coming up with a mount that cannot have it's read-only bits
cleared.
The "simplest" solution is to create a temporary overlayfs using
fsopen(2) which uses the directory where runc exists as a lowerdir,
ensuring that the container cannot access the underlying file -- and we
don't have to do any copies.
While fsopen(2) is not free because mount namespace cloning is usually
expensive (and so it seems like the difference would be marginal), some
basic performance testing seems to indicate there is a ~60% improvement
doing it this way and that it has effectively no overhead even when
compared to just using /proc/self/exe directly:
% hyperfine --warmup 50 \
> "./runc-noclone run -b bundle ctr" \
> "./runc-overlayfs run -b bundle ctr" \
> "./runc-memfd run -b bundle ctr"
Benchmark 1: ./runc-noclone run -b bundle ctr
Time (mean ± σ): 13.7 ms ± 0.9 ms [User: 6.0 ms, System: 10.9 ms]
Range (min … max): 11.3 ms … 16.1 ms 184 runs
Benchmark 2: ./runc-overlayfs run -b bundle ctr
Time (mean ± σ): 13.9 ms ± 0.9 ms [User: 6.2 ms, System: 10.8 ms]
Range (min … max): 11.8 ms … 16.0 ms 180 runs
Benchmark 3: ./runc-memfd run -b bundle ctr
Time (mean ± σ): 22.6 ms ± 1.3 ms [User: 5.7 ms, System: 20.7 ms]
Range (min … max): 19.9 ms … 26.5 ms 114 runs
Summary
./runc-noclone run -b bundle ctr ran
1.01 ± 0.09 times faster than ./runc-overlayfs run -b bundle ctr
1.65 ± 0.15 times faster than ./runc-memfd run -b bundle ctr
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This test case is frequently hanging recently. Might be caused
by a recent kernel update from 5.14.0-427.33.1.el9_4.x86_64 to
5.14.0-427.37.1.el9_4.x86_64.
Could not reproduce locally.
Let's skip it for now.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin