]> Cypherpunks repositories - gostls13.git/commitdiff
syscall: clean up variable declarations in forkAndExecInChild
authorBryan C. Mills <bcmills@google.com>
Fri, 9 Dec 2022 17:30:24 +0000 (12:30 -0500)
committerGopher Robot <gobot@golang.org>
Wed, 25 Jan 2023 03:23:11 +0000 (03:23 +0000)
The various forkAndExecInChild implementations have comments
explaining that they pre-declare variables to force allocations
to occur before forking, but then later use ":=" declarations
for additional variables.

To make it clearer that those ":=" declarations do not allocate,
we move their declarations up to the predeclared blocks.

For #57208.

Change-Id: Ie8cb577fa7180b51b64d6dc398169053fdf8ea97
Reviewed-on: https://go-review.googlesource.com/c/go/+/456516
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/syscall/exec_bsd.go
src/syscall/exec_freebsd.go
src/syscall/exec_libc.go
src/syscall/exec_libc2.go
src/syscall/exec_linux.go
src/syscall/exec_plan9.go

index 32c3ebdd9b9fe9e7101beafed81a0c22495ae235..379875561b263efa67dbe1f65935996a974b5f87 100644 (file)
@@ -55,10 +55,13 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        // Declare all variables at top in case any
        // declarations require heap allocation (e.g., err1).
        var (
-               r1     uintptr
-               err1   Errno
-               nextfd int
-               i      int
+               r1              uintptr
+               err1            Errno
+               nextfd          int
+               i               int
+               pgrp            _C_int
+               cred            *Credential
+               ngroups, groups uintptr
        )
 
        // guard against side effects of shuffling fds below.
