Skip to content

[LXC] Fix denied-symlink and most-specific masking that aborted the container - #662

Merged
Darren Hoehna (dhoehna) merged 3 commits into
microsoft:mainfrom
dhoehna:user/dahoehna/lxc-fs-masking-abort-fix
Jul 22, 2026
Merged

[LXC] Fix denied-symlink and most-specific masking that aborted the container#662
Darren Hoehna (dhoehna) merged 3 commits into
microsoft:mainfrom
dhoehna:user/dahoehna/lxc-fs-masking-abort-fix

Conversation

@dhoehna

@dhoehnaDarren Hoehna (dhoehna) commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #630 (merged). Linked work item: AB#62861419.

#630 added LXC denied-path masking and the most-specific-path resolver, but two runtime paths abort the LXC container at startup. Neither is exercised by CI, which does not run the root+LXC end-to-end scripts under tests/scripts/run_lxc_*_test.sh (they need root, LXC, and a Linux host), so #630 merged green with both latent.

1. Denied symlink masking

A denied path that is (or traverses) a symlink was masked by mounting a tmpfs / /dev/null bind over the symlink node. The kernel refuses to mount over or through a symlink, so lxc-start aborted (Received container state "ABORTING"). Each denied path is now resolved to its real host target before masking, mirroring the Bubblewrap backend''s resolve_denied_paths / resolve_through_symlinks.

2. Most-specific-path masking

A denied directory containing a re-bound rw/ro descendant (e.g. deniedPaths: ["/mnt/x/data"] + readwritePaths: ["/mnt/x/data/child"]) was masked with a read-only, zero-size tmpfs. LXC must create the descendant''s mountpoint inside that mask, and mkdir in a ro,size=0 tmpfs fails (EROFS/ENOSPC), so the container aborted. Such a directory is now masked with a writable tmpfs so the mountpoint can be created (mirroring Bubblewrap''s writable --tmpfs); leaf denies keep the tight ro,size=0 mask.

Both fixes are localized to src/backends/lxc/common/src/filesystem_mounts.rs. No schema or wire-format changes. The test configs/scripts that exercise them already landed in #630.

Validation (Linux + LXC host, as root)

  • tests/scripts/run_lxc_all_tests.sh8/8 pass (previously aborted on Most-Specific Path and Denied Masking).
  • cargo test -p lxc_common42/42 (adds a unit test for the writable-mask descendant check).
Microsoft Reviewers: Open in CodeFlow

