Skip to content

Commit 2a0e2b6

Browse files
cypharcyphar
committed
pathrs-lite: move Reopen impl to internal/procfs
This lets us make CheckSubpathOvermount a private function again and lets us clean up the user-facing API a little bit. Signed-off-by: Aleksa Sarai <[email protected]>
1 parent 3c5e239 commit 2a0e2b6

File tree

4 files changed

+56
-49
lines changed

4 files changed

+56
-49
lines changed

‎pathrs-lite/internal/procfs/procfs_linux.go‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -394,9 +394,9 @@ func (proc *Handle) OpenPid(pid int, subpath string) (*os.File, error) {
394394
return proc.OpenRoot(strconv.Itoa(pid) + "/" + subpath)
395395
}
396396

397-
// CheckSubpathOvermount checks if the dirfd and path combination is on the
397+
// checkSubpathOvermount checks if the dirfd and path combination is on the
398398
// same mount as the given root.
399-
func CheckSubpathOvermount(root, dir fd.Fd, path string) error {
399+
func checkSubpathOvermount(root, dir fd.Fd, path string) error {
400400
// Get the mntID of our procfs handle.
401401
expectedMountID, err := fd.GetMountID(root, "")
402402
if err != nil {
@@ -442,7 +442,7 @@ func (proc *Handle) Readlink(base procfsBase, subpath string) (string, error) {
442442
//
443443
// [1]: Linux commit ee2e3f50629f ("mount: fix mounting of detached mounts
444444
// onto targets that reside on shared mounts").
445-
if err := CheckSubpathOvermount(proc.Inner, link, ""); err != nil {
445+
if err := checkSubpathOvermount(proc.Inner, link, ""); err != nil {
446446
return "", fmt.Errorf("check safety of %s/%s magiclink: %w", base, subpath, err)
447447
}
448448

@@ -483,6 +483,52 @@ func CheckProcSelfFdPath(path string, file fd.Fd) error {
483483
return nil
484484
}
485485

486+
// ReopenFd takes an existing file descriptor and "re-opens" it through
487+
// /proc/thread-self/fd/<fd>. This allows for O_PATH file descriptors to be
488+
// upgraded to regular file descriptors, as well as changing the open mode of a
489+
// regular file descriptor. Some filesystems have unique handling of open(2)
490+
// which make this incredibly useful (such as /dev/ptmx).
491+
func ReopenFd(handle fd.Fd, flags int) (*os.File, error) {
492+
procRoot, err := OpenProcRoot() // subset=pid
493+
if err != nil {
494+
return nil, err
495+
}
496+
defer procRoot.Close() //nolint:errcheck // close failures aren't critical here
497+
498+
// We can't operate on /proc/thread-self/fd/$n directly when doing a
499+
// re-open, so we need to open /proc/thread-self/fd and then open a single
500+
// final component.
501+
procFdDir, closer, err := procRoot.OpenThreadSelf("fd/")
502+
if err != nil {
503+
return nil, fmt.Errorf("get safe /proc/thread-self/fd handle: %w", err)
504+
}
505+
defer procFdDir.Close() //nolint:errcheck // close failures aren't critical here
506+
defer closer()
507+
508+
// Try to detect if there is a mount on top of the magic-link we are about
509+
// to open. If we are using unsafeHostProcRoot(), this could change after
510+
// we check it (and there's nothing we can do about that) but for
511+
// privateProcRoot() this should be guaranteed to be safe (at least since
512+
// Linux 5.12[1], when anonymous mount namespaces were completely isolated
513+
// from external mounts including mount propagation events).
514+
//
515+
// [1]: Linux commit ee2e3f50629f ("mount: fix mounting of detached mounts
516+
// onto targets that reside on shared mounts").
517+
fdStr := strconv.Itoa(int(handle.Fd()))
518+
if err := checkSubpathOvermount(procRoot.Inner, procFdDir, fdStr); err != nil {
519+
return nil, fmt.Errorf("check safety of /proc/thread-self/fd/%s magiclink: %w", fdStr, err)
520+
}
521+
522+
flags |= unix.O_CLOEXEC
523+
// Rather than just wrapping fd.Openat, open-code it so we can copy
524+
// handle.Name().
525+
reopenFd, err := unix.Openat(int(procFdDir.Fd()), fdStr, flags, 0)
526+
if err != nil {
527+
return nil, fmt.Errorf("reopen fd %d: %w", handle.Fd(), err)
528+
}
529+
return os.NewFile(uintptr(reopenFd), handle.Name()), nil
530+
}
531+
486532
// Test hooks used in the procfs tests to verify that the fallback logic works.
487533
// See testing_mocks_linux_test.go and procfs_linux_test.go for more details.
488534
var (

‎pathrs-lite/internal/procfs/procfs_linux_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,14 +169,14 @@ func testProcOvermountSubdir(t *testing.T, procRootFn procRootFunc, expectOvermo
169169
defer procExe.Close() //nolint:errcheck // test code
170170

171171
// no overmount
172-
err = CheckSubpathOvermount(procRoot.Inner, procCwd, "")
172+
err = checkSubpathOvermount(procRoot.Inner, procCwd, "")
173173
assert.NoError(t, err, "checking /proc/self/cwd with no overmount should succeed") //nolint:testifylint // this is an isolated operation so we can continue despite an error
174-
err = CheckSubpathOvermount(procRoot.Inner, procSelf, "cwd")
174+
err = checkSubpathOvermount(procRoot.Inner, procSelf, "cwd")
175175
assert.NoError(t, err, "checking /proc/self/cwd with no overmount should succeed") //nolint:testifylint // this is an isolated operation so we can continue despite an error
176176
// basic overmount
177-
err = CheckSubpathOvermount(procRoot.Inner, procExe, "")
177+
err = checkSubpathOvermount(procRoot.Inner, procExe, "")
178178
assert.ErrorIs(t, err, symlinkOvermountErr, "unexpected /proc/self/exe overmount result") //nolint:testifylint // this is an isolated operation so we can continue despite an error
179-
err = CheckSubpathOvermount(procRoot.Inner, procSelf, "exe")
179+
err = checkSubpathOvermount(procRoot.Inner, procSelf, "exe")
180180
assert.ErrorIs(t, err, symlinkOvermountErr, "unexpected /proc/self/exe overmount result") //nolint:testifylint // this is an isolated operation so we can continue despite an error
181181

182182
// fd no overmount

‎pathrs-lite/internal/procfs/procfs_lookup_linux.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ func procfsLookupInRoot(procRoot fd.Fd, unsafePath string) (Handle *os.File, _ e
164164
_ = nextDir.Close()
165165
return nil, fmt.Errorf("check %q component is on procfs: %w", part, err)
166166
}
167-
if err := CheckSubpathOvermount(procRoot, nextDir, ""); err != nil {
167+
if err := checkSubpathOvermount(procRoot, nextDir, ""); err != nil {
168168
_ = nextDir.Close()
169169
return nil, fmt.Errorf("check %q component is not overmounted: %w", part, err)
170170
}
@@ -215,7 +215,7 @@ func procfsLookupInRoot(procRoot fd.Fd, unsafePath string) (Handle *os.File, _ e
215215
if err := verifyProcHandle(currentDir); err != nil {
216216
return nil, fmt.Errorf("check final handle is on procfs: %w", err)
217217
}
218-
if err := CheckSubpathOvermount(procRoot, currentDir, ""); err != nil {
218+
if err := checkSubpathOvermount(procRoot, currentDir, ""); err != nil {
219219
return nil, fmt.Errorf("check final handle is not overmounted: %w", err)
220220
}
221221
return currentDir, nil

‎pathrs-lite/open_linux.go‎

Lines changed: 1 addition & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@
1212
package pathrs
1313

1414
import (
15-
"fmt"
1615
"os"
17-
"strconv"
1816

1917
"golang.org/x/sys/unix"
2018

@@ -72,42 +70,5 @@ func OpenInRoot(root, unsafePath string) (*os.File, error) {
7270
//
7371
// [CVE-2019-19921]: https://github.com/advisories/GHSA-fh74-hm69-rqjw
7472
func Reopen(handle *os.File, flags int) (*os.File, error) {
75-
procRoot, err := procfs.OpenProcRoot() // subset=pid
76-
if err != nil {
77-
return nil, err
78-
}
79-
defer procRoot.Close() //nolint:errcheck // close failures aren't critical here
80-
81-
// We can't operate on /proc/thread-self/fd/$n directly when doing a
82-
// re-open, so we need to open /proc/thread-self/fd and then open a single
83-
// final component.
84-
procFdDir, closer, err := procRoot.OpenThreadSelf("fd/")
85-
if err != nil {
86-
return nil, fmt.Errorf("get safe /proc/thread-self/fd handle: %w", err)
87-
}
88-
defer procFdDir.Close() //nolint:errcheck // close failures aren't critical here
89-
defer closer()
90-
91-
// Try to detect if there is a mount on top of the magic-link we are about
92-
// to open. If we are using unsafeHostProcRoot(), this could change after
93-
// we check it (and there's nothing we can do about that) but for
94-
// privateProcRoot() this should be guaranteed to be safe (at least since
95-
// Linux 5.12[1], when anonymous mount namespaces were completely isolated
96-
// from external mounts including mount propagation events).
97-
//
98-
// [1]: Linux commit ee2e3f50629f ("mount: fix mounting of detached mounts
99-
// onto targets that reside on shared mounts").
100-
fdStr := strconv.Itoa(int(handle.Fd()))
101-
if err := procfs.CheckSubpathOvermount(procRoot.Inner, procFdDir, fdStr); err != nil {
102-
return nil, fmt.Errorf("check safety of /proc/thread-self/fd/%s magiclink: %w", fdStr, err)
103-
}
104-
105-
flags |= unix.O_CLOEXEC
106-
// Rather than just wrapping fd.Openat, open-code it so we can copy
107-
// handle.Name().
108-
reopenFd, err := unix.Openat(int(procFdDir.Fd()), fdStr, flags, 0)
109-
if err != nil {
110-
return nil, fmt.Errorf("reopen fd %d: %w", handle.Fd(), err)
111-
}
112-
return os.NewFile(uintptr(reopenFd), handle.Name()), nil
73+
return procfs.ReopenFd(handle, flags)
11374
}

0 commit comments

Comments
 (0)