userns: handle OpenVZ namespace inode virtualization - #255
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change is small, self-contained, and the OpenVZ detection logic matches the documented systemd checks without introducing apparent behavioral regressions.
Pull request overview
Adjusts Linux user-namespace detection to handle OpenVZ environments where procfs virtualizes namespace inode numbers, making PROC_USER_INIT_INO comparisons unreliable.
Changes:
- Detect OpenVZ containers via
/proc/vz+/proc/bcchecks (matching systemd’s approach) and use the uid_map heuristic in that case. - Keep the inode-based detection as the preferred path on non-OpenVZ systems.
- Refactor the uid_map-based detection into a helper (
runningInUserNSFromUIDMap) for reuse.
File summaries
| File | Description |
|---|---|
| userns/userns_linux.go | Adds OpenVZ detection and routes userns detection through uid_map fallback when running under OpenVZ. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| func runningInUserNS() bool { | ||
| if runningInOpenVZ() { | ||
| return runningInUserNSFromUIDMap() | ||
| } |
There was a problem hiding this comment.
I don't think there's meaningful tests we can create, without mocking everything, by which time we're just testing our test-mocks, not a real situation.
2445f1c to
7bc0dee
Compare
Commit 023b764 ("userns: detect initial namespace by inode") changed user-namespace detection to compare the inode of /proc/self/ns/user against the kernel's PROC_USER_INIT_INO value. This avoids misidentifying full-range user namespaces, such as those created by systemd's PrivateUsers=full, as the initial namespace. OpenVZ, however, virtualizes the namespace inode numbers exposed through procfs, so PROC_USER_INIT_INO cannot reliably identify the initial user namespace there. Detect when running inside an OpenVZ container using the same procfs checks as systemd, and fall back to the uid_map-based detection in that case. Keep the namespace-inode check as the preferred path elsewhere, while preserving the existing uid_map fallback for older kernels. updates: 023b764 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
7bc0dee to
9b6dd9e
Compare
| // https://github.com/systemd/systemd/blob/v261.2/src/basic/virt.c#L642-L653 | ||
| func runningInOpenVZ() bool { | ||
| var st syscall.Stat_t | ||
| if err := syscall.Stat("/proc/vz", &st); err != nil { |
There was a problem hiding this comment.
| if err := syscall.Stat("/proc/vz", &st); err != nil { | |
| if err := syscall.Stat("/proc/vz", &st); errors.Is(err, syscall.ENOENT) { |
There was a problem hiding this comment.
Not in this case; if we'd only check for ENOENT, then the second check would potentially result in a false positive. So the first check is mostly a gate; if we can't be sure, we assume it's not openVZ.
| // /proc/bc is only exposed on the host. This follows systemd's OpenVZ | ||
| // detection: | ||
| // https://github.com/systemd/systemd/blob/v261.2/src/basic/virt.c#L642-L653 | ||
| func runningInOpenVZ() bool { |
There was a problem hiding this comment.
OpenVZ no longer seems under active development.
Will we remove this within a few years?
There was a problem hiding this comment.
Yeah, possibly; apparently there's still users running it (from the Moby ticket), so it's not completely gone yet, and systemd also still detects it.
|
I'll bring this one in, and tag a v0.2.1 release (also not super-happy with the special casing, but seemed like the best thing to do for now). |
|
@mihalicyn @finist0 can this be fixed in openvz kernel? |
Commit d73e66c ("utils: detect initial user namespace by inode") changed check_running_in_user_namespace() to compare the inode of /proc/self/ns/user against PROC_USER_INIT_INO, so that a user namespace with a full identity mapping -- such as the one systemd >= 260 sets up for PrivateUsers=full -- is no longer mistaken for the initial one. OpenVZ virtualizes the namespace inode numbers it exposes through procfs, so PROC_USER_INIT_INO does not identify the initial user namespace there and the new check regressed the detection inside OpenVZ containers. This was reported for moby, which made the same change, by Sebastiaan van Stijn in containers#2150, along with the suggestion to special-case OpenVZ the way moby/sys#255 does: moby/sys#255 Detect OpenVZ with the same procfs test systemd uses -- /proc/vz is exposed both on the host and inside a container, /proc/bc only on the host -- and fall back to the uid_map heuristic there. The inode check stays the preferred path everywhere else. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
relates to:
Commit 023b764 ("userns: detect initial namespace by inode") changed user-namespace detection to compare the inode of /proc/self/ns/user against the kernel's PROC_USER_INIT_INO value. This avoids misidentifying full-range user namespaces, such as those created by systemd's PrivateUsers=full, as the initial namespace.
OpenVZ, however, virtualizes the namespace inode numbers exposed through procfs, so PROC_USER_INIT_INO cannot reliably identify the initial user namespace there.
Detect when running inside an OpenVZ container using the same procfs checks as systemd, and fall back to the uid_map-based detection in that case.
Keep the namespace-inode check as the preferred path elsewhere, while preserving the existing uid_map fallback for older kernels.
updates: 023b764