[1.4] libct: reuse tmpfs for directory masks by lifubang · Pull Request #5281 · opencontainers/runc

lifubang · 2026-05-13T15:54:18Z

Backport #5190, #5275, and #5285

#5190

A lot of filesystem-related stuff happens inside the container root
directory, and we have used its name before. It makes sense to pre-open
it and use a *os.File handle instead.

Function names in internal/pathrs are kept as is for simplicity (and it
is an internal package), but they now accept root as *os.File.

This is a somewhat naive attempt at it, but it is surprisingly simple.

#5275

This PR carries forward #5262.

Wondering if a problem that showed up recently in k8s when we added more masked paths can be handled better in runc itself? please see details below:

Kubernetes may add one sysfs thermal_throttle entry per CPU to maskedPaths. On large Intel systems this can produce many directory masks for a single container. runc currently handles each directory mask with a separate read-only tmpfs mount, and therefore a separate tmpfs superblock.

On Linux 4.18/RHEL 8 kernels, creating and tearing down many tmpfs superblocks can contend on the global shrinker_rwsem when containers start or stop concurrently.

Use one read-only tmpfs for directory masks and bind-mount it over the remaining directory targets. The first non-procfs-fd directory mount is reopened through the container root fd before it is reused. File masks still bind /dev/null, and procfs fd targets keep the existing one-tmpfs-per-target behavior because they are fd aliases rather than stable rootfs paths.

The bind mounts do not create additional tmpfs superblocks. They also retain the read-only mount flag inherited from the source vfsmount, so the masking semantics remain unchanged.

xref: kubernetes/kubernetes#138512
xref: kubernetes/kubernetes#138388
xref: kubernetes/kubernetes#131018

(With some assistance from claude/codex)

This PR is a follow-up to #5275.

In that change, we started reusing a single tmpfs mount to mask multiple directories. While this optimization works well when masking more than one path, it introduces unnecessary overhead when only a single directory needs to be masked.

This change restores the original behavior for single-path masking while preserving the shared tmpfs logic for multiple paths.

kolyshkin · 2026-05-13T18:12:08Z

Error: libcontainer/rootfs_linux.go:414:21: cannot use c.root (variable of type string) as *os.File value in argument to maskPaths
Error: libcontainer/rootfs_linux.go:1395:40: cannot use rootFd (variable of type *os.File) as string value in argument to reopenAfterMount

Either don't backport to 1.4, or this needs to be reworked

lifubang · 2026-05-14T14:35:26Z

this needs to be reworked

We also need to backport #5190.

Indeed, it does not make sense to prepend c.root once we started using MkdirAllInRoot in commit 63c2908. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 6035252) Signed-off-by: lifubang <lifubang@acmcoder.com>

No change in functionality, just a preparation for the next patch. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 78b8067) Signed-off-by: lifubang <lifubang@acmcoder.com>

A lot of filesystem-related stuff happens inside the container root directory, and we have used its name before. It makes sense to pre-open it and use a *os.File handle instead. Function names in internal/pathrs are kept as is for simplicity (and it is an internal package), but they now accept root as *os.File. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 28cb321) Signed-off-by: lifubang <lifubang@acmcoder.com>

This uses preopened rootfs in Chdir and pivotRoot. While at it, add O_PATH when opening oldroot in pivotRoot. Co-authored-by: Kir Kolyshkin <kolyshkin@gmail.com> Signed-off-by: lfbzhm <lifubang@acmcoder.com> Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 5b094ed) Signed-off-by: lifubang <lifubang@acmcoder.com>

Co-authored-by: Davanum Srinivas <davanum@gmail.com> Refactored-by: lifubang <lifubang@acmcoder.com> Signed-off-by: lifubang <lifubang@acmcoder.com> (cherry picked from commit abf70ba) Signed-off-by: lifubang <lifubang@acmcoder.com>

