From f25ff604a2a41138652010750727a685a8cc9de7 Mon Sep 17 00:00:00 2001 From: masshash Date: Sun, 2 Apr 2023 11:54:43 +0000 Subject: [PATCH 1/9] windows: add JobObjectInformationClass consts for QueryInformationJobObject Reference: https://learn.microsoft.com/en-us/windows/win32/api/jobapi2/nf-jobapi2-queryinformationjobobject#parameters Change-Id: I0d10895ffc18b345f371cc7e0cbf8362fd67f71a GitHub-Last-Rev: 28163917a07e9765495eaed5681f615bb506c153 GitHub-Pull-Request: golang/sys#154 Reviewed-on: https://go-review.googlesource.com/c/sys/+/481455 Run-TryBot: Alex Brainman Reviewed-by: Michael Knyszek Reviewed-by: Alex Brainman Reviewed-by: David Chase TryBot-Result: Gopher Robot --- windows/types_windows.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/windows/types_windows.go b/windows/types_windows.go index 0dbb208411..88e62a6385 100644 --- a/windows/types_windows.go +++ b/windows/types_windows.go @@ -2220,15 +2220,19 @@ type JOBOBJECT_BASIC_UI_RESTRICTIONS struct { } const ( - // JobObjectInformationClass + // JobObjectInformationClass for QueryInformationJobObject and SetInformationJobObject JobObjectAssociateCompletionPortInformation = 7 + JobObjectBasicAccountingInformation = 1 + JobObjectBasicAndIoAccountingInformation = 8 JobObjectBasicLimitInformation = 2 + JobObjectBasicProcessIdList = 3 JobObjectBasicUIRestrictions = 4 JobObjectCpuRateControlInformation = 15 JobObjectEndOfJobTimeInformation = 6 JobObjectExtendedLimitInformation = 9 JobObjectGroupInformation = 11 JobObjectGroupInformationEx = 14 + JobObjectLimitViolationInformation = 13 JobObjectLimitViolationInformation2 = 34 JobObjectNetRateControlInformation = 32 JobObjectNotificationLimitInformation = 12 From dbd8f99a5ea6966d2aa742c744e685f3299419d7 Mon Sep 17 00:00:00 2001 From: Sebastian Soto Date: Fri, 18 Nov 2022 10:22:56 -0500 Subject: [PATCH 2/9] windows: add Service.ListDependentServices This method allows a user to list all Windows services which are dependent upon a given service. This commit makes use of the EnumDependentServices Windows API call. Without this, a user would have to iterate through each service on the system, and check if the given service is listed in each service's dependencies list. The implementation of ListDependentServices is mostly the same as Mgr.ListServices, as the API calls behave in the same way. Fixes golang/go#56766 Change-Id: I9ec18c97afd02f48deef691ccdd5c26d6501add1 Reviewed-on: https://go-review.googlesource.com/c/sys/+/451363 Reviewed-by: Than McIntosh Run-TryBot: Alex Brainman TryBot-Result: Gopher Robot Reviewed-by: Benny Siegert Reviewed-by: Alex Brainman --- windows/service.go | 7 ++++++ windows/svc/mgr/mgr_test.go | 43 +++++++++++++++++++++++++++++++++++++ windows/svc/mgr/service.go | 43 +++++++++++++++++++++++++++++++++++-- windows/svc/service.go | 9 ++++++++ windows/zsyscall_windows.go | 9 ++++++++ 5 files changed, 109 insertions(+), 2 deletions(-) diff --git a/windows/service.go b/windows/service.go index f8deca8397..c964b6848d 100644 --- a/windows/service.go +++ b/windows/service.go @@ -141,6 +141,12 @@ const ( SERVICE_DYNAMIC_INFORMATION_LEVEL_START_REASON = 1 ) +type ENUM_SERVICE_STATUS struct { + ServiceName *uint16 + DisplayName *uint16 + ServiceStatus SERVICE_STATUS +} + type SERVICE_STATUS struct { ServiceType uint32 CurrentState uint32 @@ -245,3 +251,4 @@ type QUERY_SERVICE_LOCK_STATUS struct { //sys UnsubscribeServiceChangeNotifications(subscription uintptr) = sechost.UnsubscribeServiceChangeNotifications? //sys RegisterServiceCtrlHandlerEx(serviceName *uint16, handlerProc uintptr, context uintptr) (handle Handle, err error) = advapi32.RegisterServiceCtrlHandlerExW //sys QueryServiceDynamicInformation(service Handle, infoLevel uint32, dynamicInfo unsafe.Pointer) (err error) = advapi32.QueryServiceDynamicInformation? +//sys EnumDependentServices(service Handle, activityState uint32, services *ENUM_SERVICE_STATUS, buffSize uint32, bytesNeeded *uint32, servicesReturned *uint32) (err error) = advapi32.EnumDependentServicesW diff --git a/windows/svc/mgr/mgr_test.go b/windows/svc/mgr/mgr_test.go index 330cca4a14..bc763f0604 100644 --- a/windows/svc/mgr/mgr_test.go +++ b/windows/svc/mgr/mgr_test.go @@ -17,6 +17,7 @@ import ( "testing" "time" + "golang.org/x/sys/windows/svc" "golang.org/x/sys/windows/svc/mgr" ) @@ -294,3 +295,45 @@ func TestMyService(t *testing.T) { remove(t, s) } + +func TestListDependentServices(t *testing.T) { + m, err := mgr.Connect() + if err != nil { + if errno, ok := err.(syscall.Errno); ok && errno == syscall.ERROR_ACCESS_DENIED { + t.Skip("Skipping test: we don't have rights to manage services.") + } + t.Fatalf("SCM connection failed: %s", err) + } + defer m.Disconnect() + + baseServiceName := "testservice1" + dependentServiceName := "testservice2" + install(t, m, baseServiceName, "", mgr.Config{}) + baseService, err := m.OpenService(baseServiceName) + if err != nil { + t.Fatalf("OpenService failed: %v", err) + } + defer remove(t, baseService) + install(t, m, dependentServiceName, "", mgr.Config{Dependencies: []string{baseServiceName}}) + dependentService, err := m.OpenService(dependentServiceName) + if err != nil { + t.Fatalf("OpenService failed: %v", err) + } + defer remove(t, dependentService) + + // test that both the base service and dependent service list the correct dependencies + dependentServices, err := baseService.ListDependentServices(svc.AnyActivity) + if err != nil { + t.Fatalf("baseService.ListDependentServices failed: %v", err) + } + if len(dependentServices) != 1 || dependentServices[0] != dependentServiceName { + t.Errorf("Found %v, instead of expected contents %s", dependentServices, dependentServiceName) + } + dependentServices, err = dependentService.ListDependentServices(svc.AnyActivity) + if err != nil { + t.Fatalf("dependentService.ListDependentServices failed: %v", err) + } + if len(dependentServices) != 0 { + t.Errorf("Found %v, where no service should be listed", dependentService) + } +} diff --git a/windows/svc/mgr/service.go b/windows/svc/mgr/service.go index 0623fc0b02..90f5d95d55 100644 --- a/windows/svc/mgr/service.go +++ b/windows/svc/mgr/service.go @@ -15,8 +15,6 @@ import ( "golang.org/x/sys/windows/svc" ) -// TODO(brainman): Use EnumDependentServices to enumerate dependent services. - // Service is used to access Windows service. type Service struct { Name string @@ -76,3 +74,44 @@ func (s *Service) Query() (svc.Status, error) { ServiceSpecificExitCode: t.ServiceSpecificExitCode, }, nil } + +// ListDependentServices returns the names of the services dependent on service s, which match the given status. +func (s *Service) ListDependentServices(status svc.ActivityStatus) ([]string, error) { + var bytesNeeded, returnedServiceCount uint32 + var services []windows.ENUM_SERVICE_STATUS + for { + var servicesPtr *windows.ENUM_SERVICE_STATUS + if len(services) > 0 { + servicesPtr = &services[0] + } + allocatedBytes := uint32(len(services)) * uint32(unsafe.Sizeof(windows.ENUM_SERVICE_STATUS{})) + err := windows.EnumDependentServices(s.Handle, uint32(status), servicesPtr, allocatedBytes, &bytesNeeded, + &returnedServiceCount) + if err == nil { + break + } + if err != syscall.ERROR_MORE_DATA { + return nil, err + } + if bytesNeeded <= allocatedBytes { + return nil, err + } + // ERROR_MORE_DATA indicates the provided buffer was too small, run the call again after resizing the buffer + requiredSliceLen := bytesNeeded / uint32(unsafe.Sizeof(windows.ENUM_SERVICE_STATUS{})) + if bytesNeeded%uint32(unsafe.Sizeof(windows.ENUM_SERVICE_STATUS{})) != 0 { + requiredSliceLen += 1 + } + services = make([]windows.ENUM_SERVICE_STATUS, requiredSliceLen) + } + if returnedServiceCount == 0 { + return nil, nil + } + + // The slice mutated by EnumDependentServices may have a length greater than returnedServiceCount, any elements + // past that should be ignored. + var dependents []string + for i := 0; i < int(returnedServiceCount); i++ { + dependents = append(dependents, windows.UTF16PtrToString(services[i].ServiceName)) + } + return dependents, nil +} diff --git a/windows/svc/service.go b/windows/svc/service.go index 806baa055f..2b4a7bc6c2 100644 --- a/windows/svc/service.go +++ b/windows/svc/service.go @@ -68,6 +68,15 @@ const ( AcceptPreShutdown = Accepted(windows.SERVICE_ACCEPT_PRESHUTDOWN) ) +// ActivityStatus allows for services to be selected based on active and inactive categories of service state. +type ActivityStatus uint32 + +const ( + Active = ActivityStatus(windows.SERVICE_ACTIVE) + Inactive = ActivityStatus(windows.SERVICE_INACTIVE) + AnyActivity = ActivityStatus(windows.SERVICE_STATE_ALL) +) + // Status combines State and Accepted commands to fully describe running service. type Status struct { State State diff --git a/windows/zsyscall_windows.go b/windows/zsyscall_windows.go index 6d2a268534..a81ea2c700 100644 --- a/windows/zsyscall_windows.go +++ b/windows/zsyscall_windows.go @@ -86,6 +86,7 @@ var ( procDeleteService = modadvapi32.NewProc("DeleteService") procDeregisterEventSource = modadvapi32.NewProc("DeregisterEventSource") procDuplicateTokenEx = modadvapi32.NewProc("DuplicateTokenEx") + procEnumDependentServicesW = modadvapi32.NewProc("EnumDependentServicesW") procEnumServicesStatusExW = modadvapi32.NewProc("EnumServicesStatusExW") procEqualSid = modadvapi32.NewProc("EqualSid") procFreeSid = modadvapi32.NewProc("FreeSid") @@ -734,6 +735,14 @@ func DuplicateTokenEx(existingToken Token, desiredAccess uint32, tokenAttributes return } +func EnumDependentServices(service Handle, activityState uint32, services *ENUM_SERVICE_STATUS, buffSize uint32, bytesNeeded *uint32, servicesReturned *uint32) (err error) { + r1, _, e1 := syscall.Syscall6(procEnumDependentServicesW.Addr(), 6, uintptr(service), uintptr(activityState), uintptr(unsafe.Pointer(services)), uintptr(buffSize), uintptr(unsafe.Pointer(bytesNeeded)), uintptr(unsafe.Pointer(servicesReturned))) + if r1 == 0 { + err = errnoErr(e1) + } + return +} + func EnumServicesStatusEx(mgr Handle, infoLevel uint32, serviceType uint32, serviceState uint32, services *byte, bufSize uint32, bytesNeeded *uint32, servicesReturned *uint32, resumeHandle *uint32, groupName *uint16) (err error) { r1, _, e1 := syscall.Syscall12(procEnumServicesStatusExW.Addr(), 10, uintptr(mgr), uintptr(infoLevel), uintptr(serviceType), uintptr(serviceState), uintptr(unsafe.Pointer(services)), uintptr(bufSize), uintptr(unsafe.Pointer(bytesNeeded)), uintptr(unsafe.Pointer(servicesReturned)), uintptr(unsafe.Pointer(resumeHandle)), uintptr(unsafe.Pointer(groupName)), 0, 0) if r1 == 0 { From 3125361ba60fc0325a24bd4aead72374768fc216 Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Thu, 13 Apr 2023 23:58:33 +0200 Subject: [PATCH 3/9] unix: allow overriding GOOS using GOOS_TARGET in mkpost.go mkpost.go is platform independent and can in principle be run on a GOOS other than the one we're generating the syscall wrappers for. Allow overriding GOOS by setting GOOS_TARGET, similar to other generator programs in the repo. This e.g. allows testing mkpost.go changes on a different GOOS. Follows CL 256278 which did the same for mksyscall.go Change-Id: Ib99aa5cd266f7d27543cf9433cfb028f367eef63 Reviewed-on: https://go-review.googlesource.com/c/sys/+/484636 Run-TryBot: Tobias Klauser Auto-Submit: Tobias Klauser TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor Reviewed-by: David Chase --- unix/mkpost.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/unix/mkpost.go b/unix/mkpost.go index ec31df2c56..5624847c6c 100644 --- a/unix/mkpost.go +++ b/unix/mkpost.go @@ -24,7 +24,10 @@ import ( func main() { // Get the OS and architecture (using GOARCH_TARGET if it exists) - goos := os.Getenv("GOOS") + goos := os.Getenv("GOOS_TARGET") + if goos == "" { + goos = os.Getenv("GOOS") + } goarch := os.Getenv("GOARCH_TARGET") if goarch == "" { goarch = os.Getenv("GOARCH") From 1fb6828e5bff043c4052f75fd5f8290386b08acd Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Thu, 13 Apr 2023 23:53:53 +0200 Subject: [PATCH 4/9] unix: convert Iovec.Base to *byte in mkpost.go on solaris Instead of defining a dedicated type goIovec with the correct *byte type for its Base field like CL 412496 did, modify the Base field of the original type Iovec (generated from struct iovec) in mkpost.go. This is akin to how we already change []int8 to []byte for several other types. Fixes golang/go#55997 Change-Id: If30799bb2bfe6d17678370b45348ff0b7c5de2e9 Reviewed-on: https://go-review.googlesource.com/c/sys/+/484635 Run-TryBot: Tobias Klauser Auto-Submit: Tobias Klauser Reviewed-by: David Chase TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor --- unix/mkpost.go | 11 +++++++++++ unix/types_solaris.go | 8 +------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/unix/mkpost.go b/unix/mkpost.go index 5624847c6c..3d19d3fa75 100644 --- a/unix/mkpost.go +++ b/unix/mkpost.go @@ -101,6 +101,17 @@ func main() { }) } + if goos == "solaris" { + // Convert *int8 to *byte in Iovec.Base like on every other platform. + convertIovecBase := regexp.MustCompile(`Base\s+\*int8`) + iovecType := regexp.MustCompile(`type Iovec struct {[^}]*}`) + iovecStructs := iovecType.FindAll(b, -1) + for _, s := range iovecStructs { + newNames := convertIovecBase.ReplaceAll(s, []byte("Base *byte")) + b = bytes.Replace(b, s, newNames, 1) + } + } + // Intentionally export __val fields in Fsid and Sigset_t valRegex := regexp.MustCompile(`type (Fsid|Sigset_t) struct {(\s+)X__(bits|val)(\s+\S+\s+)}`) b = valRegex.ReplaceAll(b, []byte("type $1 struct {${2}Val$4}")) diff --git a/unix/types_solaris.go b/unix/types_solaris.go index 98a4d85963..89e4c51e25 100644 --- a/unix/types_solaris.go +++ b/unix/types_solaris.go @@ -76,12 +76,6 @@ struct sockaddr_any { char pad[sizeof(union sockaddr_all) - sizeof(struct sockaddr)]; }; -// go_iovec is used to get *byte as the base address for Iovec. -struct goIovec { - void* iov_base; - size_t iov_len; -}; - // Solaris and the major illumos distributions ship a 3rd party tun/tap driver // from https://github.com/kaizawa/tuntap // It supports a pair of IOCTLs defined at @@ -164,7 +158,7 @@ type _Socklen C.socklen_t type Linger C.struct_linger -type Iovec C.struct_goIovec +type Iovec C.struct_iovec type IPMreq C.struct_ip_mreq From 39c2d6a01da63fd9e091bbd231476308d3b193d2 Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Fri, 14 Apr 2023 11:04:58 +0200 Subject: [PATCH 5/9] unix: add UDP socket option constants on linux Change-Id: I4c947319f98754a5135b306d8ff042a986ab5440 Reviewed-on: https://go-review.googlesource.com/c/sys/+/484637 Reviewed-by: David Chase Auto-Submit: Tobias Klauser Reviewed-by: Ian Lance Taylor TryBot-Result: Gopher Robot Run-TryBot: Tobias Klauser --- unix/mkerrors.sh | 3 ++- unix/zerrors_linux.go | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/unix/mkerrors.sh b/unix/mkerrors.sh index 2045d3dadb..be0423e685 100755 --- a/unix/mkerrors.sh +++ b/unix/mkerrors.sh @@ -204,6 +204,7 @@ struct ltchars { #include #include #include +#include #include #include #include @@ -518,7 +519,7 @@ ccflags="$@" $2 ~ /^LOCK_(SH|EX|NB|UN)$/ || $2 ~ /^LO_(KEY|NAME)_SIZE$/ || $2 ~ /^LOOP_(CLR|CTL|GET|SET)_/ || - $2 ~ /^(AF|SOCK|SO|SOL|IPPROTO|IP|IPV6|TCP|MCAST|EVFILT|NOTE|SHUT|PROT|MAP|MFD|T?PACKET|MSG|SCM|MCL|DT|MADV|PR|LOCAL|TCPOPT)_/ || + $2 ~ /^(AF|SOCK|SO|SOL|IPPROTO|IP|IPV6|TCP|MCAST|EVFILT|NOTE|SHUT|PROT|MAP|MFD|T?PACKET|MSG|SCM|MCL|DT|MADV|PR|LOCAL|TCPOPT|UDP)_/ || $2 ~ /^NFC_(GENL|PROTO|COMM|RF|SE|DIRECTION|LLCP|SOCKPROTO)_/ || $2 ~ /^NFC_.*_(MAX)?SIZE$/ || $2 ~ /^RAW_PAYLOAD_/ || diff --git a/unix/zerrors_linux.go b/unix/zerrors_linux.go index 398c37e52d..de936b677b 100644 --- a/unix/zerrors_linux.go +++ b/unix/zerrors_linux.go @@ -2967,6 +2967,7 @@ const ( SOL_TCP = 0x6 SOL_TIPC = 0x10f SOL_TLS = 0x11a + SOL_UDP = 0x11 SOL_X25 = 0x106 SOL_XDP = 0x11b SOMAXCONN = 0x1000 @@ -3251,6 +3252,19 @@ const ( TRACEFS_MAGIC = 0x74726163 TS_COMM_LEN = 0x20 UDF_SUPER_MAGIC = 0x15013346 + UDP_CORK = 0x1 + UDP_ENCAP = 0x64 + UDP_ENCAP_ESPINUDP = 0x2 + UDP_ENCAP_ESPINUDP_NON_IKE = 0x1 + UDP_ENCAP_GTP0 = 0x4 + UDP_ENCAP_GTP1U = 0x5 + UDP_ENCAP_L2TPINUDP = 0x3 + UDP_GRO = 0x68 + UDP_NO_CHECK6_RX = 0x66 + UDP_NO_CHECK6_TX = 0x65 + UDP_SEGMENT = 0x67 + UDP_V4_FLOW = 0x2 + UDP_V6_FLOW = 0x6 UMOUNT_NOFOLLOW = 0x8 USBDEVICE_SUPER_MAGIC = 0x9fa2 UTIME_NOW = 0x3fffffff From 2a33a30b7974dbf88c4f633aa9b5b62b4315509e Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Tue, 18 Apr 2023 12:58:10 +0200 Subject: [PATCH 6/9] execabs: let hasExec return false on wasip1 Wasm cannot execute processes. Follow CL 479622 and update hasExec to match internal/testenv.HasExec. Updates golang/go#58141 Change-Id: Ie44dc356ee589784c44906694fda387fb1448ad5 Reviewed-on: https://go-review.googlesource.com/c/sys/+/485655 Reviewed-by: Ian Lance Taylor Run-TryBot: Tobias Klauser TryBot-Result: Gopher Robot Auto-Submit: Tobias Klauser Reviewed-by: Bryan Mills --- execabs/execabs_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/execabs/execabs_test.go b/execabs/execabs_test.go index cc312f11c0..284bd75a6c 100644 --- a/execabs/execabs_test.go +++ b/execabs/execabs_test.go @@ -21,7 +21,7 @@ import ( // Copied from internal/testenv.HasExec func hasExec() bool { switch runtime.GOOS { - case "js", "ios": + case "wasip1", "js", "ios": return false } return true From 9524d496ef36e9f1656de34a6ff9255376f0169d Mon Sep 17 00:00:00 2001 From: Craig Davison Date: Fri, 21 Apr 2023 18:13:05 +0000 Subject: [PATCH 7/9] windows/svc/mgr: Service.Control: populate Status when returning certain errors Fixes golang/go#59015 Change-Id: I45f22049f3a05f807f78d20c9ed67c6c79e3d3c1 GitHub-Last-Rev: 929aeb4acb899e813a44dee1e9a441876939493c GitHub-Pull-Request: golang/sys#156 Reviewed-on: https://go-review.googlesource.com/c/sys/+/484895 Reviewed-by: Alex Brainman Run-TryBot: Alex Brainman Reviewed-by: Dmitri Shuralyov Reviewed-by: Bryan Mills TryBot-Result: Gopher Robot --- windows/svc/mgr/mgr_test.go | 27 ++++++++++++++++++++++----- windows/svc/mgr/service.go | 14 +++++++++++--- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/windows/svc/mgr/mgr_test.go b/windows/svc/mgr/mgr_test.go index bc763f0604..6f849f3e30 100644 --- a/windows/svc/mgr/mgr_test.go +++ b/windows/svc/mgr/mgr_test.go @@ -17,6 +17,7 @@ import ( "testing" "time" + "golang.org/x/sys/windows" "golang.org/x/sys/windows/svc" "golang.org/x/sys/windows/svc/mgr" ) @@ -109,7 +110,7 @@ func testRecoveryActions(t *testing.T, s *mgr.Service, should []mgr.RecoveryActi if len(should) != len(is) { t.Errorf("recovery action mismatch: contains %v actions, but should have %v", len(is), len(should)) } - for i, _ := range is { + for i := range is { if should[i].Type != is[i].Type { t.Errorf("recovery action mismatch: Type is %v, but should have %v", is[i].Type, should[i].Type) } @@ -131,19 +132,19 @@ func testResetPeriod(t *testing.T, s *mgr.Service, should uint32) { func testSetRecoveryActions(t *testing.T, s *mgr.Service) { r := []mgr.RecoveryAction{ - mgr.RecoveryAction{ + { Type: mgr.NoAction, Delay: 60000 * time.Millisecond, }, - mgr.RecoveryAction{ + { Type: mgr.ServiceRestart, Delay: 4 * time.Minute, }, - mgr.RecoveryAction{ + { Type: mgr.ServiceRestart, Delay: time.Minute, }, - mgr.RecoveryAction{ + { Type: mgr.RunCommand, Delay: 4000 * time.Millisecond, }, @@ -208,6 +209,16 @@ func testRecoveryCommand(t *testing.T, s *mgr.Service, should string) { } } +func testControl(t *testing.T, s *mgr.Service, c svc.Cmd, expectedErr error, expectedStatus svc.Status) { + status, err := s.Control(c) + if err != expectedErr { + t.Fatalf("Unexpected return from s.Control: %v (expected %v)", err, expectedErr) + } + if expectedStatus != status { + t.Fatalf("Unexpected status from s.Control: %+v (expected %+v)", status, expectedStatus) + } +} + func remove(t *testing.T, s *mgr.Service) { err := s.Delete() if err != nil { @@ -251,6 +262,7 @@ func TestMyService(t *testing.T) { t.Fatalf("service %s is not installed", name) } defer s.Close() + defer s.Delete() c.BinaryPathName = exepath c = testConfig(t, s, c) @@ -293,6 +305,11 @@ func TestMyService(t *testing.T) { testRecoveryCommand(t, s, fmt.Sprintf("sc query %s", name)) testRecoveryCommand(t, s, "") // delete recovery command + expectedStatus := svc.Status{ + State: svc.Stopped, + } + testControl(t, s, svc.Stop, windows.ERROR_SERVICE_NOT_ACTIVE, expectedStatus) + remove(t, s) } diff --git a/windows/svc/mgr/service.go b/windows/svc/mgr/service.go index 90f5d95d55..be3d151a3f 100644 --- a/windows/svc/mgr/service.go +++ b/windows/svc/mgr/service.go @@ -45,17 +45,25 @@ func (s *Service) Start(args ...string) error { return windows.StartService(s.Handle, uint32(len(args)), p) } -// Control sends state change request c to the service s. +// Control sends state change request c to the service s. It returns the most +// recent status the service reported to the service control manager, and an +// error if the state change request was not accepted. +// Note that the returned service status is only set if the status change +// request succeeded, or if it failed with error ERROR_INVALID_SERVICE_CONTROL, +// ERROR_SERVICE_CANNOT_ACCEPT_CTRL, or ERROR_SERVICE_NOT_ACTIVE. func (s *Service) Control(c svc.Cmd) (svc.Status, error) { var t windows.SERVICE_STATUS err := windows.ControlService(s.Handle, uint32(c), &t) - if err != nil { + if err != nil && + err != windows.ERROR_INVALID_SERVICE_CONTROL && + err != windows.ERROR_SERVICE_CANNOT_ACCEPT_CTRL && + err != windows.ERROR_SERVICE_NOT_ACTIVE { return svc.Status{}, err } return svc.Status{ State: svc.State(t.CurrentState), Accepts: svc.Accepted(t.ControlsAccepted), - }, nil + }, err } // Query returns current status of service s. From 6c5289959c9811faffd66524f62cdcda16aa2d5c Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Sat, 22 Apr 2023 16:47:22 +1000 Subject: [PATCH 8/9] windows: return error if DecomposeCommandLine parameter contains NUL DecomposeCommandLine is documented to use CommandLineToArgv, and the CommandLineToArgvW system call inherently does not support strings with internal NUL bytes. This CL changes DecomposeCommandLine to reject those strings with an error instead of panicking. Fixes golang/go#58817 Change-Id: I22a026bf2e69344a21f04849c50ba19b6e7b2007 Reviewed-on: https://go-review.googlesource.com/c/sys/+/487695 TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor Reviewed-by: Dmitri Shuralyov Run-TryBot: Alex Brainman --- windows/exec_windows.go | 7 ++++++- windows/syscall_windows_test.go | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/windows/exec_windows.go b/windows/exec_windows.go index 75980fd44a..a52e0331d8 100644 --- a/windows/exec_windows.go +++ b/windows/exec_windows.go @@ -95,12 +95,17 @@ func ComposeCommandLine(args []string) string { // DecomposeCommandLine breaks apart its argument command line into unescaped parts using CommandLineToArgv, // as gathered from GetCommandLine, QUERY_SERVICE_CONFIG's BinaryPathName argument, or elsewhere that // command lines are passed around. +// DecomposeCommandLine returns error if commandLine contains NUL. func DecomposeCommandLine(commandLine string) ([]string, error) { if len(commandLine) == 0 { return []string{}, nil } + utf16CommandLine, err := UTF16FromString(commandLine) + if err != nil { + return nil, errorspkg.New("string with NUL passed to DecomposeCommandLine") + } var argc int32 - argv, err := CommandLineToArgv(StringToUTF16Ptr(commandLine), &argc) + argv, err := CommandLineToArgv(&utf16CommandLine[0], &argc) if err != nil { return nil, err } diff --git a/windows/syscall_windows_test.go b/windows/syscall_windows_test.go index c72c788f0b..42c01fc968 100644 --- a/windows/syscall_windows_test.go +++ b/windows/syscall_windows_test.go @@ -626,6 +626,22 @@ func TestCommandLineRecomposition(t *testing.T) { continue } } + + // check that windows.DecomposeCommandLine returns error for strings with NUL + testsWithNUL := []string{ + "\x00abcd", + "ab\x00cd", + "abcd\x00", + "\x00abcd\x00", + "\x00ab\x00cd\x00", + "\x00\x00\x00", + } + for _, test := range testsWithNUL { + _, err := windows.DecomposeCommandLine(test) + if err == nil { + t.Errorf("Failed to return error while decomposing %#q string with NUL inside", test) + } + } } func TestWinVerifyTrust(t *testing.T) { From ca59edaa5a761e1d0ea91d6c07b063f85ef24f78 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 3 May 2023 16:57:05 -0400 Subject: [PATCH 9/9] windows: use unsafe.Add instead of pointer arithmetic on a uintptr The existing uintptr arithmetic is arguably valid because the environment block is not located within the Go heap (see golang/go#58625). However, unsafe.Add (added in Go 1.17) expresses the same logic with fewer conversions, and in addition avoids triggering the unsafeptr vet check. For golang/go#41205. Change-Id: Ifc509279a13fd707be570908ec779d8518b4f75b Reviewed-on: https://go-review.googlesource.com/c/sys/+/492415 Reviewed-by: Ian Lance Taylor Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot Auto-Submit: Bryan Mills --- windows/env_windows.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/windows/env_windows.go b/windows/env_windows.go index 92ac05ff4e..b8ad192506 100644 --- a/windows/env_windows.go +++ b/windows/env_windows.go @@ -37,14 +37,14 @@ func (token Token) Environ(inheritExisting bool) (env []string, err error) { return nil, err } defer DestroyEnvironmentBlock(block) - blockp := uintptr(unsafe.Pointer(block)) + blockp := unsafe.Pointer(block) for { - entry := UTF16PtrToString((*uint16)(unsafe.Pointer(blockp))) + entry := UTF16PtrToString((*uint16)(blockp)) if len(entry) == 0 { break } env = append(env, entry) - blockp += 2 * (uintptr(len(entry)) + 1) + blockp = unsafe.Add(blockp, 2*(len(entry)+1)) } return env, nil }