diff --git a/core/mount/mount_idmapped_utils_linux.go b/core/mount/mount_idmapped_utils_linux.go index 373285be8d..a1b79fb469 100644 --- a/core/mount/mount_idmapped_utils_linux.go +++ b/core/mount/mount_idmapped_utils_linux.go @@ -1,5 +1,3 @@ -//go:build go1.23 && linux - /* Copyright The containerd Authors. @@ -29,17 +27,8 @@ import ( ) // getUsernsFD returns pinnable user namespace's file descriptor. -// -// NOTE: The GO runtime uses pidfd to handle subprocess since go1.23. However, -// it has double close issue tracked by [1]. We can't use pidfd directly and -// the GO runtime doesn't export interface to show if it's using pidfd or not. -// So, we call `sys.SupportsPidFD` first and then use `os.Process` directly. -// -// [1]: https://github.com/golang/go/issues/68984 func getUsernsFD(uidMaps, gidMaps []syscall.SysProcIDMap) (_ *os.File, retErr error) { - if !sys.SupportsPidFD() { - return nil, fmt.Errorf("failed to prevent pid reused issue because pidfd isn't supported") - } + var pidfd int proc, err := os.StartProcess("/proc/self/exe", []string{"containerd[getUsernsFD]"}, &os.ProcAttr{ Sys: &syscall.SysProcAttr{ @@ -50,15 +39,26 @@ func getUsernsFD(uidMaps, gidMaps []syscall.SysProcIDMap) (_ *os.File, retErr er // be in PTRACE_TRACEME mode before performing execve. Ptrace: true, Pdeathsig: syscall.SIGKILL, + PidFD: &pidfd, }, }) if err != nil { return nil, fmt.Errorf("failed to start noop process for unshare: %w", err) } - defer func() { + if pidfd == -1 || !sys.SupportsPidFD() { proc.Kill() proc.Wait() + return nil, fmt.Errorf("failed to prevent pid reused issue because pidfd isn't supported") + } + + pidFD := os.NewFile(uintptr(pidfd), "pidfd") + defer func() { + unix.PidfdSendSignal(int(pidFD.Fd()), unix.SIGKILL, nil, 0) + + pidfdWaitid(pidFD) + + pidFD.Close() }() // NOTE: @@ -78,8 +78,14 @@ func getUsernsFD(uidMaps, gidMaps []syscall.SysProcIDMap) (_ *os.File, retErr er // Ensure the child process is still alive. If the err is ESRCH, we // should return error because we can't guarantee the usernsFD and // u[g]idmapFile are valid. It's safe to return error and retry. - if err := proc.Signal(syscall.Signal(0)); err != nil { + if err := unix.PidfdSendSignal(int(pidFD.Fd()), 0, nil, 0); err != nil { return nil, fmt.Errorf("failed to ensure child process is alive: %w", err) } return usernsFD, nil } + +func pidfdWaitid(pidFD *os.File) error { + return sys.IgnoringEINTR(func() error { + return unix.Waitid(unix.P_PIDFD, int(pidFD.Fd()), nil, unix.WEXITED, nil) + }) +} diff --git a/core/mount/mount_idmapped_utils_linux_go122.go b/core/mount/mount_idmapped_utils_linux_go122.go deleted file mode 100644 index 669f395d20..0000000000 --- a/core/mount/mount_idmapped_utils_linux_go122.go +++ /dev/null @@ -1,93 +0,0 @@ -//go:build !go1.23 && linux - -/* - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package mount - -import ( - "fmt" - "os" - "syscall" - - "github.com/containerd/containerd/v2/pkg/sys" - - "golang.org/x/sys/unix" -) - -// getUsernsFD returns pinnable user namespace's file descriptor. -func getUsernsFD(uidMaps, gidMaps []syscall.SysProcIDMap) (_ *os.File, retErr error) { - var pidfd int - - proc, err := os.StartProcess("/proc/self/exe", []string{"containerd[getUsernsFD]"}, &os.ProcAttr{ - Sys: &syscall.SysProcAttr{ - Cloneflags: unix.CLONE_NEWUSER, - UidMappings: uidMaps, - GidMappings: gidMaps, - // NOTE: It's reexec but it's not heavy because subprocess - // be in PTRACE_TRACEME mode before performing execve. - Ptrace: true, - Pdeathsig: syscall.SIGKILL, - PidFD: &pidfd, - }, - }) - if err != nil { - return nil, fmt.Errorf("failed to start noop process for unshare: %w", err) - } - - if pidfd == -1 || !sys.SupportsPidFD() { - proc.Kill() - proc.Wait() - return nil, fmt.Errorf("failed to prevent pid reused issue because pidfd isn't supported") - } - - pidFD := os.NewFile(uintptr(pidfd), "pidfd") - defer func() { - unix.PidfdSendSignal(int(pidFD.Fd()), unix.SIGKILL, nil, 0) - - pidfdWaitid(pidFD) - - pidFD.Close() - }() - - // NOTE: - // - // The usernsFD will hold the userns reference in kernel. Even if the - // child process is reaped, the usernsFD is still valid. - usernsFD, err := os.Open(fmt.Sprintf("/proc/%d/ns/user", proc.Pid)) - if err != nil { - return nil, fmt.Errorf("failed to get userns file descriptor for /proc/%d/user/ns: %w", proc.Pid, err) - } - defer func() { - if retErr != nil { - usernsFD.Close() - } - }() - - // Ensure the child process is still alive. If the err is ESRCH, we - // should return error because we can't guarantee the usernsFD and - // u[g]idmapFile are valid. It's safe to return error and retry. - if err := unix.PidfdSendSignal(int(pidFD.Fd()), 0, nil, 0); err != nil { - return nil, fmt.Errorf("failed to ensure child process is alive: %w", err) - } - return usernsFD, nil -} - -func pidfdWaitid(pidFD *os.File) error { - return sys.IgnoringEINTR(func() error { - return unix.Waitid(unix.P_PIDFD, int(pidFD.Fd()), nil, unix.WEXITED, nil) - }) -} diff --git a/pkg/sys/unshare_linux.go b/pkg/sys/unshare_linux.go index 85af4fe3b8..797e8006dc 100644 --- a/pkg/sys/unshare_linux.go +++ b/pkg/sys/unshare_linux.go @@ -20,7 +20,6 @@ import ( "errors" "fmt" "os" - "runtime" "strconv" "strings" "syscall" @@ -71,21 +70,6 @@ func UnshareAfterEnterUserns(uidMap, gidMap string, unshareFlags uintptr, f func return fmt.Errorf("kernel doesn't support CLONE_PIDFD") } - // Since go1.23.{0,1} has double close issue, we should dup it before using it. - // - // References: - // - https://github.com/golang/go/issues/68984 - // - https://github.com/golang/go/milestone/371 - if goVer := runtime.Version(); goVer == "go1.23.0" || goVer == "go1.23.1" { - dupPidfd, err := unix.FcntlInt(uintptr(pidfd), syscall.F_DUPFD_CLOEXEC, 0) - if err != nil { - proc.Kill() - proc.Wait() - return fmt.Errorf("failed to dupfd: %w", err) - } - pidfd = dupPidfd - } - defer func() { derr := unix.PidfdSendSignal(pidfd, unix.SIGKILL, nil, 0) if derr != nil {