We cannot have both srcFD and idMapFD set at the same time.
So, we can simplify this struct to only have one field which is used a srcFD
most of the time and as idMapFD when we do an id map mount.
Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
Previously to this commit, we used a string for srcFD as /proc/self/fd/NN.
This commit modified to this behavior, so srcFD is only an *int and the full path
is constructed in mountViaFDs() if srcFD is different than nil.
Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
runc ps requires cgroup, but all the tests but one required root. Let's
fix this.
1. Add rootless cgroup requirement to setup() to avoid repetition.
2. Add set_cgroups_path to setup() for rootless containers because
there is no default cgroup path.
3. Modify output checks to use $output rather than $lines because in case
of rootless the first line of output contains the following warning:
> runc ps may fail if you don't have the full access to cgroups
4. While at it, move the common part of every test (creating the
container and making sure it's running) to setup().
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This commit adds support for idmap mounts as specified in the runtime-spec.
We open the idmap source paths and call mount_setattr() in runc PARENT,
as we need privileges in the init userns for that, and then sends the
fds to the child process. For this fd passing we use the same mechanism
used in other parts of thecode, the _LIBCONTAINER_ env vars.
The mount is finished (unix.MoveMount) from go code, inside the userns,
so we reuse all the prepareBindMount() security checks and the remount
logic for some flags too.
This commit only supports idmap mounts when userns are used AND the mappings
are the same specified for the userns mapping. This limitation is to
simplify the initial implementation, as all our users so far only need
this, and we can avoid sending over netlink the mappings, creating a
userns with this custom mapping, etc. Future PRs will remove this
limitation.
Co-authored-by: Francis Laniel <flaniel@linux.microsoft.com>
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
We will need to pass more slices of fds to these functions in future
patches. Let's add a struct that just contains them all, instead of
adding lot of parameters to these functions.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
We will add code that uses this function in future patches. So let's
just split it to a new function now and reuse it later.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Let's move the code to send mount sources to a generic function. Future
patches will use it for idmap sources too.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
We add idmap.h with the needed includes and defines in case the system
headers don't have the definition for the idmap syscalls we need.
Future patches will use these helpers.
Co-authored-by: Francis Laniel <flaniel@linux.microsoft.com>
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
We just added this helper for other parts of the code, let's switch this
function to use the helper too.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
This was a warning already and it was requested to make this an error
while we will add validation of idmap mounts:
https://github.com/opencontainers/runc/pull/3717#discussion_r1154705318
I've also tested a k8s cluster and the config.json generated by
containerd didn't use any relative paths. I tested one pod, so it was
definitely not an extensive test.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Processes can watch /proc/self/mounts or /mountinfo, and the kernel
will notify them whenever the namespace's mount table is modified. The
notified process still needs to read and parse the mountinfo to
determine what changed once notified. Many such processes, including
udisksd and SystemD < v248, make no attempt to rate-limit their
mountinfo notifications. This tends to not be a problem on many systems,
where mount tables are small and mounting and unmounting is uncommon.
Every runC exec which successfully uses the try_bindfd container-escape
mitigation performs two mount()s and one umount() in the host's mount
namespace, causing any mount-watching processes to wake up and parse the
mountinfo file three times in a row. Consequently, using 'exec' health
checks on containers has a larger-than-expected impact on system load
when such mount-watching daemons are running. Furthermore, the size of
the mount table in the host's mount namespace tends to be proportional
to the number of OCI containers as a unique mount is required for the
rootfs of each container. Therefore, on systems with mount-watching
processes, the system load increases *quadratically* with the number of
running containers which use health checks!
Prevent runC from incidentally modifying the host's mount namespace for
container-escape mitigations by setting up the mitigation in a temporary
mount namespace.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Modify receive_fd() and send_fd() so they can be more readily reused in
cloned_binary.c. Change receive_fd() to have a single responsibility:
receiving and returning a single file descriptor over a UNIX domain
socket. Make send_fd() useable in precarious execution contexts such as
a clone(CLONE_VFORK|CLONE_VM) "thread" where allocating heap memory or
calling exit() would be dangerous.
Signed-off-by: Cory Snider <csnider@mirantis.com>
I am nominating @lifubang for the role of runc maintainer.
He provided a number of valuable contributions to runc, and demonstrated
both deep technical expertise and the long term commitment to the
project.
As noted in MAINTAINERS_GUIDE.md, we have a week to vote, and need to
get 66% of current maintainers' votes.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The passing run (with the fix) looks like this:
----
delete.bats
✓ runc delete removes failed systemd unit [4556]
runc spec (status=0):
runc run -d --console-socket /tmp/bats-run-B08vu1/runc.lbQwU5/tty/sock test-failed-unit (status=0):
Warning: The unit file, source configuration file or drop-ins of runc-cgroups-integration-test-12869.scope changed on disk. Run 'systemctl daemon-reload' to reload units.
× runc-cgroups-integration-test-12869.scope - libcontainer container integration-test-12869
Loaded: loaded (/run/systemd/transient/runc-cgroups-integration-test-12869.scope; transient)
Transient: yes
Drop-In: /run/systemd/transient/runc-cgroups-integration-test-12869.scope.d
└─50-DevicePolicy.conf, 50-DeviceAllow.conf
Active: failed (Result: timeout) since Tue 2023-06-13 14:41:38 PDT; 751ms ago
Duration: 2.144s
CPU: 8ms
Jun 13 14:41:34 kir-rhat systemd[1]: Started runc-cgroups-integration-test-12869.scope - libcontainer container integration-test-12869.
Jun 13 14:41:37 kir-rhat systemd[1]: runc-cgroups-integration-test-12869.scope: Scope reached runtime time limit. Stopping.
Jun 13 14:41:38 kir-rhat systemd[1]: runc-cgroups-integration-test-12869.scope: Stopping timed out. Killing.
Jun 13 14:41:38 kir-rhat systemd[1]: runc-cgroups-integration-test-12869.scope: Killing process 1107438 (sleep) with signal SIGKILL.
Jun 13 14:41:38 kir-rhat systemd[1]: runc-cgroups-integration-test-12869.scope: Failed with result 'timeout'.
runc delete test-failed-unit (status=0):
Unit runc-cgroups-integration-test-12869.scope could not be found.
----
Before the fix, the test was failing like this:
----
delete.bats
✗ runc delete removes failed systemd unit
(in test file tests/integration/delete.bats, line 194)
`run -4 systemctl status "$SD_UNIT_NAME"' failed, expected exit code 4, got 3
....
----
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
runc delete is supposed to remove all the container's artefacts.
In case systemd cgroup driver is used, and the systemd unit has failed
(e.g. oom-killed), systemd won't remove the unit (that is, unless the
"CollectMode: inactive-or-failed" property is set).
Call reset-failed from manager.Destroy so the failed unit will be
removed during "runc delete".
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sometimes we call resetFailedUnit as a cleanup measure, and we don't
care if it fails or not. So, move error reporting to its callers, and
ignore error in cases we don't really expect it to succeed.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
There is no such thing as linux.resources.memorySwap (the mem+swap is
set as linux.resources.memory.swap).
As it is not used in this test anyway, remove it.
Fixes: 4929c05ad1
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Aleksa Sarai