[go] net: check error from syscall.Getsockname in (*netFD).dial

[Bryan Mills (Gerrit)]

Bryan Mills submitted this change.

View Change

Approvals: Ian Lance Taylor: Looks good to me, approved Bryan Mills: Trusted; Run TryBots Gopher Robot: TryBots succeeded

net: in (*netFD).dial, use the passed in local address if getsockname fails

'man getsockname' lists a number of possible failure modes, including
ENOBUFS (for resource exhaustion) and EBADF (which we could possibly
see in the event of a bug or race condition elsewhere in the program).

If getsockname fails for an explicit user-provided local address, the
user is probably not expecting LocalAddr on the returned net.Conn to
return nil. This may or may not fix #34611, but should at least help
us diagnose it more clearly.

While we're add it, also add more nil-checking logic in the test based
on the stack traces posted to
https://golang.org/issue/34611#issuecomment-975923748.

For #34611

Change-Id: Iba870b96787811e4b9959b74ef648afce9316602
Reviewed-on: https://go-review.googlesource.com/c/go/+/366536
Trust: Bryan Mills <bcm...@google.com>
Run-TryBot: Bryan Mills <bcm...@google.com>
Reviewed-by: Ian Lance Taylor <ia...@golang.org>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M src/net/server_test.go
M src/net/sock_posix.go
2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/src/net/server_test.go b/src/net/server_test.go
index 33d33b0..b69cd29 100644
--- a/src/net/server_test.go
+++ b/src/net/server_test.go
@@ -200,9 +200,17 @@
 				if c == nil {
 					panic("Dial returned a nil Conn")
 				}
-				if rc := reflect.ValueOf(c); rc.Kind() == reflect.Pointer && rc.IsNil() {
+				rc := reflect.ValueOf(c)
+				if rc.IsNil() {
 					panic(fmt.Sprintf("Dial returned a nil %T", c))
 				}
+				fd := rc.Elem().FieldByName("fd")
+				if fd.IsNil() {
+					panic(fmt.Sprintf("Dial returned a %T with a nil fd", c))
+				}
+				if addr := fd.Elem().FieldByName("laddr"); addr.IsNil() {
+					panic(fmt.Sprintf("Dial returned a %T whose fd has a nil laddr", c))
+				}
 				addr := c.LocalAddr()
 				if addr == nil {
 					panic(fmt.Sprintf("(%T).LocalAddr returned a nil Addr", c))
diff --git a/src/net/sock_posix.go b/src/net/sock_posix.go
index 98a4822..603fb2b 100644
--- a/src/net/sock_posix.go
+++ b/src/net/sock_posix.go
@@ -156,18 +156,24 @@
 		}
 	}
 	// Record the local and remote addresses from the actual socket.
-	// Get the local address by calling Getsockname.
+	// For the local address, use
+	// 1) the one returned by Getsockname, if that succeeds; or
+	// 2) the one passed to us as the laddr parameter; or
+	// 3) nil.
 	// For the remote address, use
 	// 1) the one returned by the connect method, if any; or
 	// 2) the one from Getpeername, if it succeeds; or
 	// 3) the one passed to us as the raddr parameter.
-	lsa, _ = syscall.Getsockname(fd.pfd.Sysfd)
+	var laddrName Addr = laddr
+	if lsa, err := syscall.Getsockname(fd.pfd.Sysfd); err == nil {
+		laddrName = fd.addrFunc()(lsa)
+	}
 	if crsa != nil {
-		fd.setAddr(fd.addrFunc()(lsa), fd.addrFunc()(crsa))
+		fd.setAddr(laddrName, fd.addrFunc()(crsa))
 	} else if rsa, _ = syscall.Getpeername(fd.pfd.Sysfd); rsa != nil {
-		fd.setAddr(fd.addrFunc()(lsa), fd.addrFunc()(rsa))
+		fd.setAddr(laddrName, fd.addrFunc()(rsa))
 	} else {
-		fd.setAddr(fd.addrFunc()(lsa), raddr)
+		fd.setAddr(laddrName, raddr)
 	}
 	return nil
 }

To view, visit change 366536. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: Iba870b96787811e4b9959b74ef648afce9316602

Gerrit-Change-Number: 366536

Gerrit-PatchSet: 7

Gerrit-Owner: Bryan Mills <bcm...@google.com>

Gerrit-Reviewer: Bryan Mills <bcm...@google.com>

Gerrit-Reviewer: Gopher Robot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-MessageType: merged