@@ -119,7 +122,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        if sys.Foreground {
                // This should really be pid_t, however _C_int (aka int32) is
                // generally equivalent.
-               pgrp := _C_int(sys.Pgid)
+               pgrp = _C_int(sys.Pgid)
                if pgrp == 0 {
                        r1, _, err1 = RawSyscall(SYS_GETPID, 0, 0, 0)
                        if err1 != 0 {
@@ -149,9 +152,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        }
 
        // User and groups
-       if cred := sys.Credential; cred != nil {
-               ngroups := uintptr(len(cred.Groups))
-               groups := uintptr(0)
+       if cred = sys.Credential; cred != nil {
+               ngroups = uintptr(len(cred.Groups))
+               groups = uintptr(0)
                if ngroups > 0 {
                        groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
                }
index af5a4158f04fec01ed1a9408b7ee2e89ed0869ee..9e1cc46c159cb80367830f9a444359daddc14886 100644 (file)
@@ -60,10 +60,14 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        // Declare all variables at top in case any
        // declarations require heap allocation (e.g., err1).
        var (
-               r1     uintptr
-               err1   Errno
-               nextfd int
-               i      int
+               r1              uintptr
+               err1            Errno
+               nextfd          int
+               i               int
+               pgrp            _C_int
+               cred            *Credential
+               ngroups, groups uintptr
+               upid            uintptr
        )
 
        // Record parent PID so child can test if it has died.
@@ -127,7 +131,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        if sys.Foreground {
                // This should really be pid_t, however _C_int (aka int32) is
                // generally equivalent.
-               pgrp := _C_int(sys.Pgid)
+               pgrp = _C_int(sys.Pgid)
                if pgrp == 0 {
                        r1, _, err1 = RawSyscall(SYS_GETPID, 0, 0, 0)
                        if err1 != 0 {
@@ -157,9 +161,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        }
 
        // User and groups
-       if cred := sys.Credential; cred != nil {
-               ngroups := uintptr(len(cred.Groups))
-               groups := uintptr(0)
+       if cred = sys.Credential; cred != nil {
+               ngroups = uintptr(len(cred.Groups))
+               groups = uintptr(0)
                if ngroups > 0 {
                        groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
                }
@@ -204,8 +208,8 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
                // using SIGKILL.
                r1, _, _ = RawSyscall(SYS_GETPPID, 0, 0, 0)
                if r1 != ppid {
-                       pid, _, _ := RawSyscall(SYS_GETPID, 0, 0, 0)
-                       _, _, err1 = RawSyscall(SYS_KILL, pid, uintptr(sys.Pdeathsig), 0)
+                       upid, _, _ = RawSyscall(SYS_GETPID, 0, 0, 0)
+                       _, _, err1 = RawSyscall(SYS_KILL, upid, uintptr(sys.Pdeathsig), 0)
                        if err1 != 0 {
                                goto childerror
                        }
index ef0c87e03c461f6c03bf8126371465d7e8db3215..f8769b9aba340b4ef887f3866949948b9244df23 100644 (file)
@@ -81,10 +81,13 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        // Declare all variables at top in case any
        // declarations require heap allocation (e.g., err1).
        var (
-               r1     uintptr
-               err1   Errno
-               nextfd int
-               i      int
+               r1              uintptr
+               err1            Errno
+               nextfd          int
+               i               int
+               pgrp            _Pid_t
+               cred            *Credential
+               ngroups, groups uintptr
        )
 
        // guard against side effects of shuffling fds below.
@@ -135,7 +138,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        }
 
        if sys.Foreground {
-               pgrp := _Pid_t(sys.Pgid)
+               pgrp = _Pid_t(sys.Pgid)
                if pgrp == 0 {
                        r1, err1 = getpid()
                        if err1 != 0 {
@@ -165,9 +168,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        }
 
        // User and groups
-       if cred := sys.Credential; cred != nil {
-               ngroups := uintptr(len(cred.Groups))
-               groups := uintptr(0)
+       if cred = sys.Credential; cred != nil {
+               ngroups = uintptr(len(cred.Groups))
+               groups = uintptr(0)
                if ngroups > 0 {
                        groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
                }
index 41bc79a721e7d4fa186d1896888f5f0b88e90732..9b04f96c81908a6ee87545f1d020c2296eb2de33 100644 (file)
@@ -52,14 +52,17 @@ func runtime_AfterForkInChild()
 // functions that do not grow the stack.
 //
 //go:norace
-func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid int, err Errno) {
+func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid int, err1 Errno) {
        // Declare all variables at top in case any
        // declarations require heap allocation (e.g., err1).
        var (
-               r1     uintptr
-               err1   Errno
-               nextfd int
-               i      int
+               r1              uintptr
+               nextfd          int
+               i               int
+               err             error
+               pgrp            _C_int
+               cred            *Credential
+               ngroups, groups uintptr
        )
 
        // guard against side effects of shuffling fds below.
@@ -94,7 +97,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
 
        // Enable tracing if requested.
        if sys.Ptrace {
-               if err := ptrace(PTRACE_TRACEME, 0, 0, 0); err != nil {
+               if err = ptrace(PTRACE_TRACEME, 0, 0, 0); err != nil {
                        err1 = err.(Errno)
                        goto childerror
                }
@@ -120,7 +123,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        if sys.Foreground {
                // This should really be pid_t, however _C_int (aka int32) is
                // generally equivalent.
-               pgrp := _C_int(sys.Pgid)
+               pgrp = _C_int(sys.Pgid)
                if pgrp == 0 {
                        r1, _, err1 = rawSyscall(abi.FuncPCABI0(libc_getpid_trampoline), 0, 0, 0)
                        if err1 != 0 {
@@ -149,9 +152,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        }
 
        // User and groups
-       if cred := sys.Credential; cred != nil {
-               ngroups := uintptr(len(cred.Groups))
-               groups := uintptr(0)
+       if cred = sys.Credential; cred != nil {
+               ngroups = uintptr(len(cred.Groups))
+               groups = uintptr(0)
                if ngroups > 0 {
                        groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
                }
index 7e0c3d250bcb984e481f828cda1db52efec15a4d..a8eb4bf927327b747786be3f6f820e0bfad48db1 100644 (file)
@@ -127,19 +127,19 @@ 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.
-       r1, err1, p, locked := forkAndExecInChild1(argv0, argv, envv, chroot, dir, attr, sys, pipe)
+       upid, err, mapPipe, locked := forkAndExecInChild1(argv0, argv, envv, chroot, dir, attr, sys, pipe)
        if locked {
                runtime_AfterFork()
        }
-       if err1 != 0 {
-               return 0, err1
+       if err != 0 {
+               return 0, err
        }
 
        // parent; return PID
-       pid = int(r1)
+       pid = int(upid)
 
        if sys.UidMappings != nil || sys.GidMappings != nil {
-               Close(p[0])
+               Close(mapPipe[0])
                var err2 Errno
                // uid/gid mappings will be written after fork and unshare(2) for user
                // namespaces.
@@ -148,8 +148,8 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
                                err2 = err.(Errno)
                        }
                }
-               RawSyscall(SYS_WRITE, uintptr(p[1]), uintptr(unsafe.Pointer(&err2)), unsafe.Sizeof(err2))
-               Close(p[1])
+               RawSyscall(SYS_WRITE, uintptr(mapPipe[1]), uintptr(unsafe.Pointer(&err2)), unsafe.Sizeof(err2))
+               Close(mapPipe[1])
        }
 
        return pid, 0
@@ -203,7 +203,7 @@ type cloneArgs struct {
 //
 //go:noinline
 //go:norace
-func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (r1 uintptr, err1 Errno, p [2]int, locked bool) {
+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) {
        // Defined in linux/prctl.h starting with Linux 4.3.
        const (
                PR_CAP_AMBIENT       = 0x2f
@@ -215,8 +215,15 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
        // processing in this stack frame and never returns, while the
        // parent returns immediately from this frame and does all
        // post-fork processing in the outer frame.
+       //
        // Declare all variables at top in case any
-       // declarations require heap allocation (e.g., err1).
+       // declarations require heap allocation (e.g., err2).
+       // ":=" should not be used to declare any variable after
+       // the call to runtime_BeforeFork.
+       //
+       // NOTE(bcmills): The allocation behavior described in the above comment
+       // seems to lack a corresponding test, and it may be rendered invalid
+       // by an otherwise-correct change in the compiler.
        var (
                err2                      Errno
                nextfd                    int
@@ -226,6 +233,11 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
                puid, psetgroups, pgid    []byte
                uidmap, setgroups, gidmap []byte
                clone3                    *cloneArgs
+               pgrp                      int32
+               dirfd                     int
+               cred                      *Credential
+               ngroups, groups           uintptr
+               c                         uintptr
        )
 
        if sys.UidMappings != nil {
@@ -264,7 +276,7 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
        // Allocate another pipe for parent to child communication for
        // synchronizing writing of User ID/Group ID mappings.
        if sys.UidMappings != nil || sys.GidMappings != nil {
-               if err := forkExecPipe(p[:]); err != nil {
+               if err := forkExecPipe(mapPipe[:]); err != nil {
                        err1 = err.(Errno)
                        return
                }
@@ -288,17 +300,17 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
        runtime_BeforeFork()
        locked = true
        if clone3 != nil {
-               r1, err1 = rawVforkSyscall(_SYS_clone3, uintptr(unsafe.Pointer(clone3)), unsafe.Sizeof(*clone3))
+               pid, err1 = rawVforkSyscall(_SYS_clone3, uintptr(unsafe.Pointer(clone3)), unsafe.Sizeof(*clone3))
        } else {
                flags |= uintptr(SIGCHLD)
                if runtime.GOARCH == "s390x" {
                        // On Linux/s390, the first two arguments of clone(2) are swapped.
-                       r1, err1 = rawVforkSyscall(SYS_CLONE, 0, flags)
+                       pid, err1 = rawVforkSyscall(SYS_CLONE, 0, flags)
                } else {
-                       r1, err1 = rawVforkSyscall(SYS_CLONE, flags, 0)
+                       pid, err1 = rawVforkSyscall(SYS_CLONE, flags, 0)
                }
        }
-       if err1 != 0 || r1 != 0 {
+       if err1 != 0 || pid != 0 {
                // If we're in the parent, we must return immediately
                // so we're not in the same stack frame as the child.
                // This can at most use the return PC, which the child
@@ -320,14 +332,14 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
 
        // Wait for User ID/Group ID mappings to be written.
        if sys.UidMappings != nil || sys.GidMappings != nil {
-               if _, _, err1 = RawSyscall(SYS_CLOSE, uintptr(p[1]), 0, 0); err1 != 0 {
+               if _, _, err1 = RawSyscall(SYS_CLOSE, uintptr(mapPipe[1]), 0, 0); err1 != 0 {
                        goto childerror
                }
-               r1, _, err1 = RawSyscall(SYS_READ, uintptr(p[0]), uintptr(unsafe.Pointer(&err2)), unsafe.Sizeof(err2))
+               pid, _, err1 = RawSyscall(SYS_READ, uintptr(mapPipe[0]), uintptr(unsafe.Pointer(&err2)), unsafe.Sizeof(err2))
                if err1 != 0 {
                        goto childerror
                }
-               if r1 != unsafe.Sizeof(err2) {
+               if pid != unsafe.Sizeof(err2) {
                        err1 = EINVAL
                        goto childerror
                }
@@ -355,11 +367,11 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
        }
 
        if sys.Foreground {
-               pgrp := int32(sys.Pgid)
+               pgrp = int32(sys.Pgid)
                if pgrp == 0 {
-                       r1, _ = rawSyscallNoError(SYS_GETPID, 0, 0, 0)
+                       pid, _ = rawSyscallNoError(SYS_GETPID, 0, 0, 0)
 
-                       pgrp = int32(r1)
+                       pgrp = int32(pid)
                }
 
                // Place process group in foreground.
@@ -381,11 +393,11 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
                }
 
                if sys.Unshareflags&CLONE_NEWUSER != 0 && sys.GidMappings != nil {
-                       dirfd := int(_AT_FDCWD)
+                       dirfd = int(_AT_FDCWD)
                        if fd1, _, err1 = RawSyscall6(SYS_OPENAT, uintptr(dirfd), uintptr(unsafe.Pointer(&psetgroups[0])), uintptr(O_WRONLY), 0, 0, 0); err1 != 0 {
                                goto childerror
                        }
-                       r1, _, err1 = RawSyscall(SYS_WRITE, uintptr(fd1), uintptr(unsafe.Pointer(&setgroups[0])), uintptr(len(setgroups)))
+                       pid, _, err1 = RawSyscall(SYS_WRITE, uintptr(fd1), uintptr(unsafe.Pointer(&setgroups[0])), uintptr(len(setgroups)))
                        if err1 != 0 {
                                goto childerror
                        }
@@ -396,7 +408,7 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
                        if fd1, _, err1 = RawSyscall6(SYS_OPENAT, uintptr(dirfd), uintptr(unsafe.Pointer(&pgid[0])), uintptr(O_WRONLY), 0, 0, 0); err1 != 0 {
                                goto childerror
                        }
-                       r1, _, err1 = RawSyscall(SYS_WRITE, uintptr(fd1), uintptr(unsafe.Pointer(&gidmap[0])), uintptr(len(gidmap)))
+                       pid, _, err1 = RawSyscall(SYS_WRITE, uintptr(fd1), uintptr(unsafe.Pointer(&gidmap[0])), uintptr(len(gidmap)))
                        if err1 != 0 {
                                goto childerror
                        }
@@ -406,11 +418,11 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
                }
 
                if sys.Unshareflags&CLONE_NEWUSER != 0 && sys.UidMappings != nil {
-                       dirfd := int(_AT_FDCWD)
+                       dirfd = int(_AT_FDCWD)
                        if fd1, _, err1 = RawSyscall6(SYS_OPENAT, uintptr(dirfd), uintptr(unsafe.Pointer(&puid[0])), uintptr(O_WRONLY), 0, 0, 0); err1 != 0 {
                                goto childerror
                        }
-                       r1, _, err1 = RawSyscall(SYS_WRITE, uintptr(fd1), uintptr(unsafe.Pointer(&uidmap[0])), uintptr(len(uidmap)))
+                       pid, _, err1 = RawSyscall(SYS_WRITE, uintptr(fd1), uintptr(unsafe.Pointer(&uidmap[0])), uintptr(len(uidmap)))
                        if err1 != 0 {
                                goto childerror
                        }
@@ -443,9 +455,9 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
        }
 
        // User and groups
-       if cred := sys.Credential; cred != nil {
-               ngroups := uintptr(len(cred.Groups))
-               groups := uintptr(0)
+       if cred = sys.Credential; cred != nil {
+               ngroups = uintptr(len(cred.Groups))
+               groups = uintptr(0)
                if ngroups > 0 {
                        groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
                }
@@ -474,7 +486,7 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
                        goto childerror
                }
 
-               for _, c := range sys.AmbientCaps {
+               for _, c = range sys.AmbientCaps {
                        // Add the c capability to the permitted and inheritable capability mask,
                        // otherwise we will not be able to add it to the ambient capability mask.
                        caps.data[capToIndex(c)].permitted |= capToMask(c)
@@ -485,7 +497,7 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
                        goto childerror
                }
 
-               for _, c := range sys.AmbientCaps {
+               for _, c = range sys.AmbientCaps {
                        _, _, err1 = RawSyscall6(SYS_PRCTL, PR_CAP_AMBIENT, uintptr(PR_CAP_AMBIENT_RAISE), c, 0, 0, 0)
                        if err1 != 0 {
                                goto childerror
@@ -511,9 +523,9 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
                // Signal self if parent is already dead. This might cause a
                // duplicate signal in rare cases, but it won't matter when
                // using SIGKILL.
-               r1, _ = rawSyscallNoError(SYS_GETPPID, 0, 0, 0)
-               if r1 != ppid {
-                       pid, _ := rawSyscallNoError(SYS_GETPID, 0, 0, 0)
+               pid, _ = rawSyscallNoError(SYS_GETPPID, 0, 0, 0)
+               if pid != ppid {
+                       pid, _ = rawSyscallNoError(SYS_GETPID, 0, 0, 0)
                        _, _, err1 = RawSyscall(SYS_KILL, pid, uintptr(sys.Pdeathsig), 0)
                        if err1 != 0 {
                                goto childerror
index 8f28b5aa22adbefc2d7f18f356e7323e4c9aa51a..8762237825c78589f7c379bcbdb360ee0376b5ff 100644 (file)
@@ -135,6 +135,8 @@ func forkAndExecInChild(argv0 *byte, argv []*byte, envv []envItem, dir *byte, at
                errbuf   [ERRMAX]byte
                statbuf  [STATMAX]byte
                dupdevfd int
+               n        int
+               b        []byte
        )
 
        // Guard against side effects of shuffling fds below.
@@ -177,14 +179,14 @@ func forkAndExecInChild(argv0 *byte, argv []*byte, envv []envItem, dir *byte, at
 dirloop:
        for {
                r1, _, _ = RawSyscall6(SYS_PREAD, uintptr(dupdevfd), uintptr(unsafe.Pointer(&statbuf[0])), uintptr(len(statbuf)), ^uintptr(0), ^uintptr(0), 0)
-               n := int(r1)
+               n = int(r1)
                switch n {
                case -1:
                        goto childerror
                case 0:
                        break dirloop
                }
-               for b := statbuf[:n]; len(b) > 0; {
+               for b = statbuf[:n]; len(b) > 0; {
                        var s []byte
                        s, b = gdirname(b)
                        if s == nil {