Aleksa Sarai (21):
rootfs: re-allow dangling symlinks in mount targets
openat2: improve resilience on busy systems
selinux: use safe procfs API for labels
rootfs: switch to fd-based handling of mountpoint targets
libct/system: use securejoin for /proc/$pid/stat
init: use securejoin for /proc/self/setgroups
init: write sysctls using safe procfs API
utils: remove unneeded EnsureProcHandle
utils: use safe procfs for /proc/self/fd loop code
apparmor: use safe procfs API for labels
rootfs: avoid using os.Create for new device inodes
internal: add wrappers for securejoin.Proc*
go.mod: update to github.com/cyphar/filepath-securejoin@v0.5.0
console: verify /dev/pts/ptmx before use
console: avoid trivial symlink attacks for /dev/console
console: add fallback for pre-TIOCGPTPEER kernels
console: use TIOCGPTPEER when allocating peer PTY
*: switch to safer securejoin.Reopen
internal: move utils.MkdirAllInRoot to internal/pathrs
internal/sys: add VerifyInode helper
internal: linux: add package doc-comment
Li Fubang (1):
libct: align param type for mountCgroupV1/V2 functions
Kir Kolyshkin (3):
libct: maskPaths: don't rely on ENOTDIR for mount
libct: maskPaths: only ignore ENOENT on mount dest
libct: add/use isDevNull, verifyDevNull
Fixes: CVE-2025-31133 GHSA-9493-h29p-rfm2
Fixes: CVE-2025-52565 GHSA-qw9x-cqr3-wc7r
Fixes: CVE-2025-52881 GHSA-cgrx-mc8f-2prm
Reported-by: Lei Wang <ssst0n3@gmail.com>
Reported-by: Li Fubang <lifubang@acmcoder.com>
Reported-by: Tõnis Tiigi <tonistiigi@gmail.com>
Reported-by: Aleksa Sarai <cyphar@cyphar.com>
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>
Previously, we would see a ~3% failure rate when starting containers
with mounts that contain ".." (which can trigger -EAGAIN). To counteract
this, filepath-securejoin v0.5.1 includes a bump of the internal retry
limit from 32 to 128, which lowers the failure rate to 0.12%.
However, there is still a risk of spurious failure on regular systems.
In order to try to provide more resilience (while avoiding DoS attacks),
this patch also includes an additional retry loop that terminates based
on a deadline rather than retry count. The deadline is 2ms, as my
testing found that ~800us for a single pathrs operation was the longest
latency due to -EAGAIN retries, and that was an outlier compared to the
more common ~400us latencies -- so 2ms should be more than enough for
any real system.
The failure rates above were based on more 50k runs of runc with an
attack script (from libpathrs) running a rename attack on all cores of a
16-core system, which is arguably a worst-case but heavily utilised
servers could likely approach similar results.
Tested-by: Phil Estes <estesp@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
All the new code appears in main (not in the release branches),
and we only want extra linter rules to apply to new code.
Disable lint-extra job if the PR is not to the main branch.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 1c4dba693f)
Signed-off-by: lifubang <lifubang@acmcoder.com>
Due to the sensitive nature of these fixes, it was not possible to
submit these upstream and vendor the upstream library. Instead, this
patch uses a fork of github.com/opencontainers/selinux, branched at
commit opencontainers/selinux@879a755db5.
In order to permit downstreams to build with this patched version, a
snapshot of the forked version has been included in
internal/third_party/selinux. Note that since we use "go mod vendor",
the patched code is usable even without being "go get"-able. Once the
embargo for this issue is lifted we can submit the patches upstream and
switch back to a proper upstream go.mod entry.
Also, this requires us to temporarily disable the CI job we have that
disallows "replace" directives.
Fixes: GHSA-cgrx-mc8f-2prm CVE-2025-52881
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>
In order to avoid lint errors due to the deprecation of the top-level
securejoin methods ported from libpathrs, we need to adjust
internal/pathrs to use the new pathrs-lite subpackage instead.
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>
This will be used for a few security patches in later patches in this
patchset. The need to verify what kind of inode we are operating on in a
race-free way turns out to be quite a common pattern...
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This is necessary for the pre-1.4 backports because internal/linux was
not present and the linters get angry when a new package without a doc
comment gets added.
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>
Sometimes we need to run runc through some wrapper (like nohup), but
because "__runc" and "runc" are bash functions in our test suite this
doesn't work trivially -- and you cannot just pass "$RUNC" because you
you need to set --root for rootless tests.
So create a setup_runc_cmdline helper which sets $RUNC_CMDLINE to the
beginning cmdline used by __runc (and switch __runc to use that).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(Cherry-pick of commit d1f6acfab06e6f5eb15b7edfaa704f50907907b1.)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
"runc" was a special wrapper around bats's "run" which output some very
useful diagnostic information to the bats log, but this was not usable
for other commands. So let's make it a more generic helper that we can
use for other commands.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(Cherry-pick of commit ea385de40c9a006737399bc72918a19e5d038736.)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The issue on arm [1] is now fixed, so let's get back to using the
packaged criu version for most of the CI matrix.
This reverts commit 105674844e
("ci: use criu built from source on gha arm").
[1]: https://github.com/checkpoint-restore/criu/issues/2709
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(Cherry-picked from commit 96f4a90a6b1ca9e3f2011ebaeffb7dc52db2ca32.)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Currently, criu package from opensuse build farm times out on GHA arm,
so let's only use criu-dev (i.e. compiled from source on CI machine).
Once this is fixed, this patch can be reverted.
Related to criu issue 2709.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(Cherry-picked from commit 105674844eaaf24bf14135ef0c64703e511882ab.)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Since GHA now provides ARM, we can switch away from actuated.
Many thanks to @alexellis (@self-actuated) for being the sponsor of this
project.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(Cherry-picked from commit 1cf096803abb770c414ce0a1e2e0be283b09001d.)
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Prevent --l3-cache-schema from clearing the intel_rdt.memBwSchema state
and --mem-bw-schema clearing l3_cache_schema, respectively.
Signed-off-by: Markus Lehtonen <markus.lehtonen@intel.com>
(cherry picked from commit 57b6a317bb)
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>
Aleksa Sarai