icmp: don't step on recycled socket descriptors

This change fixes lazy socket descriptor handling in ListenPacket to
avoid treading on someone's socket descriptors which are recycled by
descriptor duplication. The issue happens when calling ListenPacket for
non-privileged ICMP endpoint concurrently.

Fixes golang/go#16969.

Change-Id: I4d7672535f5aeb2a0b71f8af2c7ba271a085f418
Reviewed-on: https://go-review.googlesource.com/28473
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

icmp/listen_posix.go

@@ -65,22 +65,24 @@ func ListenPacket(network, address string) (*PacketConn, error) {
 		if err != nil {
 			return nil, os.NewSyscallError("socket", err)
 		}
-		defer syscall.Close(s)
 		if runtime.GOOS == "darwin" && family == syscall.AF_INET {
 			if err := syscall.SetsockoptInt(s, iana.ProtocolIP, sysIP_STRIPHDR, 1); err != nil {
+				syscall.Close(s)
 				return nil, os.NewSyscallError("setsockopt", err)
 			}
 		}
 		sa, err := sockaddr(family, address)
 		if err != nil {
+			syscall.Close(s)
 			return nil, err
 		}
 		if err := syscall.Bind(s, sa); err != nil {
+			syscall.Close(s)
 			return nil, os.NewSyscallError("bind", err)
 		}
 		f := os.NewFile(uintptr(s), "datagram-oriented icmp")
-		defer f.Close()
 		c, cerr = net.FilePacketConn(f)
+		f.Close()
 	default:
 		c, cerr = net.ListenPacket(network, address)
 	}

icmp/ping_test.go

@@ -10,6 +10,7 @@ import (
 	"net"
 	"os"
 	"runtime"
+	"sync"
 	"testing"
 	"time"
 
@@ -164,3 +165,36 @@ func doPing(tt pingTest, seq int) error {
 		return fmt.Errorf("got %+v from %v; want echo reply", rm, peer)
 	}
 }
+
+func TestConcurrentNonPrivilegedListenPacket(t *testing.T) {
+	if testing.Short() {
+		t.Skip("avoid external network")
+	}
+	switch runtime.GOOS {
+	case "darwin":
+	case "linux":
+		t.Log("you may need to adjust the net.ipv4.ping_group_range kernel state")
+	default:
+		t.Skipf("not supported on %s", runtime.GOOS)
+	}
+
+	network, address := "udp4", "127.0.0.1"
+	if !nettest.SupportsIPv4() {
+		network, address = "udp6", "::1"
+	}
+	const N = 1000
+	var wg sync.WaitGroup
+	wg.Add(N)
+	for i := 0; i < N; i++ {
+		go func() {
+			defer wg.Done()
+			c, err := icmp.ListenPacket(network, address)
+			if err != nil {
+				t.Error(err)
+				return
+			}
+			c.Close()
+		}()
+	}
+	wg.Wait()
+}