From 474ebb917cb802bf1d08434a265515d50c174082 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 8 Dec 2021 18:06:41 -0500 Subject: [PATCH] syscall: avoid writing to p when Pipe(p) fails Generally speaking Go functions make no guarantees about what has happened to result parameters on error, and Pipe is no exception: callers should avoid looking at p if Pipe returns an error. However, we had a bug in which ForkExec was using the content of p after a failed Pipe, and others may too. As a robustness fix, make Pipe avoid writing to p on failure. Updates #50057 Change-Id: Ie8955025dbd20702fabadc9bbe1d1a5ac0f36305 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1291271 Reviewed-by: Ian Lance Taylor Reviewed-on: https://go-review.googlesource.com/c/go/+/370577 Run-TryBot: Filippo Valsorda Trust: Russ Cox TryBot-Result: Gopher Robot Reviewed-by: Alex Rakoczy --- src/syscall/syscall_aix.go | 6 ++++-- src/syscall/syscall_darwin.go | 6 ++++-- src/syscall/syscall_dragonfly.go | 12 +++++++++--- src/syscall/syscall_freebsd.go | 6 ++++-- src/syscall/syscall_linux.go | 6 ++++-- src/syscall/syscall_netbsd.go | 6 ++++-- src/syscall/syscall_openbsd.go | 6 ++++-- src/syscall/syscall_plan9.go | 6 ++++-- src/syscall/syscall_solaris.go | 4 +++- 9 files changed, 40 insertions(+), 18 deletions(-) diff --git a/src/syscall/syscall_aix.go b/src/syscall/syscall_aix.go index 0f5101999ff96..739c55f1793f2 100644 --- a/src/syscall/syscall_aix.go +++ b/src/syscall/syscall_aix.go @@ -66,8 +66,10 @@ func Pipe(p []int) (err error) { } var pp [2]_C_int err = pipe(&pp) - p[0] = int(pp[0]) - p[1] = int(pp[1]) + if err == nil { + p[0] = int(pp[0]) + p[1] = int(pp[1]) + } return } diff --git a/src/syscall/syscall_darwin.go b/src/syscall/syscall_darwin.go index 5bb34e300c5f5..902d6e77e11cb 100644 --- a/src/syscall/syscall_darwin.go +++ b/src/syscall/syscall_darwin.go @@ -80,8 +80,10 @@ func Pipe(p []int) (err error) { } var q [2]int32 err = pipe(&q) - p[0] = int(q[0]) - p[1] = int(q[1]) + if err == nil { + p[0] = int(q[0]) + p[1] = int(q[1]) + } return } diff --git a/src/syscall/syscall_dragonfly.go b/src/syscall/syscall_dragonfly.go index cc92c4a93edc9..f3c0f545214c8 100644 --- a/src/syscall/syscall_dragonfly.go +++ b/src/syscall/syscall_dragonfly.go @@ -98,8 +98,11 @@ func Pipe(p []int) (err error) { if len(p) != 2 { return EINVAL } - p[0], p[1], err = pipe() - return + r, w, err := pipe() + if err == nil { + p[0], p[1] = r, w + } + return err } //sysnb pipe2(p *[2]_C_int, flags int) (r int, w int, err error) @@ -111,7 +114,10 @@ func Pipe2(p []int, flags int) (err error) { var pp [2]_C_int // pipe2 on dragonfly takes an fds array as an argument, but still // returns the file descriptors. - p[0], p[1], err = pipe2(&pp, flags) + r, w, err := pipe2(&pp, flags) + if err == nil { + p[0], p[1] = r, w + } return err } diff --git a/src/syscall/syscall_freebsd.go b/src/syscall/syscall_freebsd.go index 6f44b25cb9a41..ecb9ec825aa0d 100644 --- a/src/syscall/syscall_freebsd.go +++ b/src/syscall/syscall_freebsd.go @@ -105,8 +105,10 @@ func Pipe2(p []int, flags int) error { } var pp [2]_C_int err := pipe2(&pp, flags) - p[0] = int(pp[0]) - p[1] = int(pp[1]) + if err == nil { + p[0] = int(pp[0]) + p[1] = int(pp[1]) + } return err } diff --git a/src/syscall/syscall_linux.go b/src/syscall/syscall_linux.go index c002299641266..abcf1d5dfefc5 100644 --- a/src/syscall/syscall_linux.go +++ b/src/syscall/syscall_linux.go @@ -173,8 +173,10 @@ func Pipe2(p []int, flags int) error { } var pp [2]_C_int err := pipe2(&pp, flags) - p[0] = int(pp[0]) - p[1] = int(pp[1]) + if err == nil { + p[0] = int(pp[0]) + p[1] = int(pp[1]) + } return err } diff --git a/src/syscall/syscall_netbsd.go b/src/syscall/syscall_netbsd.go index cebef10be8859..0d562cc78e42b 100644 --- a/src/syscall/syscall_netbsd.go +++ b/src/syscall/syscall_netbsd.go @@ -114,8 +114,10 @@ func Pipe2(p []int, flags int) error { } var pp [2]_C_int err := pipe2(&pp, flags) - p[0] = int(pp[0]) - p[1] = int(pp[1]) + if err == nil { + p[0] = int(pp[0]) + p[1] = int(pp[1]) + } return err } diff --git a/src/syscall/syscall_openbsd.go b/src/syscall/syscall_openbsd.go index 195cf8617ca70..fa939ec5c8849 100644 --- a/src/syscall/syscall_openbsd.go +++ b/src/syscall/syscall_openbsd.go @@ -72,8 +72,10 @@ func Pipe2(p []int, flags int) error { } var pp [2]_C_int err := pipe2(&pp, flags) - p[0] = int(pp[0]) - p[1] = int(pp[1]) + if err == nil { + p[0] = int(pp[0]) + p[1] = int(pp[1]) + } return err } diff --git a/src/syscall/syscall_plan9.go b/src/syscall/syscall_plan9.go index d16cad45d851d..6a8ab97dc6cc9 100644 --- a/src/syscall/syscall_plan9.go +++ b/src/syscall/syscall_plan9.go @@ -198,8 +198,10 @@ func Pipe(p []int) (err error) { } var pp [2]int32 err = pipe(&pp) - p[0] = int(pp[0]) - p[1] = int(pp[1]) + if err == nil { + p[0] = int(pp[0]) + p[1] = int(pp[1]) + } return } diff --git a/src/syscall/syscall_solaris.go b/src/syscall/syscall_solaris.go index 5f12f229c4c78..f44a9e25ac97b 100644 --- a/src/syscall/syscall_solaris.go +++ b/src/syscall/syscall_solaris.go @@ -57,7 +57,9 @@ func Pipe(p []int) (err error) { if e1 != 0 { err = Errno(e1) } - p[0], p[1] = int(r0), int(w0) + if err == nil { + p[0], p[1] = int(r0), int(w0) + } return }