Previously, masked directories (e.g., /proc/acpi, /proc/scsi) were mounted as read-only tmpfs without explicit size or inode limits. Although these mounts are meant to be empty and unwritable, the lack of resource constraints means that—should an attacker bypass the read-only protection (e.g., via container escape, mount namespace manipulation, or a kernel vulnerability)—the tmpfs could consume up to 50% of system memory by default (the kernel's default tmpfs limit). To mitigate this risk in high-density container environments and adhere to the principle of least privilege, we now explicitly set: - nr_blocks=1 (sufficient for at most one block size) - nr_inodes=1 (sufficient for at most one inode) Ref: https://man7.org/linux/man-pages/man5/tmpfs.5.html These limits ensure that even if compromised, kernel memory usage remains strictly bounded and negligible. This change aligns with best practices used by other container runtimes and strengthens defense-in-depth for sensitive masked paths. Co-authored-by: Davanum Srinivas <davanum@gmail.com> Refactored-by: lifubang <lifubang@acmcoder.com> Signed-off-by: lifubang <lifubang@acmcoder.com> (cherry picked from commit e57a7a4) Signed-off-by: lifubang <lifubang@acmcoder.com>

Kubernetes may add one sysfs thermal_throttle entry per CPU to maskedPaths. On large Intel systems this can produce many directory masks for a single container. runc currently handles each directory mask with a separate read-only tmpfs mount, and therefore a separate tmpfs superblock. On Linux 4.18/RHEL 8 kernels, creating and tearing down many tmpfs superblocks can contend on the global shrinker_rwsem when containers start or stop concurrently. Use one read-only tmpfs for directory masks and bind-mount it over the remaining directory targets. The first non-procfs-fd directory mount is reopened through the container root fd before it is reused. File masks still bind /dev/null, and procfs fd targets keep the existing one-tmpfs-per-target behaviour because they are fd aliases rather than stable rootfs paths. If the bind-mount of the shared source fails (e.g. due to kernel restrictions), fall back to individual tmpfs mounts for all remaining directories. Tmpfs mounts use nr_blocks=1,nr_inodes=1 to minimise kernel resource usage. The bind mounts do not create additional tmpfs superblocks. They also retain the read-only mount flag inherited from the source vfsmount, so the masking semantics remain unchanged. xref: kubernetes/kubernetes#138512 xref: kubernetes/kubernetes#138388 xref: kubernetes/kubernetes#131018 Co-authored-by: Davanum Srinivas <davanum@gmail.com> Refactored-by: lifubang <lifubang@acmcoder.com> Signed-off-by: lifubang <lifubang@acmcoder.com> (cherry picked from commit c046c9b) Signed-off-by: lifubang <lifubang@acmcoder.com>

Co-authored-by: Davanum Srinivas <davanum@gmail.com> Signed-off-by: lifubang <lifubang@acmcoder.com> (cherry picked from commit 124772f) Signed-off-by: lifubang <lifubang@acmcoder.com>

This is a follow-up to opencontainers#5275. That change reused a single tmpfs mount to mask multiple directories, which is efficient when masking more than one path. However, it introduced unnecessary overhead when only one directory is masked. This commit restores the original behavior for the single-path case while preserving shared tmpfs logic for multiple paths. Signed-off-by: lifubang <lifubang@acmcoder.com> (cherry picked from commit e7e2f00) Signed-off-by: lifubang <lifubang@acmcoder.com>

Close the root file descriptor immediately after use in maskPaths to reduce the window during which an attacker could potentially exploit an open fd to access or manipulate the root filesystem. This follows the principle of least privilege and mitigates risks in compromised or malicious container scenarios. Co-authored-by: Kir Kolyshkin <kolyshkin@gmail.com> Signed-off-by: lifubang <lifubang@acmcoder.com> (cherry picked from commit b88635e) Signed-off-by: lifubang <lifubang@acmcoder.com>

rata

Not sure if I found a bug in the original PR (the bindFailed carry-over in iterations). The rest is more cosmetical or me wanting to understand more. Sorry I didn't review the orignal PRs

rata · 2026-05-21T09:15:35Z

+				err = mountViaFds("", sharedMaskSrc, path, dstFd, "", unix.MS_BIND, "")
+				if err != nil {
+					// A bind-mount inherits MNT_READONLY from the source vfsmount,
+					// but if it fails fall back to individual tmpfs mounts.
+					bindFailed = true
+					logrus.WithError(err).Warn("maskPaths: shared tmpfs bind-mount failed, falling back to per-directory tmpfs")


Why can this fail and we should care about a fallback instead of returning an error?

To be honest, I have no idea why this bind mount might have failed -- but treating it as a recoverable scenario (with a fallback) seems like a feature, not a flaw. That’s the robust!

I mean, if this can fail and the fallback works in some real-world scenario, I'm okay with this. Otherwise it is just dead-code and added complexity for no real good.

It seems crun also has the fallback, so maybe it is ok... I'm not convinced, but okay :D

rata

LGTM

kolyshkin

SGTM (already reviewed the original PRs, not really looking at this backport)

lifubang added the backport/1.4-pr A backport PR to release-1.4 label May 13, 2026

kolyshkin mentioned this pull request May 13, 2026

[1.3] libct: reuse tmpfs for directory masks #5282

Merged

lifubang force-pushed the backport-5275-1.4 branch from db43111 to a613bb8 Compare May 14, 2026 14:32

lifubang marked this pull request as draft May 15, 2026 03:42

kolyshkin added this to the 1.4.3 milestone May 18, 2026

kolyshkin and others added 10 commits May 20, 2026 10:21

libct: mountCgroupV1: address TODO

a4afb37

Indeed, it does not make sense to prepend c.root once we started using MkdirAllInRoot in commit 63c2908. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 6035252) Signed-off-by: lifubang <lifubang@acmcoder.com>

libct: minor refactor in mountToRootfs

63857e5

No change in functionality, just a preparation for the next patch. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 78b8067) Signed-off-by: lifubang <lifubang@acmcoder.com>

