runtime: Go 1.5.4/1.6.1 breaks running on Windows Nano Server #15286
Comments
|
Sorry, I'd never heard of Nano Server before this. I wrote b8af3fd but I did so blindly using MSDN docs (where I didn't see any reference to Nano Server), and testing on Go's trybot buidlers. Another contributor (@alexbrainman) verified it still passes on Windows XP, since we don't have a Windows XP build machine anymore (I'm working on resurrecting one). If this Nano Server is something we should support, I suppose we'll need builders for it. I filed #15287 for that. Our bar for Go point releases is very high. I don't think we would issue a point release for an unreleased in-development operating system, even for our Docker friends. We do, however, have another point release coming for all the other Go 1.6.x non-security bugs (1.6.2: https://github.com/golang/go/issues?utf8=%E2%9C%93&q=milestone%3AGo1.6.2+), so it's possible we could put some minimal change in there for that. But...
... it does sound like a Windows problem. We're using a that flag bit to say "use a system DLL, not the malicious DLL in the user's Downloads folder". If you moved Windows system DLLs around and need to remap things, I expect Windows to still map things and yet honor our intention with that flag. |
|
I'm marking this as Unplanned (which we use to just mean it's been read, basically), but send a change for review (https://golang.org/doc/contribute.html) and we can consider for 1.6.2 if it arrives soon. |
|
OK, thank you. I understand that this is a difficult request. I agree that this is a Windows problem, and ultimately we need a Windows solution. In the short term, though, we are a bit stuck. I should have an update later today or tomorrow. |
bradfitz
changed the title
|
@jstarks, I sent you CLA info to your microsoft.com email address I saw from Github. Let me know if you didn't get it. |
|
Note that we're unlikely to issue 1.5.5 for this (Go 1.5 is unsupported except for critical security fixes, and this is a non-security problem in an unreleased piece of software). Even if Docker is using 1.5.4, it seems like it should be possible (trivial even) for the Windows Nano Server build to use 1.5.3. Again, Go 1.5 is not supported (see https://golang.org/doc/devel/release.html#policy). I'm also somewhat skeptical of including a fix in 1.6.2, since again this is an unreleased piece of software, there is a plausible workaround (use 1.5.3 or 1.6), the functionality works fine on standard Windows, and it sounds like Windows Nano Server might fix the missing functionality before its official release. Especially since we now understand this section to be a security-critical part of the code, letting the fix work through the standard major release process for Go 1.7 seems preferable than rushing in a last-minute change. |
|
@jstarks please try https://go-review.googlesource.com/22054. I just followed your proposed solution description. I don't have Nano Server to test my CL. And it is your fight to get this change (if accepted) into whatever release you want. Alex |
|
CL https://golang.org/cl/22054 mentions this issue. |
For context: golang/go#15286 This commit downloads go1.5.3 in addition to go1.5.4 in order to workaround the issue. It is not expected to do a Docker release without a proper fix, however this should help unblock Docker development on Windows TP5. Signed-off-by: Tibor Vass <tibor@docker.com>
|
Thanks bradfitz, alexbrainman, and rsc for the quick and helpful response on this. Alex -- I think there is a small problem with my proposal, but see below for an alternative that should be simpler anyway. Docker is working around this for now by rolling back to Go 1.5.3 when building for Windows. This takes the pressure off getting a fix into Go, but of course is not ideal in the long term. I believe we can move to Golang 1.6.x for Docker when building on Windows -- Docker for Linux is stuck on 1.5.x due to some net/http compatibility issues that do not apply for Windows: moby/moby#20865. For Nano Server RTM, we may be able to fix this in the OS, but given our release timelines this will add some complexity that the OS team would like to avoid. We'd like you to consider a fix in Go 1.6.2 as an alternative. Edit: We are evaluating solutions to determine the simplest working one; I'll post back with updates when I have them. |
|
@jstarks I will wait for you to decide what you want before doing anything else. Alex |
|
Sorry for the long delay. Obviously we've missed the Go 1.6.2 window, and probably the Go 1.7 window. However, we've figured out how to fix this in Windows, so I'm closing this issue. Thanks again for everyone's help. It did occur to me while looking at this that there is very little reason for Go to load shell32.dll to begin with. With some small changes we can eliminate that and at least one other DLL; this has the advantage of significantly improving the launch performance of Go processes on Windows. I will open a separate issue for that soon and provide some fixes I've prepared. |
|
Thanks for the update! |
Please do. Thank you. Alex |
This works around golang/go#15286 by explicitly loading shell32.dll at load time, ensuring that syscall can load it dynamically during process startup. Signed-off-by: John Starks <jostarks@microsoft.com>
This works around golang/go#15286 by explicitly loading shell32.dll at load time, ensuring that syscall can load it dynamically during process startup. Signed-off-by: John Starks <jostarks@microsoft.com> Signed-off-by: Antonio Murdaca <runcom@redhat.com>
This regression occurred due to change b8af3fd associated with #14959. These changes, which add the LOAD_LIBRARY_SEARCH_SYSTEM32 flag to LoadLibraryEx calls to system DLLs, are incompatible with Windows Nano Server, which is currently in preview release. Go programs fail to load because shell32.dll cannot be found in the course of calling CommandLineToArgvW to parse the command line.
Nano Server does not have shell32.dll or many other DLLs that are part of standard Windows desktop and Windows Server installations. Nano Server is based on OneCore and as such doesn't have shell functionality; the API in question is moved to a different DLL. OneCore replaces explicit references to system DLLs with "API sets", which are DLL-like things that reference logical sets of APIs that may be hosted by different DLLs on different Windows platforms. For example, CommandLineToArgvW is in api-ms-win-shcore-obsolete-l1-1-0. (I don't know why it's in something marked "obsolete", that seems strange to me, too).
Of course, most software out there is not going to start linking to the new API sets any time soon, so Nano Server also has a mechanism to forward from the old DLLs to the new API sets. This mechanism is known internally as "reverse forwarders". These reverse forwarders allow Go software (and a lot of other open source software) to run on Nano Server without changes.
Unfortunately, the loader will not try to load these reverse forwarders if you pass LOAD_LIBRARY_SEARCH_SYSTEM32. This means that while Go 1.5.3 programs worked great on Nano Server, programs built with 1.5.4 will not. This is quite unfortunate for us since we are trying to support Docker on Nano Server, and Docker has just moved to Go 1.5.4 for the security fixes.
We are working internally at Microsoft to see if/how we can support LOAD_LIBRARY_SEARCH_SYSTEM32 with reverse forwarders. I am hopeful that by RTM of Nano Server we will be able to improve this messy situation. But for now, this change has broken us pretty badly.
One proposal for a workaround would be to revert to the old behavior if kernel32.dll cannot be loaded with LOAD_LIBRARY_SEARCH_SYSTEM32. That would be a pretty good indication that you're on a system that doesn't have a standard set of Windows DLLs, and that you shouldn't pass this flag. I can put together a proposed fix today if that's likely to be acceptable.
The text was updated successfully, but these errors were encountered: