Repository navigation
Conversation
|
Not a real review: But it might make sense to change the first line of the commit message to |
Upstream enabled ASLR for the Cygwin DLL in 3.4.0 (943433b, "Cygwin: Enable dynamicbase on the Cygwin DLL by default"). MSYS2 dropped the flag one day after updating to 3.4.3, because of fork() failures seen only in Docker (MSYS2-packages PR 3425: "might be related to 3.4.x setting dynamicbase. Let's see if reverting that helps."). The cause was never identified and the revert has been carried ever since. In the weeks and months after that, upstream fixed several fork() regressions from exactly that period, none of them container-specific: the child-side initialisation order broken by 30add3e (fixed in 3.4.4 by a81fef5), the allocation of shared regions under ASLR (dc0fe77 and follow-ups, 3.4.4 and 3.4.5), and the cygheap size computation introduced by the ASLR work itself (a14a0e5, 3.4.7), whose symptom was the same "child_copy: ... read copy failed, Win32 error 299" as the Docker reports. Without the flag, msys-2.0.dll is relocated by Windows' "Force randomization for images (Mandatory ASLR)", the forked child no longer finds the DLL at the parent's address, and every MSYS program fails with "forked process ... died unexpectedly, ... exit code 0xC0000142" (msys2#327, git-for-windows/git#1412). Under process isolation that host policy also applies inside Windows containers, so on a hardened host MSYS2 cannot fork in a container at all. With only the dynamicbase bit set on the shipped 3.6.10 DLL, fork() works under Mandatory ASLR without exemptions on Windows 11, and inside Windows containers on Server 2022 and Server 2025 under both process and Hyper-V isolation, 50 of 50 forks in every combination (reproduction: https://github.com/alirobe/msys2-aslr-fork-test). Restore upstream's link flag, which also drops one patch from the MSYS2 series. Addresses: msys2#327 Assisted-by: Claude Fable 5.1 Signed-off-by: Ali Robertson <[email protected]>
0d72e9a to
087ee62
Compare
|
Done. It's 3am here in oz, so I am logging off for the night - sorry if unresponsive for next few hours. Appreciate any feedback/review. |
|
Follow-up: this change is necessary but not sufficient for running without exemptions under Mandatory ASLR. 72 of the 74 DLLs in Git for Windows 2.55.0's With only the runtime flagged, So exemption-free use also needs the package DLLs linked with |
|
Update: building the DLLs with I changed the msys default in Branch: https://github.com/alirobe/MSYS2-packages/tree/dynamicbase-default Test setup: a copy of Git for Windows 2.55.0 with this PR's runtime flag, under Mandatory ASLR with no exemptions. So one-line default plus rebuilds looks like enough. I'll make a PR to MSYS2-packages for it. |
MSYS2 quickly removed the --dynamicbase link flag that upstream Cygwin introduced in 3.4.0, on 2022-12-17 (msys2/MSYS2-packages#3425). This was due to Docker-only fork() failures that emerged one day after updating to 3.4.3. The precise reason for those failures was never found.
Subsequently, upstream Cygwin fixed three fork() regressions from that period in 3.4.4, 3.4.5 and 3.4.7:
I think this solved the problem, so this patch brings back --dynamicbase from upstream.
Without this flag, Mandatory ASLR breaks every MSYS program (#327, git-for-windows/git#1412), inside process-isolated containers too. Git for Windows even has an explicit installer step to help solve this for people.
I tested the on the shipped 3.6.10 DLL with only the dynamicbase bit set, and again on the DLL CI builds from this branch (DllCharacteristics 0x0040, no high-entropy VA): on Windows 11 with Mandatory ASLR on and no exemptions, 200 subshell forks, pipelines, git and perl all work, where the stock DLL fails on the first fork.
In Windows containers on Server 2022 and Server 2025 runners, under both process and Hyper-V isolation, 50/50 forks in every combination: https://github.com/alirobe/msys2-aslr-fork-test (run 37634248186).
So I think it's safe to removes the previous emergency patch from the series, bringing our behavior closer to upstream stable behavior.
Supersedes #380.
Assisted by Claude Fable 5.1; reviewed, tested and signed off by the author.