Merge pull request #4697 from kolyshkin/eintr

Introduce/use internal/linux pkg to handle EINTR and error wrapping

internal/linux/eintr.go

@@ -0,0 +1,28 @@
+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
+		}
+	}
+}
+
+// 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
+		}
+	}
+}

internal/linux/linux.go

@@ -0,0 +1,74 @@
+package linux
+
+import (
+	"os"
+
+	"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 {
+		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)
+	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
+}
+
+// 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 {
+		return unix.Sendmsg(fd, p, oob, to, flags)
+	})
+	return os.NewSyscallError("sendmsg", err)
+}

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{
+		return err
-			Op:   "open",
-			Path: slavePath,
-			Err:  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
 		}
 	}

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)

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 {
+			if err := linux.Dup3(int(file.Fd()), fd, 0); err != nil {
-				return &os.PathError{
+				return err
-					Op:   "dup3",
-					Path: "fd " + strconv.Itoa(int(file.Fd())),
-					Err:  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)
 

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)
 }

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
+	// Runc's cwd will always be the bundle path.
-	cwd, err := getwd()
+	// 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
 	}

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"
@@ -261,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}
@@ -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)
 }

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, _, err := linux.Recvfrom(int(s.f.Fd()), nil, unix.MSG_TRUNC|unix.MSG_PEEK)
-		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
-		}
-	}
-
 	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

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

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 {
+	return linux.Sendmsg(int(socket.Fd()), []byte(msg), oob, nil, 0)
-		err := unix.Sendmsg(int(socket.Fd()), []byte(msg), oob, nil, 0)
-		if err != unix.EINTR {
-			return os.NewSyscallError("sendmsg", err)
-		}
-	}
 }

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
 }

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.

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
 	}