-
Notifications
You must be signed in to change notification settings - Fork 17.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
syscall: Windows user32 function (SendInput) behaves incorrectly when called within golang environment #31685
Comments
/cc @alexbrainman |
Go When calling dll and mfc calling dll there are some differences, for example, creating a file overwrite in ddl, mfc will not report an error when calling dll, and go to dll to prompt the file to exist. For example, get the form in dll Location information, the value of the data returned in the test of go and mfc is inconsistent (location information is obtained in ddl). Guess this may be due to win10, the above problems are triggered in win10. |
I'm not sure but I think you might need to use LockOsThread to be sure that the DLL's functions are executed on the main thread. |
@beoran doesn't make a difference, the example program still outputs an incorrect number of messages while the error message is accurate (when LockOSThread is used and the keys are sent over a channel to the goroutine issuing the syscall) |
Also tried this with multiple events (1, 2, 4, ...), the first return value seems to always return the number of events. |
Could you show me how you are using LockOsThread? You should use it in an init() function and make arrangements so the goroutine that makes the dll call is running the main thread of your function. Also, maybe the layout of the struct in Go is not the same as the C one due to alignment and padding. See here for more on this topic: https://go101.org/article/memory-layout.html |
I discovered this problem when loading my own DLL in Go which in turn spawned a new thread in C that performed SendInput. If I load my own DLL using a thin C executable rather than Go and run the same code, there is no bug. Therefore, it is not related to how the struct is written in my post or LockOsThread() but somehow related to the environment of Go. |
My code (and windows machine) is not with me right now, but I was also calling LockOSThread in |
https://play.golang.org/p/R1KtROnELm4 This program combines the "mainthread" package and the original example. Running it as a process on Win10 Pro yields the same result as the original example. That is, the number "1" is returned even when there is an "access denied error" after bringing the task manager into the foreground and making it the active window. @beoran Is there a mistake in the way the program was combined, or do we conclude that the main thread is not the underlying cause? |
I am actually skeptical that such an elaborate setup was necessary for this example this beyond calling
Could the runtime somehow be executing the syscall piece on a different thread even though there is a 1-1 goroutine/thread ratio on the "main" thread locked by init? |
I think LockOSThread is OK now, so perhaps it is then the struct layout that is the problem, as I suggested before. See here for more on this topic: https://go101.org/article/memory-layout.html Basically, it is possible that on 64 bit architectures, the go structs have padding between the uint16 members such as wVk and wScan. Like that you could be passing incorrect parameters to the windows DLL function. You could try to replace the struct by a sufficiently large byte array where you carefully place the data where the Windows API expects it. You can check the struct member offsets in C by using the ofsetof() macro, and then use those to pack the data into the byte array. |
@yohan1234 I played around with your code, and I cannot explain why your Go program output is different from C. I am not really a Windows expert. Sorry. Alex |
Thanks for all the help guys, I really appreciate you taking the time. The problem is definitely not the struct layout because the problem persists even if SendInput is called from within a DLL written in C and loaded by go. If it helps, I can post such a minimal example to eliminate that suspicion. It would be great to know what calls the go runtime performs prior to the entry point that modifies the process/thread state. I wonder if perhaps the internal runtime calls some user32 functions and corrupts the state in some way. |
I've tested this as well. SendInput appears to behave differently when called from a go process than when it's called from a process created by a c program compiled with msvc. Who maintains the windows go runtime? |
I got lost following the state of this bug. What's the current minimal repro? |
@bradfitz The minimal repro is still in my original post. I have narrowed the bug down to it being due to the go windows runtime. I think that a developer familiar with the go windows runtime might have some valuable insight into this issue. Particularly:
|
@jordanrh1, any ideas? |
Go isn't an interpreter, it's a real compiler that compiles to machine code. It executes all code in init() functions from all packages included in a program. The run time does call some platform specific setup functions, also on windows, as can be seen here: https://github.com/golang/go/blob/master/src/runtime/os_windows.go . From what I see in there, it looks like no functions from user32.dll are called, only from kernel32.dll. It also looks like no security settings are changed. But does configure threads, and does do dll loading. Also, the Go compiler has it's own assembler and linker and uses nothing from mingw. For gccgo this might be different, so maybe you could compile the go program with gccgo in stead of plain go and see it it works better or not. |
My understanding is that the runtime package bundles a few platform specific helper functions that the go compiler stitches together and calls in order to assemble an exe along with other things done directly by the compiler and linker. This seems to be where the pe is assembled: The reason why I'm prodding the pe/w32 stuff is that there might be an issue with how the program identifies itself to Windows. Apart from the packages ld and runtime, is there anything else that does platform specific things when assembling the exe in the repro? |
Your question is too general. Perhaps, you could narrow it down. The only things I can think of are Go runtime installs exception handler and ctl handler (SetConsoleCtrlHandler).
As far as I could see (I grepped Go repo). Your program above does not even loads user32.dll.
Sounds like a long shot.
Again, this question it too general. I would say you need someone who can use debug Windows DLLs / kernel. That person will be able to tell why your SendInput calls return different values when called from C and from Go. I am not such person. Sorry. Alex |
The link you posted is that of the Go linker. It seems like a long shot that there would be a bug in the linker. Rather, I checked the documentation of the SendInput function here https://docs.microsoft.com/en-us/windows/desktop/api/winuser/nf-winuser-sendinput According to the uipi Wikipedia article here https://en.m.wikipedia.org/wiki/User_Interface_Privilege_Isolation, you will need a manifest file and then sign the executable. I don't know if Go can already do that for you. If not, that is then the true "bug" or rather missing feature of Go. It would be nice if Go had tools to help you sign the executable on all platforms that require signed executables. For the time being you could use the Microsoft SignTool : https://stackoverflow.com/questions/252226/signing-a-windows-exe-file |
I don't know about signing. But you can add manifest to any executable. Manifest is stored in PE file section. I am sure there are tools to do that to any executable file. Alex |
This is not the same issue. That developer reports that the return value is zero and that an ACCESS_DENIED (5) error is generated. This is what the C version does when the secure desktop is shown and this is the behavior that is expected. This is not how the Go version of the program behaves.
The expected behavior of SendInput() is to return zero and to generate an error if it is blocked by UIPI. This is what we are simulating in the repro. When we run the repro unelevated at the secure desktop, we expect it to be blocked and to generate an error. This happens in the C example but in the Go example the return value is malformed. If you circumvent UIPI then SendInput() would never return zero or an error during these conditions. We need SendInput() to return zero when it cannot dispatch key events. In our actual program we use this to deduce whether or not input queues have changed and not if there is a security mechanism blocking SendInput().
We use https://github.com/akavel/rsrc for embedding manifests and icons into our golang programs. I've tried embedding a manifest but unfortunately that does not affect the behavior. To narrow it down a bit further:
This is a long shot kind of bug guys, let me know if you have more simple ideas as to why this bug happens. |
One thing you could still try is to call the SendInput function using cgo in stead of directly. Maybe that will give different results? |
If you spawn a thread from inside of a dll that in turn runs SendInput, then the bug still persists and differs if called from go than from c. The bug happens if the executable is booted by go and not from c, not dependent on how you call SendInput. |
@zx2c4 do you have explanation for this issue (see issue description)? I am randomly pinging you, because you might have tools and skills to answer my question. Feel free to ignore my call. Thank you. Alex |
So far as I could tell, when xxxSendInput in win32kfull.sys calls xxxInternalKeyEventDirect, and it fails by returning 0 and setting the userspace last error to 5 (converted from ntstatus 0xc0000022), the main loop in xxxSendInput returns early. But this only happens if tagTHREADINFO+624 for the calling current thread is >=0x501, and otherwise it keeps looping through the input array and incrementing the variable it will later return to you, which is the aberrant behavior you're seeing. It's been a few years since I trudged into the miseries of trawling the latest PDBs for this and I didn't take a look at what that member is, but maybe it's some struct version number or app compat value. Interestingly, your Go example does the right thing if you add somewhere: // static void dummy(void) { }
import "C" So apparently mingw or cgo initialization is doing something right, which influences that member of the thread struct, or maybe the existence of that system thread it spawns, or cgo's brief TLS setup. |
I walked away from this earlier attempting to be content that I at least contributed "something". Unfortunately I've been thoroughly nerd sniped and so this has been nagging at me.
So at the moment I'm guessing we're sticking the wrong version number in our generated PE file. I'll go try to confirm that this is the case, then consult the Go documentation on what the minimum version we support is, and then try to formulate a patch. |
Okay the fix was trivial. I'll have a gerrit thing up in a minute. |
Change https://golang.org/cl/178977 mentions this issue: |
Can we get a backport to 1.12? |
@networkimprov I didn't see much stuff that looked like potential win32k calls in mnm-hammer. There something else you have in mind you'd need this backported for? (The backport should be trivial to do, by the way. Just look at the simplicity of the 1.13 patch.) |
It seems like this could quietly interfere with both Go internals and some fraction of Go apps... |
|
@gopherbot Please open backport to 1.12. |
Backport issue(s) opened: #32261 (for 1.12). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
Change https://golang.org/cl/178738 mentions this issue: |
There is no regression and this change produces undocumented changes in the way win32 reacts to syscalls. Are we sure we want those undocumented and unknwown changes to happen in a minor release for the purpose of fixing a request that never worked in Go? |
Sorry for dragging you in. At least we all know what the problem is. :-)
I agree, this change is not suitable for minor release. This is how Go executables were configured from the start of Go. And this change is high risk even for standard release - what other effects CL 178977 will have? Alex |
I'll close the PRs and whatnot for the backport then.
It's really not "high risk". The ancient-PE compatibility stuff is really super nasty. There are some code paths (bugs!) I saw in there that probably have not been tested in decades. So I think all and all, this patch is a huge improvement, in the sense that the functions now actually match what MSDN (and by extension, our documentation) say. Also, I imagine a pretty good portion of Go Windows apps pull in MingW, whose linker bumps the NT version. So already we've had some real world testing of this. |
No, it removes undocumented behavior in exchange for the documented behavior. |
I agree. But, in the mean time, this CL might change the way Go executables behave in a way that we don't test / observe. Just like it has been demonstrated by this current issue.
External linker is only used if you use cgo. There are many downsides to cgo, so I doubt it is common. Especially on Windows, which does not come with C compiler and libs. Alex |
As I wrote in the commit message, I think most people doing serious UI work with Go on Windows are using CGo's linker, because otherwise the stack explodes. It's definitely fairly tested. Anyway, we'll have the fix in 1.13, which still has some time for regression testing, and 1.12 users can wait. I think the plan here is fine enough. |
Thanks for the help! Glad to see this fixed in 1.13. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
In our application we use ffi to perform some things in native code on Windows. In particular, we use SendInput() on Windows which dispatches mouse events and keyboard events to the OS. When a DLL is loaded by the go runtime, this function does not behave as expected. This is also when using the syscall library directly with user32.dll to call the function using pure go code.
The unexpected behavior seems to be a result of how the runtime is setup in golang. This bug is quite serious because it might have unintended consequences when interacting with the windows runtime. It might be possible that other w32 functions behave differently than expected.
We are using the Windows user32 function SendInput: https://docs.microsoft.com/en-us/windows/desktop/api/winuser/nf-winuser-sendinput
Go version
C version
What did you expect to see?
With both binaries:
Expected output (as in the C version):
ret: 1 err: 0
ret: 1 err: 0
ret: 1 err: 0
ret: 0 err: 5
ret: 0 err: 5
ret: 0 err: 5
ret: 1 err: 5
ret: 1 means that 1 event was dispatched
ret: 0 means that no events were dispatched
err: 5 means access denied in windows
When the secure desktop was present, ret is 0 and err is 5. This is as expected. No events were dispatched and an error was generated.
What did you see instead?
In the golang version you get:
2019/04/25 14:20:43 ret: 1 error: The operation completed successfully.
2019/04/25 14:20:44 ret: 1 error: The operation completed successfully.
2019/04/25 14:20:45 ret: 1 error: The operation completed successfully.
2019/04/25 14:20:46 ret: 1 error: Access is denied.
2019/04/25 14:20:47 ret: 1 error: Access is denied.
2019/04/25 14:20:48 ret: 1 error: Access is denied.
2019/04/25 14:20:49 ret: 1 error: The operation completed successfully.
The bug
ret is always 1 when you are at the secure desktop and SendInput is used in golang, regardless of whether or not an event was dispatched. We know that an event was not dispatched because an error is generated (5 = Access is denied.)
This is worrisome because you are not suppose to check for any errors unless the return value of SendInput is 0.
Remember that this has nothing to do with the syscall calling convention in golang or anything like that. You can put the c code snippet in a dll, call it from a standard C runtime and get a different result than if called from a go runtime.
This bug is quite serious for us because the behavior of user32 changes when interacting with go. It might be the case that other functions also behave erroneously.
The text was updated successfully, but these errors were encountered: