Aleksa Sarai (22):
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
ci: add lint to forbid the usage of os.Create
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>
openSUSE has an unfortunate default udev setup which forcefully sets all
loop devices to use the "none" scheduler, even if you manually set it.
As this is a property of the host configuration (and udev is monitoring
from the host) we cannot really change this behaviour from inside our
test container.
So we should just skip the test in this (hopefully unusual) case.
Ideally tools running the test suite should disable this behaviour when
running our test suite.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit e6b4b5a128)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
If an error occurs during a test which sets up loopback devices, the
loopback device is not freed. Since most systems have very conservative
limits on the number of loopback devices, re-running a failing test
locally to debug it often ends up erroring out due to loopback device
exhaustion.
So let's just move the "losetup -d" to teardown, where it belongs.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(cherry picked from commit ceef984fb3)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Apparently, having a minor of 0 does not always mean it's the
whole device (not a partition):
=== /proc/partitions (using major: 259) ===
major minor #blocks name
8 16 78643200 sdb
8 17 77593583 sdb1
8 30 4096 sdb14
8 31 108544 sdb15
259 0 934912 sdb16
8 0 78643200 sda
8 1 78641152 sda1
Rewrite the test to not assume minor is 0, and use
lsblk -d to find out whole devices.
This fixes a test case which was added in commit 7696402da.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 46dac589c1)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case there's a duplicate in the device list, the latter entry
overrides the former one.
So, we need to modify the last entry, not the first one. To do that,
use slices.Backward.
Amend the test case to test the fix.
Reported-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 0b01dccfbb)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This support was missing from runc, and thus the example from the
podman-update wasn't working.
To fix, introduce a function to either update or insert new weights and iops.
Add integration tests.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 7696402dac)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.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>
Since gofumpt v0.9.0 there's a new formatting rule to "clothe" any naked
returns.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit b2f8a74de5)
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>
os.Create is shorthand for open(O_CREAT|O_TRUNC) *without* O_EXCL, which
is incredibly unsafe for us to do when interacting with a container
rootfs (especially before pivot_root) as an attacker could swap the
target path with a symlink that points to the host filesystem, causing
us to delete the contents of or create host files.
We did have a similar bug in CVE-2024-45310, but in that case we
(luckily) didn't have O_TRUNC set which avoided the worst possible case.
However, os.Create does set O_TRUNC and we were using it in scenarios
that may have been exploitable.
Because of how easy it us for us to accidentally introduce this kind of
bug, we should simply not allow the usage of os.Create in our entire
codebase.
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>
(Backported by Jared Ledvina.)
dependabot[bot]:
build(deps): bump github.com/opencontainers/cgroups from 0.0.3 to 0.0.4
Kir Kolyshkin (4):
deps: bump cgroups to v0.0.3, fix tests
libct: State: ensure Resources is not nil
deps: bump opencontainers/cgroups to v0.0.2
tests/int: simplify using check_cpu_quota
LGTMs: kolyshkin cyphar
Since opencontainers/cgroups v0.0.2 (commit b206a015), all stuct
Resources fields are annotated with "omitempty" attribute.
As a result, the loaded configuration may have Resources == nil.
It is totally OK (rootless containers may have no resources configured)
except since commit 6c5441e5, cgroup v1 fs manager requires Resources to
be set in the call to NewManager (this is a cgroup v1 deficiency,
or maybe our implementation deficiency, or both).
To work around this, let's add code to ensure Resources is never nil
after loading from state.json.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Instead of providing systemd CPU quota value (CPUQuotaPerSec),
calculate it based on how opencontainers/cgroups/systemd handles
it (see addCPUQuota).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When a non–page-aligned value is written to memory.max, the kernel aligns it
down to the nearest page boundary. On systems with a page size greater
than 4K (e.g., 64K), this caused failures because the configured
memory.max value was not 64K aligned.
This patch fixes the issue by explicitly aligning the memory.max value
to 64K. Since 64K is also a multiple of 4K, the value is correctly
aligned on both 4K and 64K page size systems.
However, this approach will still fail on systems where the hardcoded
memory.max value is not aligned to the system page size.
Fixes: https://github.com/opencontainers/runc/issues/4841
Signed-off-by: Vishal Chourasia <vishalc@linux.ibm.com>
Signed-off-by: Donet Tom <donettom@linux.ibm.com>
(cherry picked from commit 830c479ae2)
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Aleksa Sarai