Since the recent CVE fixes, TestFdLeaksSystemd sometimes fails:
=== RUN TestFdLeaksSystemd
exec_test.go:1750: extra fd 9 -> /12224/task/13831/fd
exec_test.go:1753: found 1 extra fds after container.Run
--- FAIL: TestFdLeaksSystemd (0.10s)
It might have been caused by the change to the test code in commit
ff6fe13 ("utils: use safe procfs for /proc/self/fd loop code") -- we are
now opening a file descriptor during the logic to get a list of file
descriptors. If the file descriptor happens to be allocated to a
different number, you'll get an error.
Let's try to filter out the fd used to read a directory.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 5fbc3bb019)
Signed-off-by: lifubang <lifubang@acmcoder.com>
This was always the intended behaviour but commit 72fbb34f50 ("rootfs:
switch to fd-based handling of mountpoint targets") regressed it when
adding a mechanism to create a file handle to the target if it didn't
already exist (causing the later stat to always succeed).
A lot of people depend on this functionality, so add some tests to make
sure we don't break it in the future.
Fixes: 72fbb34f50 ("rootfs: switch to fd-based handling of mountpoint targets")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit 9a9719eeb4)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
On MIPS arches, Rdev is uint32 so we have to convert it.
Fixes issue 4962.
Fixes: 8476df83 ("libct: add/use isDevNull, verifyDevNull")
Fixes: de87203e ("console: verify /dev/pts/ptmx before use")
Fixes: 398955bc ("console: add fallback for pre-TIOCGPTPEER kernels")
Reported-by: Tianon Gravi <admwiggin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 1b954f1f06)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
It seems there are a fair few images where dangling symlinks are used as
path components for mount targets, which pathrs-lite does not support
(and it would be difficult to fully support this in a race-free way).
This was actually meant to be blocked by commit 63c2908164 ("rootfs:
try to scope MkdirAll to stay inside the rootfs"), followed by commit
dd827f7b71 ("utils: switch to securejoin.MkdirAllHandle"). However, we
still used SecureJoin to construct mountpoint targets, which means that
dangling symlinks were "resolved" before reaching pathrs-lite.
This patch basically re-adds this hack in order to reduce the breakages
we've seen so far.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
An attacker could race with us during mount configuration in order to
trick us into mounting over an unexpected path. This would bypass
checkProcMount() and would allow for security profiles to be left
unapplied by mounting over /proc/self/attr/... (or even more serious
outcomes such as killing the entire system by tricking runc into writing
strings to /proc/sysrq-trigger).
This is a larger issue with our current mount infrastructure, and the
ideal solution would be to rewrite it all to be fd-based (which would
also allow us to support the "new" mount API, which also avoids a bunch
of other issues with mount(8)). However, such a rewrite is not really
workable as a security fix, so this patch is a bit of a compromise
approach to fix the issue while also moving us a bit towards that
eventual end-goal.
The core issue in CVE-2025-52881 is that we currently use the (insecure)
SecureJoin to re-resolve mountpoint target paths multiple times during
mounting. Rather than generating a string from createMountpoint(), we
instead open an *os.File handle to the target mountpoint directly and
then operate on that handle. This will make it easier to remove
utils.WithProcfd() and rework mountViaFds() in the future.
The only real issue we need to work around is that we need to re-open
the mount target after doing the mount in order to get a handle to the
mountpoint -- pathrs.Reopen() doesn't work in this case (it just
re-opens the inode under the mountpoint) so we need to do a naive
re-open using the full path. Note that if we used move_mount(2) this
wouldn't be a problem because we would have a handle to the mountpoint
itself.
Note that this is still somewhat of a temporary solution -- ideally
mountViaFds would use *os.File directly to let us avoid some other
issues with using bare /proc/... paths, as well as also letting us more
easily use the new mount API on modern kernels.
Fixes: GHSA-cgrx-mc8f-2prm CVE-2025-52881
Co-developed-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
sysctls could in principle also be used as a write gadget for arbitrary
procfs files. As this requires getting a non-subset=pid /proc handle we
amortise this by only allocating a single procfs handle for all sysctl
writes.
Fixes: GHSA-cgrx-mc8f-2prm CVE-2025-52881
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
All of the callers of EnsureProcHandle now use filepath-securejoin's
ProcThreadSelf to get a file handle, which has much stricter
verification to avoid procfs attacks than EnsureProcHandle's very
simplistic filesystem type check.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
From a safety perspective this might not be strictly required, but it
paves the way for us to remove utils.ProcThreadSelf.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
EnsureProcHandle only protects us against a tmpfs mount, but the risk of
a procfs path being used (such as /proc/self/sched) has been known for a
while. Now that filepath-securejoin has a reasonably safe procfs API,
switch to it.
Fixes: GHSA-cgrx-mc8f-2prm CVE-2025-52881
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
If an attacker were to make the target of a device inode creation be a
symlink to some host path, os.Create would happily truncate the target
which could lead to all sorts of issues. This exploit is probably not as
exploitable because device inodes are usually only bind-mounted for
rootless containers, which cannot overwrite important host files (though
user files would still be up for grabs).
The regular inode creation logic could also theoretically be tricked
into changing the access mode and ownership of host files if the
newly-created device inode was swapped with a symlink to a host path.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This is primarily done out of an abudance of caution against runc exec
being attacked by a container where /dev/pts/ptmx has been replaced with
some other bad inode (a disconnected NFS handle, a symlink that goes
through a leaked runc file descriptor to reference a host ptmx, etc).
Unfortunately, we cannot trivially verify that /dev/pts/ptmx is actually
the /dev/pts from the container without storing stuff like the fsid in
the runc state.json, which is probably not worth the extra effort. This
should at least avoid the most concerning cases.
Reported-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
An attacker could make /dev/console a symlink. This presents two
possible issues:
1. os.Create will happily truncate targets, which could have resulted
in a worse version of CVE-2024-4531. Luckily, this all happens after
pivot_root(2) so the scope of that particular attack is fairly
limited (you are unlikely to be able to easily access host rootfs
files -- though it might be possible to take advantage of leaks such
as in CVE-2024-21626). However, O_CREAT|O_NOFOLLOW is what we should
be doing for all file creations.
2. Because we passed /dev/console as the only mount path (as opposed to
using a /proc/self/fd/$n path), an attacker could swap the symlink
to point to any other path and thus cause us to mount over some
other path. This is not as big of a problem because all the mounts
are in the container namespace after pivot_root(2), and users
usually can create arbitrary mount targets inside the container.
These issues don't seem particularly exploitable, but they deserve to be
hardened regardless.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The pty driver has very consistent allocation rules for the major:minor
numbers of /dev/pts/$n inodes, so it is possible to somewhat safely open
/dev/pts/* paths if we validate that the inode is the one we expect.
It is possible for an attacker to have over-mounted a pts peer from a
different devpts instance, but to fix this would require more tracking
of devpts instances than runc currently can do.
This means runc should continue to work on very old kernels.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
When opening the peer end of a pty, the old kernel API required us to
open /dev/pts/$num inside the container (at least since we fixed console
handling many years ago in commit 244c9fc426 ("*: console rewrite")).
The problem is that in a hostile container it is possible for
/dev/pts/$num to be an attacker-controlled symlink that runc can be
tricked into resolving when doing bind-mounts. This allows the attacker
to (among other things) persist /proc/... entries that are later masked
by runc, allowing an attacker to escape through the kernel.core_pattern
sysctl (/proc/sys/kernel/core_pattern). This is the original issue
reported by Lei Wang and Li Fu Bang in CVE-2025-52565.
However, it should be noted that this is not entirely a newly-discovered
problem. Way back in Linux 4.13 (2017), I added the TIOCGPTPEER ioctl,
which allows us to get a pty peer without touching the /dev/pts inside
the container. The original threat model was around an attacker
replacing /dev/pts/$n or /dev/pts/ptmx with some malicious inode (a DoS
inode, or possibly a PTY they wanted a confused deputy to operate on).
Unfortunately, there was no practical way for runc to cache a safe
O_PATH handle to /dev/pts/ptmx (unlike other runtimes like LXC, which
switched to TIOCGPTPEER way back in 2017). Since it wasn't clear how we
could protect against the main attack TIOCGPTPEER was meant to protect
against, we never switched to it (even though I implemented it
specifically to harden container runtimes).
Unfortunately, It turns out that mount *sources* are a threat we didn't
fully consider. Since TIOCGPTPEER already solves this problem entirely
for us in a race free way, we should just use that. In a later patch, we
will add some hardening for /dev/pts/$num opening to maintain support
for very old kernels (Linux 4.13 is very old at this point, but RHEL 7
is still kicking and is stuck on Linux 3.10).
Fixes: GHSA-qw9x-cqr3-wc7r CVE-2025-52565
Reported-by: Lei Wang <ssst0n3@gmail.com> (CVE-2025-52565)
Reported-by: lfbzhm <lifubang@acmcoder.com> (CVE-2025-52565)
Reported-by: Aleksa Sarai <cyphar@cyphar.com> (TIOCGPTPEER)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Currently, we rely on mount returning ENOTDIR when the destination is a
directory (and so mount tells us that the source is not), and fall back
to read-only tmpfs bind mount for such cases.
Theoretically, ENOTDIR can also be returned in some other cases,
resulting in the wrong type of mount being used.
Let's be more straightforward here -- call fstat on destination file
descriptor, and use the proper mount depending on whether it is a
directory.
Reported-by: Rodrigo Campos <rodrigoca@microsoft.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When mounting a path being masked, the /dev/null might disappear from
under us, and mount (even on an opened /dev/null file descriptor) will
return ENOENT, which we deliberately ignore, as there's no need to mask
non-existent paths.
Let's open the destination path and ignore ENOENT during open, then
mount via the destination file descriptor, not ignoring ENOENT.
Reported-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The /dev/null in a container should not be trusted, because when /dev
is a bind mount, /dev/null is not created by runc itself.
1. Add isDevNull which checks the fd minor/major and device type,
and verifyDevNull which does the stat and the check.
2. Rewrite maskPath to open and check /dev/null, and use its fd to
perform mounts. Move the loop over the MaskPaths into the function,
and rename it to maskPaths.
3. reOpenDevNull: use verifyDevNull and isDevNull.
4. fixStdioPermissions: use isDevNull instead of stat.
Fixes: GHSA-9493-h29p-rfm2 CVE-2025-31133
Co-authored-by: Rodrigo Campos <rodrigoca@microsoft.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
filepath-securejoin v0.3 gave us a much safer re-open primitive, we
should use it to avoid any theoretical attacks. Rather than using it
direcly, add a small pathrs wrapper to make libpathrs migrations in the
future easier...
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
We will have more wrappers around filepath-securejoin, and so move them
to their own specific package so that we can eventually use libpathrs
fairly cleanly (by swapping out the implementation).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Instead of generating a list of tmpfs mount and have a special function
to check whether the path is in the list, let's go over the list of
mounts directly. This simplifies the code and improves readability.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit ce3cd4234c)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since its code is now trivial, and it is only called from a single
place, it does not make sense to have it as a separate function.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit f91fbd34d9)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It makes sense to ignore cgroup mounts much early in the code,
saving some time on unnecessary operations.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit b8aa5481db)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Replace the big "if !" block with the if block and continue,
simplifying the code flow.
2. Move comments closer to the code, improving readability.
This commit is best reviewed with --ignore-all-space or similar.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 0c93d41c65)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In certain deployments, it's possible for runc to be spawned by a
process with a restrictive cpumask (such as from a systemd unit with
CPUAffinity=... configured) which will be inherited by runc and thus the
container process by default.
The cpuset cgroup used to reconfigure the cpumask automatically for
joining processes, but kcommit da019032819a ("sched: Enforce user
requested affinity") changed this behaviour in Linux 6.2.
The solution is to try to emulate the expected behaviour by resetting
our cpumask to correspond with the configured cpuset (in the case of
"runc exec", if the user did not configure an alternative one). Normally
we would have to parse /proc/stat and /sys/fs/cgroup, but luckily
sched_setaffinity(2) will transparently convert an all-set cpumask (even
if it has more entries than the number of CPUs on the system) to the
correct value for our usecase.
For some reason, in our CI it seems that rootless --systemd-cgroup
results in the cpuset (presumably temporarily?) being configured such
that sched_setaffinity(2) will allow the full set of CPUs. For this
particular case, all we care about is that it is different to the
original set, so include some special-casing (but we should probably
investigate this further...).
Reported-by: ningmingxiao <ning.mingxiao@zte.com.cn>
Reported-by: Martin Sivak <msivak@redhat.com>
Reported-by: Peter Hunt <pehunt@redhat.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(Cherry-pick of commit 121192ade6c55f949d32ba486219e2b1d86898b2.)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This was added in 2ee9cbbd12 ("It's /proc/stat, not /proc/stats") with
no actual justification, and doesn't really make much sense on further
inspection:
* /proc/net is a symlink to "self/net", which means that /proc/net/dev
is a per-process file, and so overmounting it would only affect pid1.
Any other program that cares about /proc/net/dev would see their own
process's configuration, and unprivileged processes wouldn't be able
to see /proc/1/... data anyway.
In addition, the fact that this is a symlink means that runc will
deny the overmount because /proc/1/net/dev is not in the proc
overmount allowlist. This means that this has not worked for many
years, and probably never worked in the first place.
* /proc/self/net is already namespaced with network namespaces, so the
primary argument for allowing /proc overmounts (lxcfs-like masking of
procfs files to emulate namespacing for files that are not properly
namespaced for containers -- such as /proc/cpuinfo) is moot.
It goes without saying that lxcfs has never overmounted
/proc/self/net/... files, so the general "because lxcfs"
justification doesn't hold water either.
* The kernel has slowly been moving towards blocking overmounts in
/proc/self/. Linux 6.12 blocked overmounts for fd, fdinfo, and
map_files; future Linux versions will probably end up blocking
everything under /proc/self/.
Fixes: 2ee9cbbd12 ("It's /proc/stat, not /proc/stats")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry-picked from commit 3620185d06b79da836559b75161027c6273fff7b.)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
While debugging an issue involving failing mounts, I discovered that
just returning the plain mount error message when we are in the fallback
code for handling locked mounts leads to unnecessary confusion.
It also doesn't help that podman currently forcefully sets "rw" on
mounts, which means that rootless containers are likely to hit the
locked mounts issue fairly often.
So we should improve our error messages to explain why the mount is
failing in the locked flags case.
Fixes: 7c71a22705 ("rootfs: remove --no-mount-fallback and finally fix MS_REMOUNT")
(cherry picked from commit 58c3ab77b0)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
When reading mount errors, it is quite hard to make sense of mount flags
in their hex form. As this is the error path, the minor performance
impact of constructing a string is probably not worth hyper-optimising.
(cherry pick from commit 30302a2850)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Since v3.14, CRIU always restores processes into a time namespace to
prevent backward jumps of monotonic and boottime clocks. This change
updates the container configuration to ensure that `runc exec` launches
new processes within the container's time namespace.
Fixes#2610
Signed-off-by: Andrei Vagin <avagin@gmail.com>
(cherry picked from commit b68cbdff34)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
lifubang (2):
libct: don't send config to nsexec when joining an existing timens
test: exec into a container with private time ns
LGTMs: cyphar lifubang
The previous logic caused runc to hang if CloseExecFrom returned an
error, as the defer waiting on logsDone never finished as the parent
process was never started (and it controls the closing of logsDone via
it's logsPipe).
This moves the defer to after we have started the parent, with means all
the logic related to managing the logsPipe should also be running.
Signed-off-by: Evan Phoenix <evan@phx.io>
(cherry picked from commit 7b26da9ee3)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Prior to kernel Linux 5.5, F_SEAL_FUTURE_WRITE has a bug which maps
memory as shared between processes even if it is set as private. See
kernel commit 05d351102dbe ("mm, memfd: fix COW issue on MAP_PRIVATE and
F_SEAL_FUTURE_WRITE mappings") for more details.
According to the fcntl(2) man pages, F_SEAL_WRITE is enough:
> Furthermore, trying to create new shared, writable memory-mappings via
> mmap(2) will also fail with EPERM.
>
> Using the F_ADD_SEALS operation to set the F_SEAL_WRITE seal fails
> with EBUSY if any writable, shared mapping exists. Such mappings must
> be unmapped before you can add this seal.
F_SEAL_FUTURE_WRITE only makes sense if a read-write shared mapping in
one process should be read-only in another process. This is not case for
runc, especially not for the /proc/self/exe we are protecting.
Signed-off-by: Tomasz Duda <tomaszduda23@gmail.com>
(cyphar: improve the comment regarding F_SEAL_FUTURE_WRITE)
(cyphar: improve commit message)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit c43ea7d629)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Retry Recvfrom, Sendmsg, Readmsg, and Read as they can return EINTR.
Signed-off-by: Evan Phoenix <evan@phx.io>
(cherry picked from commit 28475f12e3)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
We should configure the process's timens offset only when we need to
create new time namespace, we shouldn't do it if we are joining an
existing time namespace. (#4635)
Signed-off-by: lifubang <lifubang@acmcoder.com>
(cherry picked from commit ad09197e41)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This helper was added for runc-dmz in commit dac417174, but runc-dmz was
later removed in commit 871057d, which forgot to remove the helper.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 83350c24a9)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes k3s cross-compilation on Windows, broken by commit
1912d5988b ("*: actually support joining a userns with a new
container").
[@kolyshkin: commit message]
Fixes: 1912d5988b
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit ccb589bd7d)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This release includes a minor breaking API change that requires us to
rework the types of our wrappers, but there is no practical behaviour
change.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit 70e500e7d1)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since v1.2.0 was released, a number of users complained that the removal
of tun/tap device access from the default device ruleset is causing a
regression in their workloads.
Additionally, it seems that some upper-level orchestration tools
(Docker Swarm, Kubernetes) makes it either impossible or cumbersome
to supply additional device rules.
While it's probably not quite right to have /dev/net/tun in a default
device list, it was there from the very beginning, and users rely on it.
Let's keep it there for the sake of backward compatibility.
This reverts commit 2ce40b6ad7.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(Cherry-pick of commit 394f4c3b7012674ebe0232c560713e57cbd653e6.)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(This is a cherry-pick of c0044c7aa403ecf2d9172bd9386d05433b011076.)
If we get an unexpected error here, it is probably because of a library
or kernel change that could cause our detection logic to be invalid. As
a result, these warnings should be louder so users have a chance to tell
us about them sooner (or so we might notice them before doing a release,
as happened with the 1.2.0 regression).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Kir Kolyshkin