Skip to content

Commit ff6fe13

Browse files
committed
utils: use safe procfs for /proc/self/fd loop code
From a safety perspective this might not be strictly required, but it paves the way for us to remove utils.ProcThreadSelf. Signed-off-by: Aleksa Sarai <[email protected]>
1 parent fdcc9d3 commit ff6fe13

File tree

4 files changed

+40
-14
lines changed

4 files changed

+40
-14
lines changed

internal/linux/linux.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,25 @@ func SetMempolicy(mode uint, mask *unix.CPUSet) error {
8686
return os.NewSyscallError("set_mempolicy", err)
8787
}
8888

89+
// Readlinkat wraps [unix.Readlinkat].
90+
func Readlinkat(dir *os.File, path string) (string, error) {
91+
size := 4096
92+
for {
93+
linkBuf := make([]byte, size)
94+
n, err := retryOnEINTR2(func() (int, error) {
95+
return unix.Readlinkat(int(dir.Fd()), path, linkBuf)
96+
})
97+
if err != nil {
98+
return "", &os.PathError{Op: "readlinkat", Path: dir.Name() + "/" + path, Err: err}
99+
}
100+
if n != size {
101+
return string(linkBuf[:n]), nil
102+
}
103+
// Possible truncation, resize the buffer.
104+
size *= 2
105+
}
106+
}
107+
89108
// GetPtyPeer is a wrapper for ioctl(TIOCGPTPEER).
90109
func GetPtyPeer(ptyFd uintptr, unsafePeerPath string, flags int) (*os.File, error) {
91110
// Make sure O_NOCTTY is always set -- otherwise runc might accidentally

libcontainer/integration/exec_test.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@ import (
1515

1616
"github.com/opencontainers/cgroups"
1717
"github.com/opencontainers/cgroups/systemd"
18+
"github.com/opencontainers/runc/internal/linux"
19+
"github.com/opencontainers/runc/internal/pathrs"
1820
"github.com/opencontainers/runc/libcontainer"
1921
"github.com/opencontainers/runc/libcontainer/configs"
2022
"github.com/opencontainers/runc/libcontainer/internal/userns"
21-
"github.com/opencontainers/runc/libcontainer/utils"
2223
"github.com/opencontainers/runtime-spec/specs-go"
2324

2425
"golang.org/x/sys/unix"
@@ -1683,11 +1684,9 @@ func TestFdLeaksSystemd(t *testing.T) {
16831684
}
16841685

16851686
func fdList(t *testing.T) []string {
1686-
procSelfFd, closer := utils.ProcThreadSelf("fd")
1687-
defer closer()
1688-
1689-
fdDir, err := os.Open(procSelfFd)
1687+
fdDir, closer, err := pathrs.ProcThreadSelfOpen("fd/", unix.O_DIRECTORY|unix.O_CLOEXEC)
16901688
ok(t, err)
1689+
defer closer()
16911690
defer fdDir.Close()
16921691

16931692
fds, err := fdDir.Readdirnames(-1)
@@ -1726,8 +1725,10 @@ func testFdLeaks(t *testing.T, systemd bool) {
17261725

17271726
count := 0
17281727

1729-
procSelfFd, closer := utils.ProcThreadSelf("fd/")
1728+
procSelfFd, closer, err := pathrs.ProcThreadSelfOpen("fd/", unix.O_DIRECTORY|unix.O_CLOEXEC)
1729+
ok(t, err)
17301730
defer closer()
1731+
defer procSelfFd.Close()
17311732

17321733
next_fd:
17331734
for _, fd1 := range fds1 {
@@ -1736,7 +1737,7 @@ next_fd:
17361737
continue next_fd
17371738
}
17381739
}
1739-
dst, _ := os.Readlink(filepath.Join(procSelfFd, fd1))
1740+
dst, _ := linux.Readlinkat(procSelfFd, fd1)
17401741
for _, ex := range excludedPaths {
17411742
if ex == dst {
17421743
continue next_fd

libcontainer/utils/utils_unix.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
securejoin "github.com/cyphar/filepath-securejoin"
1616
"github.com/opencontainers/runc/internal/linux"
17+
"github.com/opencontainers/runc/internal/pathrs"
1718
"github.com/sirupsen/logrus"
1819
"golang.org/x/sys/unix"
1920
)
@@ -59,15 +60,15 @@ type fdFunc func(fd int)
5960
// fdRangeFrom calls the passed fdFunc for each file descriptor that is open in
6061
// the current process.
6162
func fdRangeFrom(minFd int, fn fdFunc) error {
62-
procSelfFd, closer := ProcThreadSelf("fd")
63-
defer closer()
64-
65-
fdDir, err := os.Open(procSelfFd)
63+
fdDir, closer, err := pathrs.ProcThreadSelfOpen("fd/", unix.O_DIRECTORY|unix.O_CLOEXEC)
6664
if err != nil {
67-
return err
65+
return fmt.Errorf("get handle to /proc/thread-self/fd: %w", err)
6866
}
67+
defer closer()
6968
defer fdDir.Close()
7069

70+
// NOTE: This is not really necessary since securejoin.ProcThreadSelf
71+
// verifies this in a far stricter sense than EnsureProcHandle.
7172
if err := EnsureProcHandle(fdDir); err != nil {
7273
return err
7374
}

utils_linux.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/urfave/cli"
1717
"golang.org/x/sys/unix"
1818

19+
"github.com/opencontainers/runc/internal/pathrs"
1920
"github.com/opencontainers/runc/libcontainer"
2021
"github.com/opencontainers/runc/libcontainer/configs"
2122
"github.com/opencontainers/runc/libcontainer/specconv"
@@ -241,10 +242,14 @@ func (r *runner) run(config *specs.Process) (int, error) {
241242
process.ExtraFiles = append(process.ExtraFiles, r.listenFDs...)
242243
}
243244
baseFd := 3 + len(process.ExtraFiles)
244-
procSelfFd, closer := utils.ProcThreadSelf("fd/")
245+
procSelfFd, closer, err := pathrs.ProcThreadSelfOpen("fd/", unix.O_DIRECTORY|unix.O_CLOEXEC)
246+
if err != nil {
247+
return -1, err
248+
}
245249
defer closer()
250+
defer procSelfFd.Close()
246251
for i := baseFd; i < baseFd+r.preserveFDs; i++ {
247-
_, err = os.Stat(filepath.Join(procSelfFd, strconv.Itoa(i)))
252+
err := unix.Faccessat(int(procSelfFd.Fd()), strconv.Itoa(i), unix.F_OK, 0)
248253
if err != nil {
249254
return -1, fmt.Errorf("unable to stat preserved-fd %d (of %d): %w", i-baseFd, r.preserveFDs, err)
250255
}

0 commit comments

Comments
 (0)