…ontainer
Two latent defects in the LXC filesystem-policy backend aborted the container
at startup. Neither was caught by CI, which does not run the root+LXC E2E
scripts (run_lxc_*_test.sh).
1. Denied symlink masking: a denied path that is (or traverses) a symlink was
masked by mounting tmpfs / a /dev/null bind over the symlink node. The
kernel refuses to mount over or through a symlink, so the container aborted.
Resolve each denied path to its real host target before masking (mirrors the
Bubblewrap backend's resolve_denied_paths).
2. Most-specific-path masking: a denied *directory* that contains a re-bound
rw/ro descendant was masked with a read-only, zero-size tmpfs. LXC must
create the descendant's mountpoint inside that mask, and mkdir in a
ro,size=0 tmpfs fails (EROFS/ENOSPC), so the container aborted. Mask such a
directory with a writable tmpfs so the mountpoint can be created (mirrors
Bubblewrap's writable --tmpfs); leaf denies keep the tight ro,size=0 mask.
Verified locally: full LXC E2E suite 8/8 pass (previously aborted on
Most-Specific Path and Denied Masking); cargo test -p lxc_common 42/42.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b6b3323b-7297-4b07-9e6e-ab4b220124e6
CopilotAI review requested due to automatic review settings July 20, 2026 21:32
@dhoehna
Darren Hoehna (dhoehna) requested a review from a team as a code ownerJuly 20, 2026 21:32
@dhoehna

Copy link
Copy Markdown
ContributorAuthor

Soham Das (@SohamDas2021) — follow-up to #630 (which merged before these landed).

While running #630''s LXC end-to-end suite locally I hit two container-abort bugs in the denied-path masking:

  1. Denied symlink masked over the symlink node → kernel refuses to mount through a symlink → lxc-start aborts. Now resolves to the real host target before masking (mirrors the bwrap backend).
  2. Most-specific denied-parent + rw-child: the denied dir was masked tmpfs ro,size=0, so LXC can''t mkdir the child mountpoint inside it (EROFS/ENOSPC) → abort. Now a denied dir with a re-bound descendant uses a writable tmpfs (leaf denies keep ro,size=0).

Both are latent because CI doesn''t run the root+LXC run_lxc_*_test.sh scripts. Locally run_lxc_all_tests.sh now passes 8/8 and cargo test -p lxc_common is 42/42. Mind taking a look when you have a moment?

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes two LXC filesystem-policy masking edge cases introduced in #630 that could cause lxc-start to abort at container startup. The changes are localized to the LXC mount-entry generation logic.

Changes:

  • Resolve denied host paths through symlinks before masking, to avoid LXC/kernel “no mount over/through symlink” aborts.
  • Mask denied directories that contain re-bound (rw/ro) descendants with a writable tmpfs so LXC can create the descendant mountpoints.
  • Add unit tests covering the descendant detection and symlink resolution behavior.

Comment on lines +92 to +95
/// Returns the original path unchanged when it does not traverse a symlink or
/// cannot be resolved. Fails closed if a resolved path is not valid UTF-8: the
/// LXC config is a `String` pipeline that cannot represent it faithfully, and a
/// lossy replacement could mask the wrong path and leave the target exposed.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ab1a1e7. Rewrote the doc comment: resolution goes through resolve_through_symlinks, which canonicalizes every existing prefix (following symlinks like the kernel) and folds a not-yet-existing tail lexically, so an existing non-symlink path comes back in canonical absolute form — not necessarily byte-identical to the input; only a path that resolves to nothing is returned unchanged. The two fail-closed cases (non-UTF-8 and residual symlink) are now documented as well.

Comment on lines +96 to +110
fn resolve_denied_host_path(host_path: &str) -> Result<String, String> {
match resolve_through_symlinks(Path::new(host_path)) {
Some(resolved) => {
let real = resolved.to_str().ok_or_else(|| {
format!(
"deniedPaths entry {:?} resolves to a non-UTF-8 host path that cannot be \
safely masked; refusing to start.",
host_path
)
})?;
Ok(real.to_owned())
}
None => Ok(host_path.to_owned()),
}
}

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ab1a1e7. resolve_denied_host_path now fails closed when the resolved path is still a symlink (dangling/unresolvable): it returns Err instead of emitting a mask that would abort lxc-start over a symlink node. A non-existent but non-symlink tail still returns Err from symlink_metadata and is treated as safe to mask as an empty dir (existing behavior preserved). Unlike Bubblewrap — which tolerates a /dev/null bind over a symlink node — the LXC mount pipeline can't guarantee that, so refusing to start is the safe, deterministic choice (now documented). Added test resolve_denied_host_path_fails_closed_on_dangling_symlink.

// stays hidden underneath, only the ephemeral tmpfs is
// writable, and the descendant bind lands on top of it. This
// mirrors the Bubblewrap backend's writable `--tmpfs`.
format!("tmpfs {} tmpfs create=dir 0 0", container_path)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ab1a1e7. The writable denied-dir mask is now tmpfs {} tmpfs size=1m,create=dir 0 0. The mask only needs to hold empty mountpoint directories, so size=1m bounds it and prevents sandboxed code from exhausting host memory through the ephemeral tmpfs. (The read-only mask already used ro,size=0.)

@SohamDas2021

Copy link
Copy Markdown
Contributor

I figured there could be another gap that needs to be addressed in the file system policy.
This was a bug in the WSLC flow as well, that I had fixed.

normalize_filesystem_paths does not fold .. / . it only does string dedup + existence warnings, right? So .. spellings reach the runner intact. Tracing resolve_through_symlinks on a non-existent path like /X/Y/../Z : the backward walk drops the .. (Rust's file_name() returns None for a .. component) then re-adds Y, reconstructing /X/Y/Z instead of the correct /X/Z. If an ancestor is a symlink, the mask lands on the wrong target and a real denied path can go unmasked.

Can you please check if the issue is real in the lxc flow as well? I guess normalize_filesystem_paths is common code, but I do not remember if lxc is wired

/// canonicalizes it, then re-appends the non-existent tail. Returns `None` only
/// when no ancestor can be canonicalized. Mirrors the Bubblewrap backend's
/// `resolve_through_symlinks` so both backends mask the same real target.
fn resolve_through_symlinks(path: &Path) -> Option<PathBuf> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebound_container_paths are built from unresolved mount paths, but has_rebound_descendant is called with the resolved denied container_path . If a denied directory is a symlink, the prefix compare misses the descendant -> the tight ro,size=0 mask is (re)selected, and the child bind still traverses the symlink - reintroducing the abort this PR set out to fix or breaking most-specific-wins?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in ab1a1e7. The rebound-descendant comparison set is now built from BOTH the literal rw/ro path and its symlink-resolved form via a new rebound_comparison_paths helper (configure_filesystem_mounts now .flat_maps it). Because a denied dir is masked at its resolved path, a child re-bound through a symlinked ancestor physically lands inside that resolved mask; including the resolved rebound form lets has_rebound_descendant detect it and pick the writable mask instead of the read-only size=0 tmpfs, which previously made the child mountpoint mkdir fail with EROFS and aborted lxc-start. Added test rebound_comparison_paths_detects_child_under_symlinked_denied_parent.

`resolve_through_symlinks` (LXC + Bubblewrap) resolved a denied path by
walking backward via `file_name()`, which returns `None` for a `..`
component. A `..` was therefore silently dropped: `/link/missing/../secret`
under a symlinked `link -> /real` (with `missing` absent) resolved to
`/real/missing/secret` instead of `/real/secret`. The tmpfs / `/dev/null`
mask then landed on a bystander path and the real denied target stayed
readable inside the container.
The common `normalize_filesystem_paths` does not fold `..`, so these
spellings reach the runner intact (WSLC has its own `..` guard in
`validate_denied_path_overlap`; LXC and Bubblewrap had none).
Replace the backward walk with a forward realpath-style walk: canonicalize
each existing prefix (following symlinks like the kernel) and fold `.` / `..`
lexically on the non-existent tail. A missing component cannot be a symlink,
so lexical folding there matches kernel path resolution. Adds a regression
test to each backend.
Reported by SohamDas2021 on PR microsoft#662 (same class of bug he fixed in WSLC).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b6b3323b-7297-4b07-9e6e-ab4b220124e6
…dants; cap mask tmpfs
Address PR microsoft#662 review feedback in filesystem_mounts.rs:
- resolve_denied_host_path: correct the doc to reflect that resolution canonicalizes existing prefixes (it does not 'return the original unchanged'), and fail closed when the resolved path is still a symlink (dangling/unresolvable) instead of emitting a mask that would abort lxc-start over a symlink node.
- configure_filesystem_mounts: build the rebound-descendant comparison set from both the literal rw/ro path and its symlink-resolved form (new rebound_comparison_paths helper). A denied dir is masked at its resolved path, so a child re-bound through a symlinked ancestor lands inside that mask and must be detected as a descendant to select the writable mask instead of the read-only size=0 tmpfs that aborts the mkdir.
- Cap the writable denied-dir mask at size=1m so sandboxed code cannot exhaust host memory through the ephemeral tmpfs.
Add unit tests for the dangling-symlink fail-closed path and symlink-resolved rebound descendant detection.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@microsoft-github-policy-servicemicrosoft-github-policy-serviceBot added Needs-Attention Issue needs attention from Microsoft and removed Needs-Author-Feedback Issue needs attention from issue or PR author labels Jul 21, 2026
@dhoehna

Copy link
Copy Markdown
ContributorAuthor

Review feedback addressed in ab1a1e7 (all in src/backends/lxc/common/src/filesystem_mounts.rs):

  1. Doc on resolve_denied_host_path (Copilot) — rewritten. Resolution goes through resolve_through_symlinks, which canonicalizes every existing prefix (following symlinks like the kernel) and folds a not-yet-existing tail lexically, so an existing non-symlink path comes back in canonical absolute form (not necessarily byte-identical); only a path that resolves to nothing is returned unchanged. Both fail-closed cases (non-UTF-8 and residual symlink) are now documented.
  2. Residual/dangling symlink (Copilot) — resolve_denied_host_path now fails closed (Err) when the resolved path is still a symlink, instead of emitting a mask that would abort lxc-start over a symlink node. A non-existent but non-symlink tail is still treated as safe (masked as an empty dir). LXC intentionally diverges from Bubblewrap here (which tolerates a /dev/null bind over a symlink node) — now documented. New test resolve_denied_host_path_fails_closed_on_dangling_symlink.
  3. Writable mask memory cap (Copilot) — the writable denied-dir mask is now tmpfs {} tmpfs size=1m,create=dir 0 0. The mask only holds empty mountpoint directories, so size=1m bounds it and prevents sandboxed code from exhausting host memory through the ephemeral tmpfs.
  4. Rebound descendant vs. resolved denied path (Soham Das (@SohamDas2021), line 71) — the rebound comparison set is now built from both the literal rw/ro path and its symlink-resolved form (new rebound_comparison_paths helper; configure_filesystem_mounts.flat_maps it). Because a denied dir is masked at its resolved path, a child re-bound through a symlinked ancestor lands inside that resolved mask; including the resolved form lets has_rebound_descendant detect it and select the writable mask instead of the read-only size=0 tmpfs that made the child mountpoint mkdir fail (EROFS) and aborted lxc-start. New test rebound_comparison_paths_detects_child_under_symlinked_denied_parent.

Re: the .. traversal question (Soham Das (@SohamDas2021), review comment) — already fixed in commit 299fdcc ("Fix .. traversal dropping the real denied target in symlink resolver"): resolve_through_symlinks now folds ./.. lexically for the not-yet-existing tail (Component::ParentDir => result.pop()), so /link/missing/../secret resolves to /real/secret rather than the bystander /real/missing/secret. Covered by test resolve_denied_host_path_folds_dotdot_under_symlinked_ancestor.

Validation:cargo fmt clean; cargo clippy --target x86_64-unknown-linux-gnu -p lxc_common --all-targets -- -D warnings clean; cargo test -p lxc_common45 passed, 0 failed (run on Linux/WSL).

@dhoehna
Darren Hoehna (dhoehna) merged commit 630af78 into microsoft:mainJul 22, 2026
20 checks passed
@microsoft-github-policy-servicemicrosoft-github-policy-serviceBot removed the Needs-Attention Issue needs attention from Microsoft label Jul 22, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@dhoehna@SohamDas2021