From 8cc1eb379bb0422c9198723d1b49d83c7295212e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 25 Mar 2025 14:24:20 -0700 Subject: [PATCH 1/5] Introduce and use internal/linux This package is to provide unix.* wrappers to ensure that: - they retry on EINTR; - a "rich" error is returned on failure. A first such wrapper, Sendmsg, is introduced. Signed-off-by: Kir Kolyshkin --- internal/linux/eintr.go | 18 ++++++++++++++++++ internal/linux/linux.go | 15 +++++++++++++++ libcontainer/utils/cmsg.go | 8 ++------ notify_socket.go | 7 ++++--- 4 files changed, 39 insertions(+), 9 deletions(-) create mode 100644 internal/linux/eintr.go create mode 100644 internal/linux/linux.go diff --git a/internal/linux/eintr.go b/internal/linux/eintr.go new file mode 100644 index 000000000..4b3dd5197 --- /dev/null +++ b/internal/linux/eintr.go @@ -0,0 +1,18 @@ +package linux + +import ( + "errors" + + "golang.org/x/sys/unix" +) + +// retryOnEINTR takes a function that returns an error and calls it +// until the error returned is not EINTR. +func retryOnEINTR(fn func() error) error { + for { + err := fn() + if !errors.Is(err, unix.EINTR) { + return err + } + } +} diff --git a/internal/linux/linux.go b/internal/linux/linux.go new file mode 100644 index 000000000..d5ce74d2a --- /dev/null +++ b/internal/linux/linux.go @@ -0,0 +1,15 @@ +package linux + +import ( + "os" + + "golang.org/x/sys/unix" +) + +// Sendmsg wraps [unix.Sendmsg]. +func Sendmsg(fd int, p, oob []byte, to unix.Sockaddr, flags int) error { + err := retryOnEINTR(func() error { + return unix.Sendmsg(fd, p, oob, to, flags) + }) + return os.NewSyscallError("sendmsg", err) +} diff --git a/libcontainer/utils/cmsg.go b/libcontainer/utils/cmsg.go index 8b63a1295..96aa3ccc9 100644 --- a/libcontainer/utils/cmsg.go +++ b/libcontainer/utils/cmsg.go @@ -21,6 +21,7 @@ import ( "os" "runtime" + "github.com/opencontainers/runc/internal/linux" "golang.org/x/sys/unix" ) @@ -126,10 +127,5 @@ func SendFile(socket *os.File, file *os.File) error { // SendRawFd sends a specific file descriptor over the given AF_UNIX socket. func SendRawFd(socket *os.File, msg string, fd uintptr) error { oob := unix.UnixRights(int(fd)) - for { - err := unix.Sendmsg(int(socket.Fd()), []byte(msg), oob, nil, 0) - if err != unix.EINTR { - return os.NewSyscallError("sendmsg", err) - } - } + return linux.Sendmsg(int(socket.Fd()), []byte(msg), oob, nil, 0) } diff --git a/notify_socket.go b/notify_socket.go index 34c31cc3f..afe818b3a 100644 --- a/notify_socket.go +++ b/notify_socket.go @@ -11,6 +11,7 @@ import ( "strconv" "time" + "github.com/opencontainers/runc/internal/linux" "github.com/opencontainers/runc/libcontainer" "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" @@ -181,7 +182,7 @@ func sdNotifyBarrier(client *net.UnixConn) error { return err } - // Get the FD for the unix socket file to be able to do perform syscall.Sendmsg. + // Get the FD for the unix socket file to be able to use sendmsg. clientFd, err := client.File() if err != nil { return err @@ -189,9 +190,9 @@ func sdNotifyBarrier(client *net.UnixConn) error { // Send the write end of the pipe along with a BARRIER=1 message. fdRights := unix.UnixRights(int(pipeW.Fd())) - err = unix.Sendmsg(int(clientFd.Fd()), []byte("BARRIER=1"), fdRights, nil, 0) + err = linux.Sendmsg(int(clientFd.Fd()), []byte("BARRIER=1"), fdRights, nil, 0) if err != nil { - return &os.SyscallError{Syscall: "sendmsg", Err: err} + return err } // Close our copy of pipeW. From 431b8bb4d8dde8b6c96a280c950829481d617de2 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 25 Mar 2025 14:36:42 -0700 Subject: [PATCH 2/5] int/linux: add/use Getwd Signed-off-by: Kir Kolyshkin --- internal/linux/eintr.go | 10 ++++++++++ internal/linux/linux.go | 6 ++++++ libcontainer/init_linux.go | 5 +++-- libcontainer/specconv/spec_linux.go | 20 +++++--------------- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/internal/linux/eintr.go b/internal/linux/eintr.go index 4b3dd5197..36a6e3e29 100644 --- a/internal/linux/eintr.go +++ b/internal/linux/eintr.go @@ -16,3 +16,13 @@ func retryOnEINTR(fn func() error) error { } } } + +// retryOnEINTR2 is like retryOnEINTR, but it returns 2 values. +func retryOnEINTR2[T any](fn func() (T, error)) (T, error) { + for { + val, err := fn() + if !errors.Is(err, unix.EINTR) { + return val, err + } + } +} diff --git a/internal/linux/linux.go b/internal/linux/linux.go index d5ce74d2a..efcd985d4 100644 --- a/internal/linux/linux.go +++ b/internal/linux/linux.go @@ -6,6 +6,12 @@ import ( "golang.org/x/sys/unix" ) +// Getwd wraps [unix.Getwd]. +func Getwd() (wd string, err error) { + wd, err = retryOnEINTR2(unix.Getwd) + return wd, os.NewSyscallError("getwd", err) +} + // Sendmsg wraps [unix.Sendmsg]. func Sendmsg(fd int, p, oob []byte, to unix.Sockaddr, flags int) error { err := retryOnEINTR(func() error { diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index ef7b0cf13..4546af9da 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -20,6 +20,7 @@ import ( "golang.org/x/sys/unix" "github.com/opencontainers/cgroups" + "github.com/opencontainers/runc/internal/linux" "github.com/opencontainers/runc/libcontainer/capabilities" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/system" @@ -280,10 +281,10 @@ func verifyCwd() error { // details, and CVE-2024-21626 for the security issue that motivated this // check. // - // We have to use unix.Getwd() here because os.Getwd() has a workaround for + // We do not use os.Getwd() here because it has a workaround for // $PWD which involves doing stat(.), which can fail if the current // directory is inaccessible to the container process. - if wd, err := unix.Getwd(); errors.Is(err, unix.ENOENT) { + if wd, err := linux.Getwd(); errors.Is(err, unix.ENOENT) { return errors.New("current working directory is outside of container mount namespace root -- possible container breakout detected") } else if err != nil { return fmt.Errorf("failed to verify if current working directory is safe: %w", err) diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index be1e3d8ea..37068da7f 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -16,6 +16,7 @@ import ( dbus "github.com/godbus/dbus/v5" "github.com/opencontainers/cgroups" devices "github.com/opencontainers/cgroups/devices/config" + "github.com/opencontainers/runc/internal/linux" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/internal/userns" "github.com/opencontainers/runc/libcontainer/seccomp" @@ -344,24 +345,13 @@ type CreateOpts struct { RootlessCgroups bool } -// getwd is a wrapper similar to os.Getwd, except it always gets -// the value from the kernel, which guarantees the returned value -// to be absolute and clean. -func getwd() (wd string, err error) { - for { - wd, err = unix.Getwd() - if err != unix.EINTR { - break - } - } - return wd, os.NewSyscallError("getwd", err) -} - // CreateLibcontainerConfig creates a new libcontainer configuration from a // given specification and a cgroup name func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) { - // runc's cwd will always be the bundle path - cwd, err := getwd() + // Runc's cwd will always be the bundle path. + // Use the value from the kernel, which guarantees the returned value + // to be absolute and clean. + cwd, err := linux.Getwd() if err != nil { return nil, err } From c690b66d7f75036ccf04ae73c27aa75aedec04e0 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 25 Mar 2025 14:41:41 -0700 Subject: [PATCH 3/5] int/linux: add/use Exec Drop the libcontainer/system/exec, and use the linux.Exec instead. Signed-off-by: Kir Kolyshkin --- internal/linux/linux.go | 11 +++++++++++ libcontainer/setns_init_linux.go | 3 ++- libcontainer/standard_init_linux.go | 3 ++- libcontainer/system/linux.go | 9 --------- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/internal/linux/linux.go b/internal/linux/linux.go index efcd985d4..9c81a7434 100644 --- a/internal/linux/linux.go +++ b/internal/linux/linux.go @@ -6,6 +6,17 @@ import ( "golang.org/x/sys/unix" ) +// Exec wraps [unix.Exec]. +func Exec(cmd string, args []string, env []string) error { + err := retryOnEINTR(func() error { + return unix.Exec(cmd, args, env) + }) + if err != nil { + return &os.PathError{Op: "exec", Path: cmd, Err: err} + } + return nil +} + // Getwd wraps [unix.Getwd]. func Getwd() (wd string, err error) { wd, err = retryOnEINTR2(unix.Getwd) diff --git a/libcontainer/setns_init_linux.go b/libcontainer/setns_init_linux.go index 1c014d62e..a3d9a8d92 100644 --- a/libcontainer/setns_init_linux.go +++ b/libcontainer/setns_init_linux.go @@ -10,6 +10,7 @@ import ( "github.com/sirupsen/logrus" "golang.org/x/sys/unix" + "github.com/opencontainers/runc/internal/linux" "github.com/opencontainers/runc/libcontainer/apparmor" "github.com/opencontainers/runc/libcontainer/keys" "github.com/opencontainers/runc/libcontainer/seccomp" @@ -156,5 +157,5 @@ func (l *linuxSetnsInit) Init() error { if err := utils.UnsafeCloseFrom(l.config.PassedFilesCount + 3); err != nil { return err } - return system.Exec(name, l.config.Args, l.config.Env) + return linux.Exec(name, l.config.Args, l.config.Env) } diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index 1b37190c0..cccb1d89a 100644 --- a/libcontainer/standard_init_linux.go +++ b/libcontainer/standard_init_linux.go @@ -11,6 +11,7 @@ import ( "github.com/sirupsen/logrus" "golang.org/x/sys/unix" + "github.com/opencontainers/runc/internal/linux" "github.com/opencontainers/runc/libcontainer/apparmor" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/keys" @@ -298,5 +299,5 @@ func (l *linuxStandardInit) Init() error { if err := utils.UnsafeCloseFrom(l.config.PassedFilesCount + 3); err != nil { return err } - return system.Exec(name, l.config.Args, l.config.Env) + return linux.Exec(name, l.config.Args, l.config.Env) } diff --git a/libcontainer/system/linux.go b/libcontainer/system/linux.go index e8ce0ecac..48167bd62 100644 --- a/libcontainer/system/linux.go +++ b/libcontainer/system/linux.go @@ -32,15 +32,6 @@ func (p ParentDeathSignal) Set() error { return SetParentDeathSignal(uintptr(p)) } -func Exec(cmd string, args []string, env []string) error { - for { - err := unix.Exec(cmd, args, env) - if err != unix.EINTR { - return &os.PathError{Op: "exec", Path: cmd, Err: err} - } - } -} - func SetParentDeathSignal(sig uintptr) error { if err := unix.Prctl(unix.PR_SET_PDEATHSIG, sig, 0, 0, 0); err != nil { return err From e655abc0da9ba2c2c36579ccbdc845a9bcf356b7 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 25 Mar 2025 14:51:07 -0700 Subject: [PATCH 4/5] int/linux: add/use Dup3, Open, Openat Signed-off-by: Kir Kolyshkin --- internal/linux/linux.go | 30 ++++++++++++++++++++++++++ libcontainer/console_linux.go | 11 ++++------ libcontainer/rootfs_linux.go | 17 ++++++--------- libcontainer/standard_init_linux.go | 4 ++-- libcontainer/utils/utils_unix.go | 5 +++-- tests/cmd/seccompagent/seccompagent.go | 3 ++- 6 files changed, 48 insertions(+), 22 deletions(-) diff --git a/internal/linux/linux.go b/internal/linux/linux.go index 9c81a7434..b281a3347 100644 --- a/internal/linux/linux.go +++ b/internal/linux/linux.go @@ -6,6 +6,14 @@ import ( "golang.org/x/sys/unix" ) +// Dup3 wraps [unix.Dup3]. +func Dup3(oldfd, newfd, flags int) error { + err := retryOnEINTR(func() error { + return unix.Dup3(oldfd, newfd, flags) + }) + return os.NewSyscallError("dup3", err) +} + // Exec wraps [unix.Exec]. func Exec(cmd string, args []string, env []string) error { err := retryOnEINTR(func() error { @@ -23,6 +31,28 @@ func Getwd() (wd string, err error) { return wd, os.NewSyscallError("getwd", err) } +// Open wraps [unix.Open]. +func Open(path string, mode int, perm uint32) (fd int, err error) { + fd, err = retryOnEINTR2(func() (int, error) { + return unix.Open(path, mode, perm) + }) + if err != nil { + return -1, &os.PathError{Op: "open", Path: path, Err: err} + } + return fd, nil +} + +// Openat wraps [unix.Openat]. +func Openat(dirfd int, path string, mode int, perm uint32) (fd int, err error) { + fd, err = retryOnEINTR2(func() (int, error) { + return unix.Openat(dirfd, path, mode, perm) + }) + if err != nil { + return -1, &os.PathError{Op: "openat", Path: path, Err: err} + } + return fd, nil +} + // Sendmsg wraps [unix.Sendmsg]. func Sendmsg(fd int, p, oob []byte, to unix.Sockaddr, flags int) error { err := retryOnEINTR(func() error { diff --git a/libcontainer/console_linux.go b/libcontainer/console_linux.go index e506853e4..c42e60e3c 100644 --- a/libcontainer/console_linux.go +++ b/libcontainer/console_linux.go @@ -3,6 +3,7 @@ package libcontainer import ( "os" + "github.com/opencontainers/runc/internal/linux" "golang.org/x/sys/unix" ) @@ -26,16 +27,12 @@ func mountConsole(slavePath string) error { // dupStdio opens the slavePath for the console and dups the fds to the current // processes stdio, fd 0,1,2. func dupStdio(slavePath string) error { - fd, err := unix.Open(slavePath, unix.O_RDWR, 0) + fd, err := linux.Open(slavePath, unix.O_RDWR, 0) if err != nil { - return &os.PathError{ - Op: "open", - Path: slavePath, - Err: err, - } + return err } for _, i := range []int{0, 1, 2} { - if err := unix.Dup3(fd, i, 0); err != nil { + if err := linux.Dup3(fd, i, 0); err != nil { return err } } diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 3df948eeb..ec2bef074 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -24,6 +24,7 @@ import ( "github.com/opencontainers/cgroups" devices "github.com/opencontainers/cgroups/devices/config" "github.com/opencontainers/cgroups/fs2" + "github.com/opencontainers/runc/internal/linux" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/utils" ) @@ -883,12 +884,8 @@ func reOpenDevNull() error { } if stat.Rdev == devNullStat.Rdev { // Close and re-open the fd. - if err := unix.Dup3(int(file.Fd()), fd, 0); err != nil { - return &os.PathError{ - Op: "dup3", - Path: "fd " + strconv.Itoa(int(file.Fd())), - Err: err, - } + if err := linux.Dup3(int(file.Fd()), fd, 0); err != nil { + return err } } } @@ -1063,15 +1060,15 @@ func pivotRoot(rootfs string) error { // with pivot_root this allows us to pivot without creating directories in // the rootfs. Shout-outs to the LXC developers for giving us this idea. - oldroot, err := unix.Open("/", unix.O_DIRECTORY|unix.O_RDONLY, 0) + oldroot, err := linux.Open("/", unix.O_DIRECTORY|unix.O_RDONLY, 0) if err != nil { - return &os.PathError{Op: "open", Path: "/", Err: err} + return err } defer unix.Close(oldroot) - newroot, err := unix.Open(rootfs, unix.O_DIRECTORY|unix.O_RDONLY, 0) + newroot, err := linux.Open(rootfs, unix.O_DIRECTORY|unix.O_RDONLY, 0) if err != nil { - return &os.PathError{Op: "open", Path: rootfs, Err: err} + return err } defer unix.Close(newroot) diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index cccb1d89a..6bf5ba39c 100644 --- a/libcontainer/standard_init_linux.go +++ b/libcontainer/standard_init_linux.go @@ -262,9 +262,9 @@ func (l *linuxStandardInit) Init() error { // user process. We open it through /proc/self/fd/$fd, because the fd that // was given to us was an O_PATH fd to the fifo itself. Linux allows us to // re-open an O_PATH fd through /proc. - fd, err := unix.Open(fifoPath, unix.O_WRONLY|unix.O_CLOEXEC, 0) + fd, err := linux.Open(fifoPath, unix.O_WRONLY|unix.O_CLOEXEC, 0) if err != nil { - return &os.PathError{Op: "open exec fifo", Path: fifoPath, Err: err} + return err } if _, err := unix.Write(fd, []byte("0")); err != nil { return &os.PathError{Op: "write exec fifo", Path: fifoPath, Err: err} diff --git a/libcontainer/utils/utils_unix.go b/libcontainer/utils/utils_unix.go index 98caa3151..3c32769dd 100644 --- a/libcontainer/utils/utils_unix.go +++ b/libcontainer/utils/utils_unix.go @@ -14,6 +14,7 @@ import ( _ "unsafe" // for go:linkname securejoin "github.com/cyphar/filepath-securejoin" + "github.com/opencontainers/runc/internal/linux" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -358,9 +359,9 @@ func Openat(dir *os.File, path string, flags int, mode uint32) (*os.File, error) } flags |= unix.O_CLOEXEC - fd, err := unix.Openat(dirFd, path, flags, mode) + fd, err := linux.Openat(dirFd, path, flags, mode) if err != nil { - return nil, &os.PathError{Op: "openat", Path: path, Err: err} + return nil, err } return os.NewFile(uintptr(fd), dir.Name()+"/"+path), nil } diff --git a/tests/cmd/seccompagent/seccompagent.go b/tests/cmd/seccompagent/seccompagent.go index be1e6c255..e9cc72303 100644 --- a/tests/cmd/seccompagent/seccompagent.go +++ b/tests/cmd/seccompagent/seccompagent.go @@ -14,6 +14,7 @@ import ( "strings" securejoin "github.com/cyphar/filepath-securejoin" + "github.com/opencontainers/runc/internal/linux" "github.com/opencontainers/runtime-spec/specs-go" libseccomp "github.com/seccomp/libseccomp-golang" "github.com/sirupsen/logrus" @@ -124,7 +125,7 @@ func handleNewMessage(sockfd int) (uintptr, string, error) { func readArgString(pid uint32, offset int64) (string, error) { buffer := make([]byte, 4096) // PATH_MAX - memfd, err := unix.Open(fmt.Sprintf("/proc/%d/mem", pid), unix.O_RDONLY, 0o777) + memfd, err := linux.Open(fmt.Sprintf("/proc/%d/mem", pid), unix.O_RDONLY, 0o777) if err != nil { return "", err } From 491326cdeb3762a8b5f926be9bb5ddd36115e31d Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 25 Mar 2025 14:55:33 -0700 Subject: [PATCH 5/5] int/linux: add/use Recvfrom Signed-off-by: Kir Kolyshkin --- internal/linux/linux.go | 12 ++++++++++++ libcontainer/sync_unix.go | 16 +++------------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/internal/linux/linux.go b/internal/linux/linux.go index b281a3347..9b0496479 100644 --- a/internal/linux/linux.go +++ b/internal/linux/linux.go @@ -53,6 +53,18 @@ func Openat(dirfd int, path string, mode int, perm uint32) (fd int, err error) { return fd, nil } +// Recvfrom wraps [unix.Recvfrom]. +func Recvfrom(fd int, p []byte, flags int) (n int, from unix.Sockaddr, err error) { + err = retryOnEINTR(func() error { + n, from, err = unix.Recvfrom(fd, p, flags) + return err + }) + if err != nil { + return 0, nil, os.NewSyscallError("recvfrom", err) + } + return n, from, err +} + // Sendmsg wraps [unix.Sendmsg]. func Sendmsg(fd int, p, oob []byte, to unix.Sockaddr, flags int) error { err := retryOnEINTR(func() error { diff --git a/libcontainer/sync_unix.go b/libcontainer/sync_unix.go index 69c0228db..2c6e4387d 100644 --- a/libcontainer/sync_unix.go +++ b/libcontainer/sync_unix.go @@ -6,6 +6,7 @@ import ( "os" "sync/atomic" + "github.com/opencontainers/runc/internal/linux" "golang.org/x/sys/unix" ) @@ -42,20 +43,9 @@ func (s *syncSocket) WritePacket(b []byte) (int, error) { } func (s *syncSocket) ReadPacket() ([]byte, error) { - var ( - size int - err error - ) - - for { - size, _, err = unix.Recvfrom(int(s.f.Fd()), nil, unix.MSG_TRUNC|unix.MSG_PEEK) - if err != unix.EINTR { //nolint:errorlint // unix errors are bare - break - } - } - + size, _, err := linux.Recvfrom(int(s.f.Fd()), nil, unix.MSG_TRUNC|unix.MSG_PEEK) if err != nil { - return nil, fmt.Errorf("fetch packet length from socket: %w", os.NewSyscallError("recvfrom", err)) + return nil, fmt.Errorf("fetch packet length from socket: %w", err) } // We will only get a zero size if the socket has been closed from the // other end (otherwise recvfrom(2) will block until a packet is ready). In