]> Cypherpunks repositories - gostls13.git/commitdiff
syscall: fix getting pidfd when using CLONE_NEWUSER
authorKir Kolyshkin <kolyshkin@gmail.com>
Thu, 16 Nov 2023 08:42:04 +0000 (00:42 -0800)
committerGopher Robot <gobot@golang.org>
Tue, 21 Nov 2023 20:13:01 +0000 (20:13 +0000)
While working on CL 528798, I found out that sys.PidFD field (added
in CL 520266) is not filled in when CLONE_NEWUSER is used.

This happens because the code assumed that the parent and the child
run in the same memory space. This assumption is right only when
CLONE_VM is used for clone syscall, and the code only sets CLONE_VM
when CLONE_NEWUSER is not used.

Fix this, and add a test case (which fails before the fix).

Updates #51246.

Change-Id: I805203c1369cadd63d769568b132a9ffd92cc184
Reviewed-on: https://go-review.googlesource.com/c/go/+/542698
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>

src/syscall/exec_linux.go
src/syscall/exec_linux_test.go

index e1c71b5a347f8f15e9ef41a00791110f9af80a0d..e6d6343ed889cdce78452d02c495c2d938949c86 100644 (file)
@@ -133,7 +133,7 @@ func runtime_AfterForkInChild()
 func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid int, err Errno) {
        // Set up and fork. This returns immediately in the parent or
        // if there's an error.
-       upid, err, mapPipe, locked := forkAndExecInChild1(argv0, argv, envv, chroot, dir, attr, sys, pipe)
+       upid, pidfd, err, mapPipe, locked := forkAndExecInChild1(argv0, argv, envv, chroot, dir, attr, sys, pipe)
        if locked {
                runtime_AfterFork()
        }
@@ -143,6 +143,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
 
        // parent; return PID
        pid = int(upid)
+       if sys.PidFD != nil {
+               *sys.PidFD = int(pidfd)
+       }
 
        if sys.UidMappings != nil || sys.GidMappings != nil {
                Close(mapPipe[0])
@@ -210,7 +213,7 @@ type cloneArgs struct {
 //go:noinline
 //go:norace
 //go:nocheckptr
-func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid uintptr, err1 Errno, mapPipe [2]int, locked bool) {
+func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid uintptr, pidfd int32, err1 Errno, mapPipe [2]int, locked bool) {
        // Defined in linux/prctl.h starting with Linux 4.3.
        const (
                PR_CAP_AMBIENT       = 0x2f
@@ -241,12 +244,12 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
                uidmap, setgroups, gidmap []byte
                clone3                    *cloneArgs
                pgrp                      int32
-               pidfd                     _C_int = -1
                dirfd                     int
                cred                      *Credential
                ngroups, groups           uintptr
                c                         uintptr
        )
+       pidfd = -1
 
        rlim := origRlimitNofile.Load()
 
@@ -341,10 +344,6 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
 
        // Fork succeeded, now in child.
 
-       if sys.PidFD != nil {
-               *sys.PidFD = int(pidfd)
-       }
-
        // Enable the "keep capabilities" flag to set ambient capabilities later.
        if len(sys.AmbientCaps) > 0 {
                _, _, err1 = RawSyscall6(SYS_PRCTL, PR_SET_KEEPCAPS, 1, 0, 0, 0, 0)
index f255930aa82da84129ae0a07254b3a37c5129841..a7af00d2c00a618d18ec7cec61d1a07386ac0e31 100644 (file)
@@ -522,7 +522,7 @@ func TestCloneTimeNamespace(t *testing.T) {
        }
 }
 
-func testPidFD(t *testing.T) error {
+func testPidFD(t *testing.T, userns bool) error {
        testenv.MustHaveExec(t)
 
        if os.Getenv("GO_WANT_HELPER_PROCESS") == "1" {
@@ -541,6 +541,9 @@ func testPidFD(t *testing.T) error {
        cmd.SysProcAttr = &syscall.SysProcAttr{
                PidFD: &pidfd,
        }
+       if userns {
+               cmd.SysProcAttr.Cloneflags = syscall.CLONE_NEWUSER
+       }
        if err := cmd.Start(); err != nil {
                return err
        }
@@ -572,7 +575,13 @@ func testPidFD(t *testing.T) error {
 }
 
 func TestPidFD(t *testing.T) {
-       if err := testPidFD(t); err != nil {
+       if err := testPidFD(t, false); err != nil {
+               t.Fatal("can't start a process:", err)
+       }
+}
+
+func TestPidFDWithUserNS(t *testing.T) {
+       if err := testPidFD(t, true); err != nil {
                t.Fatal("can't start a process:", err)
        }
 }
@@ -581,7 +590,7 @@ func TestPidFDClone3(t *testing.T) {
        *syscall.ForceClone3 = true
        defer func() { *syscall.ForceClone3 = false }()
 
-       if err := testPidFD(t); err != nil {
+       if err := testPidFD(t, false); err != nil {
                if testenv.SyscallIsNotSupported(err) {
                        t.Skip("clone3 not supported:", err)
                }