mnt: new-API path silently drops nosuid/nodev/noexec on nested submounts if the recursive mount_setattr fails
- Dominant language
- C++
- Stars
- 4.1k
- Forks
- 371
- Avg merge
- 4d 7h
- Merged PRs (30d)
- 7
Description
## Summary
In the new mount-API path, the per-mount `nosuid`/`nodev`/`noexec` attributes of a recursive bind are applied with a single `mount_setattr()` call whose result is discarded (warning only). The later remount pass re-applies flags with a non-recursive `mount(2)` (top mount only), so if `mount_setattr()` fails, nested submounts of the cloned subtree stay suid/dev/exec-permitting inside the jail — including for mandatory mount points.
## Details (at f100fd9)
- `doBindMountAt()` clones the whole source subtree with `open_tree(..., OPEN_TREE_CLONE | AT_RECURSIVE)` and applies the requested flags via `applyMountFlags(..., recursive=true)`; the return value is only logged (`mnt_newapi.cc:497-499`).
- The final remount goes through `remountWithLegacyMount()` → `mount(dst, dst, MS_REMOUNT|MS_BIND|...)` without `MS_REC` — top mount only (`mnt_newapi.cc:170-186`; `computeLegacyRemountFlags()` adds no `MS_REC`).
- `remountPt()` therefore "succeeds", and `init()` only aborts when a *mandatory* mount's remount fails (`mnt.cc:330-332`) — a failed recursive `mount_setattr()` never surfaces.
- The recursive read-only pass on the jail root is not an equivalent backstop: it only sets `MOUNT_ATTR_RDONLY` (`mnt_newapi.cc:745-754`), so nosuid/nodev/noexec have no second recursive layer.
- The same discarded-return pattern exists for dynamic-content mounts (`mnt_newapi.cc:456-458`) and fs-mounts (`mnt_newapi.cc:568-570`).
## Impact
A config like "recursive-bind /usr with nosuid" over a source subtree containing a nested mount leaves that nested mount without nosuid inside the jail when `mount_setattr()` fails (fs-specific restrictions, permissions). A setuid binary on the nested mount stays setuid-effective for jailed processes. To be clear: this is fail-open hardening behavior rather than an exploitable bug — I did not find a way to force `mount_setattr()` to fail on a normal system.
## Suggested direction
Make `applyMountFlags()` failure fatal, at least for mandatory mount points, consistent with the recent fail-closed hardening (#306, #310, and #284 which introduced the recursive application). Happy to send a patch if the direction sounds right — equally fine to close as by-design best-effort.
Contributor guide
Research direction
Start with applyMountFlags() and its call sites in mnt_newapi.cc:456-458, 497-499, and 568-570, then trace remountWithLegacyMount() and init() in mnt_newapi.cc and mnt.cc:330-332. Review the fail-closed precedent in issues #306, #310, and #284 and determine the expected handling for mandatory versus non-mandatory mounts. Done means recursive flag-application failures are handled consistently and nested mounts cannot silently lose the requested hardening.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, linux
- Domain
- operating-systems, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100