x/sys/windows/svc/mgr: Service.Control() should return valid svc.Status on certain errors

[craig65535]

What version of Go are you using (go version)?

$ go version
1.19.5

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

windows/amd64

What did you do?

On a given *mgr.Service (s), in a state like StopPending that does not accept controls, try:

status, err := s.Control(svc.Stop)

What did you expect to see?

err should be windows.ERROR_INVALID_SERVICE_CONTROL, and status should be filled in with the service's current state and set of accepted commands. This is the behaviour described for ControlService:

[out] lpServiceStatus

A pointer to a SERVICE_STATUS structure that receives the latest service status information. The information returned reflects the most recent status that the service reported to the service control manager.

The service control manager fills in the structure only when GetLastError returns one of the following error codes: NO_ERROR, ERROR_INVALID_SERVICE_CONTROL, ERROR_SERVICE_CANNOT_ACCEPT_CTRL, or ERROR_SERVICE_NOT_ACTIVE. Otherwise, the structure is not filled in.

This behaviour is useful because checking the service state independently of controlling it is a TOCTOU bug.

What did you see instead?

err is correctly set to windows.ERROR_INVALID_SERVICE_CONTROL, but status is unset.

[craig65535]

I am able to work around this by calling windows.ControlService() directly:

var t windows.SERVICE_STATUS
err := windows.ControlService(s.Handle, windows.SERVICE_CONTROL_STOP, &t)

This has the expected behaviour and fills in t when it returns windows.ERROR_INVALID_SERVICE_CONTROL. However, it seems like this should also be in the mgr package.

[cherrymui]

cc @golang/windows

[alexbrainman]

Thank you @craig65535 for filling the issue.

I agree there is a bug in this code.

I suggest we update returned status parameter when error is one of the following error codes: NO_ERROR, ERROR_INVALID_SERVICE_CONTROL, ERROR_SERVICE_CANNOT_ACCEPT_CTRL, or ERROR_SERVICE_NOT_ACTIVE. As per Microsoft documentation.

Would you like to contribute the fix as per https://go.dev/doc/contribute ? Maybe even add new test for it.

Alex

[craig65535]

@alexbrainman Sorry for the delayed response. I'll look into contributing.

[craig65535]

golang/sys#156

[gopherbot]

Change https://go.dev/cl/484895 mentions this issue: windows/svc/mgr: Service.Control: populate Status when returning certain errors