From 7d6fe89192fbf6fce63ca516cde306c9a17b8a7e Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 19 Oct 2020 09:36:52 -0700 Subject: winipcfg, embeddable-dll-service, wintrust: fix Go 1.15 checkptr violations Avoids "converted pointer straddles allocation" failures at runtime when building binaries in race mode with Go 1.15, which enables checkptr: https://golang.org/doc/go1.15#windows Signed-off-by: Brad Fitzpatrick [Jason: Note Go 1.16/1.17 todo item.] Signed-off-by: Jason A. Donenfeld --- embeddable-dll-service/main.go | 2 +- go.mod | 2 +- go.sum | 4 +- tunnel/winipcfg/types.go | 71 +++++++++++++++++---------------- tunnel/winipcfg/winipcfg_test.go | 2 +- version/wintrust/certificate_windows.go | 6 ++- 6 files changed, 47 insertions(+), 40 deletions(-) diff --git a/embeddable-dll-service/main.go b/embeddable-dll-service/main.go index 27fca4ee..e27ac1a9 100644 --- a/embeddable-dll-service/main.go +++ b/embeddable-dll-service/main.go @@ -22,7 +22,7 @@ import ( //export WireGuardTunnelService func WireGuardTunnelService(confFile16 *uint16) bool { - confFile := windows.UTF16ToString((*[(1 << 30) - 1]uint16)(unsafe.Pointer(confFile16))[:]) + confFile := windows.UTF16PtrToString(confFile16) conf.PresetRootDirectory(filepath.Dir(confFile)) tunnel.UseFixedGUIDInsteadOfDeterministic = true err := tunnel.Run(confFile) diff --git a/go.mod b/go.mod index e791b9a4..b8583b6b 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/lxn/win v0.0.0-20191128105842-2da648fda5b4 golang.org/x/crypto v0.0.0-20200709230013-948cd5f35899 golang.org/x/net v0.0.0-20200707034311-ab3426394381 - golang.org/x/sys v0.0.0-20200728102440-3e129f6d46b1 + golang.org/x/sys v0.0.0-20201020230747-6e5568b54d1a golang.org/x/text v0.3.3 golang.zx2c4.com/wireguard v0.0.20200321-0.20200715051853-507f148e1c42 ) diff --git a/go.sum b/go.sum index 916b67a1..3733df2e 100644 --- a/go.sum +++ b/go.sum @@ -12,8 +12,8 @@ golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200501145240-bc7a7d42d5c3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200728102440-3e129f6d46b1 h1:sIky/MyNRSHTrdxfsiUSS4WIAMvInbeXljJz+jDjeYE= -golang.org/x/sys v0.0.0-20200728102440-3e129f6d46b1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20201020230747-6e5568b54d1a h1:e3IU37lwO4aq3uoRKINC7JikojFmE5gO7xhfxs8VC34= +golang.org/x/sys v0.0.0-20201020230747-6e5568b54d1a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k= diff --git a/tunnel/winipcfg/types.go b/tunnel/winipcfg/types.go index 81f9335d..72ff75e2 100644 --- a/tunnel/winipcfg/types.go +++ b/tunnel/winipcfg/types.go @@ -6,7 +6,6 @@ package winipcfg import ( - "bytes" "net" "unsafe" @@ -581,12 +580,6 @@ const ( ScopeLevelCount = 16 ) -// Theoretical array index limitations -const ( - maxIndexCount8 = (1 << 31) - 1 - maxIndexCount16 = (1 << 30) - 1 -) - // RouteData structure describes a route to add type RouteData struct { Destination net.IPNet @@ -609,15 +602,7 @@ func (obj *IPAdapterDNSSuffix) String() string { // AdapterName method returns the name of the adapter with which these addresses are associated. // Unlike an adapter's friendly name, the adapter name returned by AdapterName is permanent and cannot be modified by the user. func (addr *IPAdapterAddresses) AdapterName() string { - if addr.adapterName == nil { - return "" - } - slice := (*(*[maxIndexCount8]uint8)(unsafe.Pointer(addr.adapterName)))[:] - null := bytes.IndexByte(slice, 0) - if null != -1 { - slice = slice[:null] - } - return string(slice) + return windows.BytePtrToString(addr.adapterName) } // DNSSuffix method returns adapter DNS suffix associated with this adapter. @@ -625,7 +610,7 @@ func (addr *IPAdapterAddresses) DNSSuffix() string { if addr.dnsSuffix == nil { return "" } - return windows.UTF16ToString((*(*[maxIndexCount16]uint16)(unsafe.Pointer(addr.dnsSuffix)))[:]) + return windows.UTF16PtrToString(addr.dnsSuffix) } // Description method returns description for the adapter. @@ -633,7 +618,7 @@ func (addr *IPAdapterAddresses) Description() string { if addr.description == nil { return "" } - return windows.UTF16ToString((*(*[maxIndexCount16]uint16)(unsafe.Pointer(addr.description)))[:]) + return windows.UTF16PtrToString(addr.description) } // FriendlyName method returns a user-friendly name for the adapter. For example: "Local Area Connection 1." @@ -642,7 +627,7 @@ func (addr *IPAdapterAddresses) FriendlyName() string { if addr.friendlyName == nil { return "" } - return windows.UTF16ToString((*(*[maxIndexCount16]uint16)(unsafe.Pointer(addr.friendlyName)))[:]) + return windows.UTF16PtrToString(addr.friendlyName) } // PhysicalAddress method returns the Media Access Control (MAC) address for the adapter. @@ -693,9 +678,9 @@ func (row *MibIPInterfaceRow) Set() error { } // get method returns all table rows as a Go slice. -func (tab *mibIPInterfaceTable) get() []MibIPInterfaceRow { - const maxCount = maxIndexCount8 / unsafe.Sizeof(MibIPInterfaceRow{}) - return (*[maxCount]MibIPInterfaceRow)(unsafe.Pointer(&tab.table[0]))[:tab.numEntries] +func (tab *mibIPInterfaceTable) get() (s []MibIPInterfaceRow) { + unsafeSlice(unsafe.Pointer(&s), unsafe.Pointer(&tab.table[0]), int(tab.numEntries)) + return } // free method frees the buffer allocated by the functions that return tables of network interfaces, addresses, and routes. @@ -731,9 +716,9 @@ func (row *MibIfRow2) get() (ret error) { } // get method returns all table rows as a Go slice. -func (tab *mibIfTable2) get() []MibIfRow2 { - const maxCount = maxIndexCount8 / unsafe.Sizeof(MibIfRow2{}) - return (*[maxCount]MibIfRow2)(unsafe.Pointer(&tab.table[0]))[:tab.numEntries] +func (tab *mibIfTable2) get() (s []MibIfRow2) { + unsafeSlice(unsafe.Pointer(&s), unsafe.Pointer(&tab.table[0]), int(tab.numEntries)) + return } // free method frees the buffer allocated by the functions that return tables of network interfaces, addresses, and routes. @@ -821,9 +806,9 @@ func (row *MibUnicastIPAddressRow) Delete() error { } // get method returns all table rows as a Go slice. -func (tab *mibUnicastIPAddressTable) get() []MibUnicastIPAddressRow { - const maxCount = maxIndexCount8 / unsafe.Sizeof(MibUnicastIPAddressRow{}) - return (*[maxCount]MibUnicastIPAddressRow)(unsafe.Pointer(&tab.table[0]))[:tab.numEntries] +func (tab *mibUnicastIPAddressTable) get() (s []MibUnicastIPAddressRow) { + unsafeSlice(unsafe.Pointer(&s), unsafe.Pointer(&tab.table[0]), int(tab.numEntries)) + return } // free method frees the buffer allocated by the functions that return tables of network interfaces, addresses, and routes. @@ -851,9 +836,9 @@ func (row *MibAnycastIPAddressRow) Delete() error { } // get method returns all table rows as a Go slice. -func (tab *mibAnycastIPAddressTable) get() []MibAnycastIPAddressRow { - const maxCount = maxIndexCount8 / unsafe.Sizeof(MibAnycastIPAddressRow{}) - return (*[maxCount]MibAnycastIPAddressRow)(unsafe.Pointer(&tab.table[0]))[:tab.numEntries] +func (tab *mibAnycastIPAddressTable) get() (s []MibAnycastIPAddressRow) { + unsafeSlice(unsafe.Pointer(&s), unsafe.Pointer(&tab.table[0]), int(tab.numEntries)) + return } // free method frees the buffer allocated by the functions that return tables of network interfaces, addresses, and routes. @@ -944,9 +929,9 @@ func (row *MibIPforwardRow2) Delete() error { } // get method returns all table rows as a Go slice. -func (tab *mibIPforwardTable2) get() []MibIPforwardRow2 { - const maxCount = maxIndexCount8 / unsafe.Sizeof(MibIPforwardRow2{}) - return (*[maxCount]MibIPforwardRow2)(unsafe.Pointer(&tab.table[0]))[:tab.numEntries] +func (tab *mibIPforwardTable2) get() (s []MibIPforwardRow2) { + unsafeSlice(unsafe.Pointer(&s), unsafe.Pointer(&tab.table[0]), int(tab.numEntries)) + return } // free method frees the buffer allocated by the functions that return tables of network interfaces, addresses, and routes. @@ -954,3 +939,21 @@ func (tab *mibIPforwardTable2) get() []MibIPforwardRow2 { func (tab *mibIPforwardTable2) free() { freeMibTable(unsafe.Pointer(tab)) } + +// unsafeSlice updates the slice slicePtr to be a slice +// referencing the provided data with its length & capacity set to +// lenCap. +// +// TODO: when Go 1.16 or Go 1.17 is the minimum supported version, +// update callers to use unsafe.Slice instead of this. +func unsafeSlice(slicePtr, data unsafe.Pointer, lenCap int) { + type sliceHeader struct { + Data unsafe.Pointer + Len int + Cap int + } + h := (*sliceHeader)(slicePtr) + h.Data = data + h.Len = lenCap + h.Cap = lenCap +} diff --git a/tunnel/winipcfg/winipcfg_test.go b/tunnel/winipcfg/winipcfg_test.go index 0251aecf..bd2eba9f 100644 --- a/tunnel/winipcfg/winipcfg_test.go +++ b/tunnel/winipcfg/winipcfg_test.go @@ -107,7 +107,7 @@ func TestAdaptersAddresses(t *testing.T) { i.PhysicalAddress() i.DHCPv6ClientDUID() for dnsSuffix := i.FirstDNSSuffix; dnsSuffix != nil; dnsSuffix = dnsSuffix.Next { - dnsSuffix.String() + _ = dnsSuffix.String() } } } diff --git a/version/wintrust/certificate_windows.go b/version/wintrust/certificate_windows.go index 1e145095..65b1eaed 100644 --- a/version/wintrust/certificate_windows.go +++ b/version/wintrust/certificate_windows.go @@ -48,7 +48,11 @@ func ExtractCertificates(path string) ([]x509.Certificate, error) { break } buf := make([]byte, cert.Length) - copy(buf, (*[1 << 20]byte)(unsafe.Pointer(cert.EncodedCert))[:]) + //TODO: when Go 1.16 or 1.17 comes out, switch to: + // copy(buf, unsafe.Slice(unsafe.Pointer(cert.EncodedCert), len(buf))) + for i := range buf { + buf[i] = *(*byte)(unsafe.Pointer(uintptr(unsafe.Pointer(cert.EncodedCert)) + uintptr(i))) + } if c, err := x509.ParseCertificate(buf); err == nil { certs = append(certs, *c) } else { -- cgit v1.2.3-59-g8ed1b