windows: fix -d=checkptr slice failures

This CL fixes unsafe casts to slices that are missing length or capacity.

Running tests with -d=checkptr enabled may panic on casting unsafe.Pointer to a static array of large predefined length, that is most likely much bigger than the size of the actual array in memory. Checkptr check is not satisfied if slicing operator misses length and capacity arguments `(*[(1 << 30) - 1]uint16)(unsafe.Pointer(p))[:]`, or when there is no slicing at all `(*[(1 << 30) - 1]uint16)(unsafe.Pointer(p))`.

To find all potential cases I used `grep -nr ")(unsafe.Pointer(" ./windows`, then filtered out safe casts when object size is always static and known at compile time.

To reproduce the issue run tests with checkptr enabled `go test -a -gcflags=all=-d=checkptr ./windows/...`.

Updates golang/go#34972
Fixes golang/go#38355

Change-Id: I9dd2084b4f9fb7618cdb140fb2f38b56b6d6cc04
GitHub-Last-Rev: 73288ad18a2692b1510f8ed564daad8d1cbd7e59
GitHub-Pull-Request: golang/sys#65
Reviewed-on: https://go-review.googlesource.com/c/sys/+/225418
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>

windows/svc/mgr/config.go

@@ -8,7 +8,6 @@ package mgr
 
 import (
 	"syscall"
-	"unicode/utf16"
 	"unsafe"
 
 	"golang.org/x/sys/windows"
@@ -46,28 +45,21 @@ type Config struct {
 	DelayedAutoStart bool   // the service is started after other auto-start services are started plus a short delay
 }
 
-func toString(p *uint16) string {
-	if p == nil {
-		return ""
-	}
-	return syscall.UTF16ToString((*[4096]uint16)(unsafe.Pointer(p))[:])
-}
-
 func toStringSlice(ps *uint16) []string {
-	if ps == nil {
-		return nil
-	}
 	r := make([]string, 0)
-	for from, i, p := 0, 0, (*[1 << 24]uint16)(unsafe.Pointer(ps)); true; i++ {
-		if p[i] == 0 {
-			// empty string marks the end
-			if i <= from {
-				break
-			}
-			r = append(r, string(utf16.Decode(p[from:i])))
-			from = i + 1
+	p := unsafe.Pointer(ps)
+
+	for {
+		s := windows.UTF16PtrToString((*uint16)(p))
+		if len(s) == 0 {
+			break
 		}
+
+		r = append(r, s)
+		offset := unsafe.Sizeof(uint16(0)) * (uintptr)(len(s)+1)
+		p = unsafe.Pointer(uintptr(p) + offset)
 	}
+
 	return r
 }
 
@@ -110,13 +102,13 @@ func (s *Service) Config() (Config, error) {
 		ServiceType:      p.ServiceType,
 		StartType:        p.StartType,
 		ErrorControl:     p.ErrorControl,
-		BinaryPathName:   toString(p.BinaryPathName),
-		LoadOrderGroup:   toString(p.LoadOrderGroup),
+		BinaryPathName:   windows.UTF16PtrToString(p.BinaryPathName),
+		LoadOrderGroup:   windows.UTF16PtrToString(p.LoadOrderGroup),
 		TagId:            p.TagId,
 		Dependencies:     toStringSlice(p.Dependencies),
-		ServiceStartName: toString(p.ServiceStartName),
-		DisplayName:      toString(p.DisplayName),
-		Description:      toString(p2.Description),
+		ServiceStartName: windows.UTF16PtrToString(p.ServiceStartName),
+		DisplayName:      windows.UTF16PtrToString(p.DisplayName),
+		Description:      windows.UTF16PtrToString(p2.Description),
 		DelayedAutoStart: delayedStart,
 	}, nil
 }

windows/svc/mgr/mgr.go

@@ -17,6 +17,7 @@ import (
 	"unicode/utf16"
 	"unsafe"
 
+	"golang.org/x/sys/internal/unsafeheader"
 	"golang.org/x/sys/windows"
 )
 
@@ -73,7 +74,7 @@ func (m *Mgr) LockStatus() (*LockStatus, error) {
 		status := &LockStatus{
 			IsLocked: lockStatus.IsLocked != 0,
 			Age:      time.Duration(lockStatus.LockDuration) * time.Second,
-			Owner:    windows.UTF16ToString((*[(1 << 30) - 1]uint16)(unsafe.Pointer(lockStatus.LockOwner))[:]),
+			Owner:    windows.UTF16PtrToString(lockStatus.LockOwner),
 		}
 		return status, nil
 	}
@@ -201,10 +202,16 @@ func (m *Mgr) ListServices() ([]string, error) {
 	if servicesReturned == 0 {
 		return nil, nil
 	}
-	services := (*[1 << 20]windows.ENUM_SERVICE_STATUS_PROCESS)(unsafe.Pointer(&buf[0]))[:servicesReturned]
+
+	var services []windows.ENUM_SERVICE_STATUS_PROCESS
+	hdr := (*unsafeheader.Slice)(unsafe.Pointer(&services))
+	hdr.Data = unsafe.Pointer(&buf[0])
+	hdr.Len = int(servicesReturned)
+	hdr.Cap = int(servicesReturned)
+
 	var names []string
 	for _, s := range services {
-		name := syscall.UTF16ToString((*[1 << 20]uint16)(unsafe.Pointer(s.ServiceName))[:])
+		name := windows.UTF16PtrToString(s.ServiceName)
 		names = append(names, name)
 	}
 	return names, nil

windows/svc/mgr/recovery.go

@@ -12,6 +12,7 @@ import (
 	"time"
 	"unsafe"
 
+	"golang.org/x/sys/internal/unsafeheader"
 	"golang.org/x/sys/windows"
 )
 
@@ -68,8 +69,13 @@ func (s *Service) RecoveryActions() ([]RecoveryAction, error) {
 		return nil, err
 	}
 
+	var actions []windows.SC_ACTION
+	hdr := (*unsafeheader.Slice)(unsafe.Pointer(&actions))
+	hdr.Data = unsafe.Pointer(p.Actions)
+	hdr.Len = int(p.ActionsCount)
+	hdr.Cap = int(p.ActionsCount)
+
 	var recoveryActions []RecoveryAction
-	actions := (*[1024]windows.SC_ACTION)(unsafe.Pointer(p.Actions))[:p.ActionsCount]
 	for _, action := range actions {
 		recoveryActions = append(recoveryActions, RecoveryAction{Type: int(action.Type), Delay: time.Duration(action.Delay) * time.Millisecond})
 	}
@@ -112,7 +118,7 @@ func (s *Service) RebootMessage() (string, error) {
 		return "", err
 	}
 	p := (*windows.SERVICE_FAILURE_ACTIONS)(unsafe.Pointer(&b[0]))
-	return toString(p.RebootMsg), nil
+	return windows.UTF16PtrToString(p.RebootMsg), nil
 }
 
 // SetRecoveryCommand sets the command line of the process to execute in response to the RunCommand service controller action.
@@ -131,5 +137,5 @@ func (s *Service) RecoveryCommand() (string, error) {
 		return "", err
 	}
 	p := (*windows.SERVICE_FAILURE_ACTIONS)(unsafe.Pointer(&b[0]))
-	return toString(p.Command), nil
+	return windows.UTF16PtrToString(p.Command), nil
 }