libct: skip mount for duplicate masked paths

b9e7a27

Co-authored-by: Davanum Srinivas <davanum@gmail.com> Refactored-by: lifubang <lifubang@acmcoder.com> Signed-off-by: lifubang <lifubang@acmcoder.com> (cherry picked from commit abf70ba) Signed-off-by: lifubang <lifubang@acmcoder.com>

integration: reuse tmpfs for directory masks

e454170

Co-authored-by: Davanum Srinivas <davanum@gmail.com> Signed-off-by: lifubang <lifubang@acmcoder.com> (cherry picked from commit 124772f) Signed-off-by: lifubang <lifubang@acmcoder.com>

lifubang force-pushed the backport-5275-1.4 branch from a613bb8 to 0d9be80 Compare May 20, 2026 10:21

lifubang marked this pull request as ready for review May 20, 2026 10:23

kolyshkin requested review from cyphar and rata and removed request for cyphar May 20, 2026 20:47

rata reviewed May 21, 2026

View reviewed changes

rata approved these changes May 21, 2026

View reviewed changes

kolyshkin approved these changes May 21, 2026

View reviewed changes

kolyshkin merged commit 3079745 into opencontainers:release-1.4 May 21, 2026
68 of 69 checks passed

This was referenced Jun 26, 2026

Recent update to moby-runc causes execution failures on Ubuntu:Focal (20.04) #5348

Closed

Recent update to moby-runc causes execution failures on Ubuntu:Focal (can't mask dir "/proc/acpi": [...]: invalid argument) moby/moby#52972

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[1.4] libct: reuse tmpfs for directory masks#5281

[1.4] libct: reuse tmpfs for directory masks#5281
kolyshkin merged 10 commits into
opencontainers:release-1.4from
lifubang:backport-5275-1.4

lifubang commented May 13, 2026 •

edited

Loading

Uh oh!

kolyshkin commented May 13, 2026

Uh oh!

lifubang commented May 14, 2026

Uh oh!

rata left a comment

Uh oh!

Uh oh!

Uh oh!

rata May 21, 2026

Uh oh!

lifubang May 21, 2026

Uh oh!

rata May 21, 2026 •

edited

Loading

Uh oh!

rata left a comment

Uh oh!

kolyshkin left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

lifubang commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backport #5190, #5275, and #5285

#5190

#5275

This PR carries forward #5262.

(With some assistance from claude/codex)

Uh oh!

kolyshkin commented May 13, 2026

Uh oh!

lifubang commented May 14, 2026

Uh oh!

rata left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rata May 21, 2026

Choose a reason for hiding this comment

Uh oh!

lifubang May 21, 2026

Choose a reason for hiding this comment

Uh oh!

rata May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rata left a comment

Choose a reason for hiding this comment

Uh oh!

kolyshkin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lifubang commented May 13, 2026 •

edited

Loading

rata May 21, 2026 •

edited

Loading