From 2a33a30b7974dbf88c4f633aa9b5b62b4315509e Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Tue, 18 Apr 2023 12:58:10 +0200 Subject: [PATCH 1/4] 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 cc312f11c..284bd75a6 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 2/4] 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 bc763f060..6f849f3e3 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 90f5d95d5..be3d151a3 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 3/4] 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 75980fd44..a52e0331d 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 c72c788f0..42c01fc96 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 4/4] 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 92ac05ff4..b8ad19250 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 }