1
0
mirror of https://github.com/golang/go synced 2024-11-17 06:34:53 -07:00

syscall: fix ForkLock spurious close(0) on pipe failure

Pipe (and therefore forkLockPipe) does not make any guarantees
about the state of p after a failed Pipe(p). Avoid that assumption
and the too-clever goto, so that we don't accidentally Close a real fd
if the failed pipe leaves p[0] or p[1] set >= 0.

Fixes #50057
Fixes CVE-2021-44717

Change-Id: Iff8e19a6efbba0c73cc8b13ecfae381c87600bb4
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1291270
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/370576
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
This commit is contained in:
Russ Cox 2021-12-08 18:05:11 -05:00 committed by Filippo Valsorda
parent d6c4583ad4
commit a76511f3a4

View File

@ -152,9 +152,6 @@ func forkExec(argv0 string, argv []string, attr *ProcAttr) (pid int, err error)
sys = &zeroSysProcAttr sys = &zeroSysProcAttr
} }
p[0] = -1
p[1] = -1
// Convert args to C form. // Convert args to C form.
argv0p, err := BytePtrFromString(argv0) argv0p, err := BytePtrFromString(argv0)
if err != nil { if err != nil {
@ -204,14 +201,17 @@ func forkExec(argv0 string, argv []string, attr *ProcAttr) (pid int, err error)
// Allocate child status pipe close on exec. // Allocate child status pipe close on exec.
if err = forkExecPipe(p[:]); err != nil { if err = forkExecPipe(p[:]); err != nil {
goto error ForkLock.Unlock()
return 0, err
} }
// Kick off child. // Kick off child.
pid, err1 = forkAndExecInChild(argv0p, argvp, envvp, chroot, dir, attr, sys, p[1]) pid, err1 = forkAndExecInChild(argv0p, argvp, envvp, chroot, dir, attr, sys, p[1])
if err1 != 0 { if err1 != 0 {
err = Errno(err1) Close(p[0])
goto error Close(p[1])
ForkLock.Unlock()
return 0, Errno(err1)
} }
ForkLock.Unlock() ForkLock.Unlock()
@ -243,14 +243,6 @@ func forkExec(argv0 string, argv []string, attr *ProcAttr) (pid int, err error)
// Read got EOF, so pipe closed on exec, so exec succeeded. // Read got EOF, so pipe closed on exec, so exec succeeded.
return pid, nil return pid, nil
error:
if p[0] >= 0 {
Close(p[0])
Close(p[1])
}
ForkLock.Unlock()
return 0, err
} }
// Combination of fork and exec, careful to be thread safe. // Combination of fork and exec, careful to be thread safe.