From d32ec38f1cfe69ed2a553272f6c8f425c59577f2 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 5 Jun 2019 19:59:58 -0400 Subject: [PATCH] net: remove non-cgo macOS resolver code The built-in Go resolver works significantly better. In particular, the use of res_search does not support CNAME or PTR queries and may not even be thread-safe. This CL is essentially a revert of CL 166297 plus fixes, including CL 180842. See CL 180842 for additional notes about problems with this approach. Fixes #31705. Change-Id: I0a30a0de2fbd04f6c461520fd34378c84aadf66c Reviewed-on: https://go-review.googlesource.com/c/go/+/180843 Run-TryBot: Russ Cox Reviewed-by: Keith Randall --- src/net/cgo_darwin_stub.go | 242 ------------------------------ src/net/cgo_darwin_stub_test.go | 80 ---------- src/net/cgo_stub.go | 1 - src/net/conf.go | 5 - src/runtime/lookup_darwin.go | 35 ----- src/runtime/lookup_darwin_386.s | 50 ------ src/runtime/lookup_darwin_amd64.s | 40 ----- src/runtime/lookup_darwin_arm.s | 25 --- src/runtime/lookup_darwin_arm64.s | 21 --- 9 files changed, 499 deletions(-) delete mode 100644 src/net/cgo_darwin_stub.go delete mode 100644 src/net/cgo_darwin_stub_test.go delete mode 100644 src/runtime/lookup_darwin.go delete mode 100644 src/runtime/lookup_darwin_386.s delete mode 100644 src/runtime/lookup_darwin_amd64.s delete mode 100644 src/runtime/lookup_darwin_arm.s delete mode 100644 src/runtime/lookup_darwin_arm64.s diff --git a/src/net/cgo_darwin_stub.go b/src/net/cgo_darwin_stub.go deleted file mode 100644 index fc50809d40..0000000000 --- a/src/net/cgo_darwin_stub.go +++ /dev/null @@ -1,242 +0,0 @@ -// Copyright 2019 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// This file is intended to be used in non-cgo builds of darwin binaries, -// in particular when cross-compiling a darwin binary from a non-darwin machine. -// All OS calls on darwin have to be done via C libraries, and this code makes such -// calls with the runtime's help. (It is a C call but does not require the cgo tool to -// be compiled, and as such it is possible to build even when cross-compiling.) -// -// The specific C library calls are to res_init and res_search from /usr/lib/system/libsystem_info.dylib. -// Unfortunately, an ordinary C program calling these names would actually end up with -// res_9_init and res_9_search from /usr/lib/libresolv.dylib, not libsystem_info. -// It may well be that the libsystem_info routines are completely unused on macOS systems -// except for this code. At the least, they have the following problems: -// -// - TypeALL requests do not work, so if we want both IPv4 and IPv6 addresses, -// we have to do two requests, one for TypeA and one for TypeAAAA. -// - TypeCNAME requests hang indefinitely. -// - TypePTR requests fail unconditionally. -// - Detailed error information is stored in the global h_errno value, -// which cannot be accessed safely (it is not per-thread like errno). -// - The routines may not be safe to call from multiple threads. -// If you run net.test under lldb, that emits syslog prints to stderr -// that suggest double-free problems. (If not running under lldb, -// it is unclear where the syslog prints go, if anywhere.) -// -// This code is marked for deletion. If it is to be revived, it should be changed to use -// res_9_init and res_9_search from libresolv and special care should be paid to -// error detail and thread safety. - -// +build !netgo,!cgo -// +build darwin - -package net - -import ( - "context" - "errors" - "sync" - - "golang.org/x/net/dns/dnsmessage" -) - -type addrinfoErrno int - -func (eai addrinfoErrno) Error() string { return "" } -func (eai addrinfoErrno) Temporary() bool { return false } -func (eai addrinfoErrno) Timeout() bool { return false } - -func cgoLookupHost(ctx context.Context, name string) (addrs []string, err error, completed bool) { - // The 4-suffix indicates IPv4, TypeA lookups. - // The 6-suffix indicates IPv6, TypeAAAA lookups. - // If resSearch is updated to call the libresolv res_9_search (see comment at top of file), - // it may be possible to make one call for TypeALL - // and get both address kinds out. - r4, err4 := resSearch(ctx, name, int32(dnsmessage.TypeA), int32(dnsmessage.ClassINET)) - if err4 == nil { - addrs, err4 = appendHostsFromResources(addrs, r4) - } - r6, err6 := resSearch(ctx, name, int32(dnsmessage.TypeAAAA), int32(dnsmessage.ClassINET)) - if err6 == nil { - addrs, err6 = appendHostsFromResources(addrs, r6) - } - if err4 != nil && err6 != nil { - return nil, err4, false - } - return addrs, nil, true -} - -func cgoLookupPort(ctx context.Context, network, service string) (port int, err error, completed bool) { - port, err = goLookupPort(network, service) // we can just use netgo lookup - return port, err, err == nil -} - -func cgoLookupIP(ctx context.Context, network, name string) (addrs []IPAddr, err error, completed bool) { - // The 4-suffix indicates IPv4, TypeA lookups. - // The 6-suffix indicates IPv6, TypeAAAA lookups. - // If resSearch is updated to call the libresolv res_9_search (see comment at top of file), - // it may be possible to make one call for TypeALL (when vers != '6' and vers != '4') - // and get both address kinds out. - var r4, r6 []dnsmessage.Resource - var err4, err6 error - vers := ipVersion(network) - if vers != '6' { - r4, err4 = resSearch(ctx, name, int32(dnsmessage.TypeA), int32(dnsmessage.ClassINET)) - if err4 == nil { - addrs, err4 = appendIPsFromResources(addrs, r4) - } - } - if vers != '4' { - r6, err6 = resSearch(ctx, name, int32(dnsmessage.TypeAAAA), int32(dnsmessage.ClassINET)) - if err6 == nil { - addrs, err6 = appendIPsFromResources(addrs, r6) - } - } - if err4 != nil && err6 != nil { - return nil, err4, false - } - - return addrs, nil, true -} - -func cgoLookupCNAME(ctx context.Context, name string) (cname string, err error, completed bool) { - resources, err := resSearch(ctx, name, int32(dnsmessage.TypeCNAME), int32(dnsmessage.ClassINET)) - if err != nil { - return - } - cname, err = parseCNAMEFromResources(resources) - if err != nil { - return "", err, false - } - return cname, nil, true -} - -func cgoLookupPTR(ctx context.Context, addr string) (ptrs []string, err error, completed bool) { - resources, err := resSearch(ctx, addr, int32(dnsmessage.TypePTR), int32(dnsmessage.ClassINET)) - if err != nil { - return - } - ptrs, err = parsePTRsFromResources(resources) - if err != nil { - return - } - return ptrs, nil, true -} - -var ( - resInitOnce sync.Once - resInitResult int32 -) - -// resSearch will make a call to the 'res_search' routine in libSystem -// and parse the output as a slice of resource resources which can then be parsed -func resSearch(ctx context.Context, hostname string, rtype, class int32) ([]dnsmessage.Resource, error) { - // We have to use res_init and res_search, but these do not set errno on failure. - // (They set h_errno, which is a global int shared by all threads and therefore - // racy to use.) - // https://opensource.apple.com/source/Libinfo/Libinfo-517.200.9/dns.subproj/res_query.c.auto.html - resInitOnce.Do(func() { - resInitResult = res_init() - }) - if resInitResult < 0 { - return nil, errors.New("res_init failure") - } - - // res_search does not set errno. - // It returns the size of the DNS response packet. - // But if the DNS response packet contains failure-like response codes, - // res_search returns -1 even though it has copied the packet into buf, - // giving us no way to find out how big the packet is. - // For now, we are willing to take res_search's word that there's nothing - // useful in the response, even though there *is* a response. - name := make([]byte, len(hostname)+1) // +1 for NUL at end for C - copy(name, hostname) - var buf [1024]byte - size, _ := res_search(&name[0], class, rtype, &buf[0], int32(len(buf))) - if size <= 0 { - return nil, errors.New("res_search failure") - } - - var p dnsmessage.Parser - if _, err := p.Start(buf[:size]); err != nil { - return nil, err - } - p.SkipAllQuestions() - resources, err := p.AllAnswers() - if err != nil { - return nil, err - } - return resources, nil -} - -func copyBytes(x []byte) []byte { - y := make([]byte, len(x)) - copy(y, x) - return y -} - -func appendHostsFromResources(answers []string, resources []dnsmessage.Resource) ([]string, error) { - for i := range resources { - switch resources[i].Header.Type { - case dnsmessage.TypeA: - b := resources[i].Body.(*dnsmessage.AResource) - answers = append(answers, IP(b.A[:]).String()) - case dnsmessage.TypeAAAA: - b := resources[i].Body.(*dnsmessage.AAAAResource) - answers = append(answers, IP(b.AAAA[:]).String()) - default: - return nil, errors.New("could not parse an A or AAAA response from message buffer") - } - } - return answers, nil -} - -func appendIPsFromResources(answers []IPAddr, resources []dnsmessage.Resource) ([]IPAddr, error) { - for i := range resources { - switch resources[i].Header.Type { - case dnsmessage.TypeA: - b := resources[i].Body.(*dnsmessage.AResource) - answers = append(answers, IPAddr{IP: IP(copyBytes(b.A[:]))}) - case dnsmessage.TypeAAAA: - b := resources[i].Body.(*dnsmessage.AAAAResource) - answers = append(answers, IPAddr{IP: IP(copyBytes(b.AAAA[:]))}) - default: - return nil, errors.New("could not parse an A or AAAA response from message buffer") - } - } - return answers, nil -} - -func parseCNAMEFromResources(resources []dnsmessage.Resource) (string, error) { - if len(resources) == 0 { - return "", errors.New("no CNAME record received") - } - c, ok := resources[0].Body.(*dnsmessage.CNAMEResource) - if !ok { - return "", errors.New("could not parse CNAME record") - } - return c.CNAME.String(), nil -} - -func parsePTRsFromResources(resources []dnsmessage.Resource) ([]string, error) { - var answers []string - for i := range resources { - switch resources[i].Header.Type { - case dnsmessage.TypePTR: - p := resources[0].Body.(*dnsmessage.PTRResource) - answers = append(answers, p.PTR.String()) - default: - return nil, errors.New("could not parse a PTR response from message buffer") - - } - } - return answers, nil -} - -// res_init and res_search are defined in runtime/lookup_darwin.go - -func res_init() int32 - -func res_search(dname *byte, class int32, rtype int32, answer *byte, anslen int32) (int32, int32) diff --git a/src/net/cgo_darwin_stub_test.go b/src/net/cgo_darwin_stub_test.go deleted file mode 100644 index f694e2a0cb..0000000000 --- a/src/net/cgo_darwin_stub_test.go +++ /dev/null @@ -1,80 +0,0 @@ -// Copyright 2019 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// +build !netgo,!cgo -// +build darwin - -package net - -import ( - "context" - "strings" - "testing" -) - -func TestPseudoCgoLookupHost(t *testing.T) { - addrs, err, ok := cgoLookupHost(context.Background(), "google.com") - t.Logf("cgoLookupHost google.com: %v %v %v", addrs, err, ok) - if !ok { - t.Fatal("cgoLookupHost ok=false") - } - if err != nil { - t.Fatalf("cgoLookupHost: %v", err) - } - // cgoLookupHost need not return IPv4 before IPv6 in general, - // but for the current implementation it does. - // If that changes, this test will need updating. - if len(addrs) < 1 || strings.Count(addrs[0], ".") != 3 || !strings.Contains(addrs[len(addrs)-1], "::") { - t.Fatalf("cgoLookupHost google.com = %v, want IPv4 and IPv6", addrs) - } -} - -func TestPseudoCgoLookupIP(t *testing.T) { - ips, err, ok := cgoLookupIP(context.Background(), "ip", "google.com") - t.Logf("cgoLookupIP google.com: %v %v %v", ips, err, ok) - if !ok { - t.Fatal("cgoLookupIP ok=false") - } - if err != nil { - t.Fatalf("cgoLookupIP: %v", err) - } - // cgoLookupIP need not return IPv4 before IPv6 in general, - // but for the current implementation it does. - // If that changes, this test will need updating. - if len(ips) < 1 || len(ips[0].IP) != 4 || len(ips[len(ips)-1].IP) != 16 { - t.Fatalf("cgoLookupIP google.com = %v, want IPv4 and IPv6", ips) - } -} - -func TestPseudoCgoLookupCNAME(t *testing.T) { - t.Skip("res_search on macOS hangs in TypeCNAME queries (even in plain C programs)") - - cname, err, ok := cgoLookupCNAME(context.Background(), "redirect.swtch.com") - t.Logf("cgoLookupCNAME redirect.swtch.com: %v %v %v", cname, err, ok) - if !ok { - t.Fatal("cgoLookupCNAME ok=false") - } - if err != nil { - t.Fatalf("cgoLookupCNAME: %v", err) - } - if !strings.HasSuffix(cname, ".com") { - t.Fatalf("cgoLookupCNAME redirect.swtch.com = %v, want *.com", cname) - } -} - -func TestPseudoCgoLookupPTR(t *testing.T) { - t.Skip("res_search on macOS does not support TypePTR") - - ptrs, err, ok := cgoLookupPTR(context.Background(), "8.8.8.8") - t.Logf("cgoLookupPTR 8.8.8.8: %v %v %v", ptrs, err, ok) - if !ok { - t.Fatal("cgoLookupPTR ok=false") - } - if err != nil { - t.Fatalf("cgoLookupPTR: %v", err) - } - if len(ptrs) < 1 || ptrs[0] != "google-public-dns-a.google.com" { - t.Fatalf("cgoLookupPTR = %v, want google-public-dns-a.google.com", ptrs) - } -} diff --git a/src/net/cgo_stub.go b/src/net/cgo_stub.go index aa15ab4dc1..041f8af129 100644 --- a/src/net/cgo_stub.go +++ b/src/net/cgo_stub.go @@ -3,7 +3,6 @@ // license that can be found in the LICENSE file. // +build !cgo netgo -// +build !darwin package net diff --git a/src/net/conf.go b/src/net/conf.go index 1c88f096ba..971b1a399a 100644 --- a/src/net/conf.go +++ b/src/net/conf.go @@ -70,11 +70,6 @@ func initConfVal() { // their own DNS requests. So always use cgo instead, which // avoids that. if runtime.GOOS == "darwin" { - // Normally we force netGo to be true if building without cgo enabled. - // On Darwin, we can call libc even if cgo is not enabled, so only set netGo to true - // if explicitly requested. - confVal.netGo = dnsMode == "go" - confVal.forceCgoLookupHost = true return } diff --git a/src/runtime/lookup_darwin.go b/src/runtime/lookup_darwin.go deleted file mode 100644 index c39b937ccf..0000000000 --- a/src/runtime/lookup_darwin.go +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright 2019 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package runtime - -import ( - "unsafe" -) - -//go:linkname res_init net.res_init -//go:nosplit -//go:cgo_unsafe_args -func res_init() int32 { - return libcCall(unsafe.Pointer(funcPC(res_init_trampoline)), nil) -} -func res_init_trampoline() - -//go:linkname res_search net.res_search -//go:nosplit -//go:cgo_unsafe_args -func res_search(dname *byte, class int32, rtype int32, answer *byte, anslen int32) (int32, int32) { - args := struct { - dname *byte - class, rtype int32 - answer *byte - anslen, retSize, retErr int32 - }{dname, class, rtype, answer, anslen, 0, 0} - libcCall(unsafe.Pointer(funcPC(res_search_trampoline)), unsafe.Pointer(&args)) - return args.retSize, args.retErr -} -func res_search_trampoline() - -//go:cgo_import_dynamic libc_res_search res_search "/usr/lib/libSystem.B.dylib" -//go:cgo_import_dynamic libc_res_init res_init "/usr/lib/libSystem.B.dylib" diff --git a/src/runtime/lookup_darwin_386.s b/src/runtime/lookup_darwin_386.s deleted file mode 100644 index e185532231..0000000000 --- a/src/runtime/lookup_darwin_386.s +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2019 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -#include "go_asm.h" -#include "go_tls.h" -#include "textflag.h" - -TEXT runtime·res_init_trampoline(SB),NOSPLIT,$0 - PUSHL BP - MOVL SP, BP - SUBL $8, SP - CALL libc_res_init(SB) - CMPL AX, $-1 - JNE ok - CALL libc_error(SB) -ok: - MOVL BP, SP - POPL BP - RET - -TEXT runtime·res_search_trampoline(SB),NOSPLIT,$0 - PUSHL BP - MOVL SP, BP - SUBL $24, SP - MOVL 32(SP), CX - MOVL 16(CX), AX // arg 5 anslen - MOVL AX, 16(SP) - MOVL 12(CX), AX // arg 4 answer - MOVL AX, 12(SP) - MOVL 8(CX), AX // arg 3 type - MOVL AX, 8(SP) - MOVL 4(CX), AX // arg 2 class - MOVL AX, 4(SP) - MOVL 0(CX), AX // arg 1 name - MOVL AX, 0(SP) - CALL libc_res_search(SB) - XORL DX, DX - CMPL AX, $-1 - JNE ok - CALL libc_error(SB) - MOVL (AX), DX - XORL AX, AX -ok: - MOVL 32(SP), CX - MOVL AX, 20(CX) - MOVL DX, 24(CX) - MOVL BP, SP - POPL BP - RET diff --git a/src/runtime/lookup_darwin_amd64.s b/src/runtime/lookup_darwin_amd64.s deleted file mode 100644 index 587e43612e..0000000000 --- a/src/runtime/lookup_darwin_amd64.s +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright 2019 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -#include "go_asm.h" -#include "go_tls.h" -#include "textflag.h" - -TEXT runtime·res_init_trampoline(SB),NOSPLIT,$0 - PUSHQ BP - MOVQ SP, BP - CALL libc_res_init(SB) - CMPL AX, $-1 - JNE ok - CALL libc_error(SB) -ok: - POPQ BP - RET - -TEXT runtime·res_search_trampoline(SB),NOSPLIT,$0 - PUSHQ BP - MOVQ SP, BP - MOVQ DI, BX // move DI into BX to preserve struct addr - MOVL 24(BX), R8 // arg 5 anslen - MOVQ 16(BX), CX // arg 4 answer - MOVL 12(BX), DX // arg 3 type - MOVL 8(BX), SI // arg 2 class - MOVQ 0(BX), DI // arg 1 name - CALL libc_res_search(SB) - XORL DX, DX - CMPL AX, $-1 - JNE ok - CALL libc_error(SB) - MOVLQSX (AX), DX // move return from libc_error into DX - XORL AX, AX // size on error is 0 -ok: - MOVL AX, 28(BX) // size - MOVL DX, 32(BX) // error code - POPQ BP - RET diff --git a/src/runtime/lookup_darwin_arm.s b/src/runtime/lookup_darwin_arm.s deleted file mode 100644 index c74419f58b..0000000000 --- a/src/runtime/lookup_darwin_arm.s +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2015 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// System calls and other sys.stuff for ARM64, Darwin -// System calls are implemented in libSystem, this file contains -// trampolines that convert from Go to C calling convention. - -#include "go_asm.h" -#include "go_tls.h" -#include "textflag.h" - -// On darwin/arm, the runtime always uses runtime/cgo -// for resolution. This will just exit with a nominal -// exit code. - -TEXT runtime·res_search_trampoline(SB),NOSPLIT,$0 - MOVW $90, R0 - BL libc_exit(SB) - RET - -TEXT runtime·res_init_trampoline(SB),NOSPLIT,$0 - MOVW $91, R0 - BL libc_exit(SB) - RET diff --git a/src/runtime/lookup_darwin_arm64.s b/src/runtime/lookup_darwin_arm64.s deleted file mode 100644 index e13662b037..0000000000 --- a/src/runtime/lookup_darwin_arm64.s +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2019 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -#include "go_asm.h" -#include "go_tls.h" -#include "textflag.h" - -// On darwin/arm, the runtime always uses runtime/cgo -// for resolution. This will just exit with a nominal -// exit code. - -TEXT runtime·res_search_trampoline(SB),NOSPLIT,$0 - MOVW $90, R0 - BL libc_exit(SB) - RET - -TEXT runtime·res_init_trampoline(SB),NOSPLIT,$0 - MOVW $91, R0 - BL libc_exit(SB) - RET -- 2.50.0