fix: permission denied error for /proc/self/mountinfo - #100
fix: permission denied error for /proc/self/mountinfo#100ritesh-harihar wants to merge 6 commits into
Conversation
| var ee *exec.ExitError | ||
| if errors.As(err, &ee) { | ||
| code = ee.ExitCode() | ||
| } else { | ||
| debug("task command failed: %v", err) | ||
| code = subproc.ExitFailure | ||
| } |
There was a problem hiding this comment.
Not related to this PR.
If cmd.Dir points to an un-unveiled directory, cmd.Run() returns a *fs.PathError from the kernel-level chdir(2), not an *exec.ExitError. This bare type assertion panicked on the wrong type, leaving the allocation stuck. Replaced with errors.As and returns ExitFailure = 1 .
tgross
left a comment
There was a problem hiding this comment.
A couple things seem to be missing here for my understanding:'
- Where does this report come from? Didn't this pop up when you were testing #98 ?
- If
unshareis setting up a PID namespace and--mount-proc, won't the inode for/procin the "container" be different than the one we're reading here regardless?
In PR #97 , the testing was done mostly when Before |
|
If the case is The logic added in
Yes, the device number changes. The specific file inodes like But
This bug was specifically for a file("/proc/self/mountinfo","r"). A specific file inode has no stable equivalent across a remount, the root always does. So |
Problem reported
A workload running under the exec2 driver received
Permission deniedwhen reading/proc/self/mountinfo, even though the path was explicitly listed in the task's unveil config. No Landlock violations appeared in the audit log — the access was silently blocked from within the sandboxing layer itself.Jobspec that triggered the issue:
Root cause
convert()calledos.Stat(filepath)on every unveil entry. For/proc/self/mountinfo,os.Statfollows the/proc/selfmagic symlink and resolves it to/proc/<shim-pid>/mountinfo—> the shim's own PID entry. Landlock locks that specific inode beforeunshare --mount-procruns. After the unshare, the task's private/proccontains completely different inodes; the shim-PID entry no longer exists, so every read returns EPERM.Paths without symlink indirection —
/proc/cpuinfo,/proc/meminfo,r:/procwere unaffected because their inodes are stable across the namespace boundary.Fix:
For any path matching
/proc/self/*or/proc/thread-self/*before calling os.Stat, promote it toDir("/proc", mode). This is because go-landlock registers rules viaO_PATHwhich pins the inode at registration time. /proc/selfresolves to/proc/<shim-pid>, an inode that does not exist in the task's private mount namespace afterunshare --mount-proc. Pinning it would always produceEPERM. Promoting to Dir("/proc", mode) uses the stable /proc directory inode instead, which covers all descendants.The defaults block also adds
Dir("/proc", "r")unconditionally whendefaults=true, so runtimes (JVM, Go) can read/proc/self/cgroupand/proc/self/mountinfowithout needing any explicit unveil entry.Testing
Details
Result Before:
Result After:
2) Test for panic (unveil_defaults=false + command NOT in any unveil list)
(defaults=false, nothing unveiled → Before: panic(ExitCode 2)→ After: No panic
Result Before:
Result After:
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.