[Bug 37034] New: Stars!: Crashes on startup
http://bugs.winehq.org/show_bug.cgi?id=37034 Bug ID: 37034 Summary: Stars!: Crashes on startup Product: Wine Version: 1.7.23 Hardware: x86 OS: Mac OS X Status: UNCONFIRMED Severity: normal Priority: P2 Component: -unknown Assignee: wine-bugs(a)winehq.org Reporter: planetbeing(a)gmail.com Created attachment 49213 --> http://bugs.winehq.org/attachment.cgi?id=49213 Console output when the crash occured Page fault happens in DOSVM_AllocCodeUMB of krnl386.exe16. The apparent cause is that the DOS upper memory block was never actually allocated before being used. I don't know how to attach multiple attachments, so this is a patch I made that appeared to fix the problem. I don't know much about the internals of Wine so I'm not sure if this is a good patch or not. diff --git a/dlls/krnl386.exe16/dosvm.c b/dlls/krnl386.exe16/dosvm.c index 87adf33..7a3c96e 100644 --- a/dlls/krnl386.exe16/dosvm.c +++ b/dlls/krnl386.exe16/dosvm.c @@ -881,6 +881,8 @@ void DOSVM_InitSegments(void) 0xfb, 0x66, 0xcb /* sti and 32-bit far return */ }; + VirtualAlloc((void *)DOSVM_UMB_BOTTOM, DOSVM_UMB_TOP - DOSVM_UMB_BOTTOM, MEM_RESERVE | MEM_COMMIT, PAGE_EXECUTE_READWRITE); + /* * Allocate pointer array. */ -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=37034 --- Comment #1 from planetbeing(a)gmail.com --- Created attachment 49214 --> http://bugs.winehq.org/attachment.cgi?id=49214 Patch that appears to fix the problem for me -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=37034 --- Comment #2 from planetbeing(a)gmail.com --- Found the real reason for this. The space was reserved by the WINE_DOS segment. However, due to ASLR on Mac, that segment is always put into a random location and does not start at 0x1000 as expected. It seems to me that the correction solution is to reserve those segments correcting for the ASLR slide. The previous patch helped but was unreliable. This new patch seems to have solved the problem for good. diff --git a/loader/main.c b/loader/main.c index ac67290..80e5523 100644 --- a/loader/main.c +++ b/loader/main.c @@ -42,6 +42,7 @@ #include "main.h" #ifdef __APPLE__ +#include <mach-o/dyld.h> #ifndef __clang__ __asm__(".zerofill WINE_DOS, WINE_DOS, ___wine_dos, 0x40000000"); @@ -63,6 +64,7 @@ static const struct wine_preload_info wine_main_preload_info[] = static inline void reserve_area( void *addr, size_t size ) { + addr = (void*)((uintptr_t)addr - _dyld_get_image_vmaddr_slide(0)); wine_anon_mmap( addr, size, PROT_NONE, MAP_FIXED | MAP_NORESERVE ); wine_mmap_add_reserved_area( addr, size ); } -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=37034 planetbeing(a)gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #49214|0 |1 is obsolete| | --- Comment #3 from planetbeing(a)gmail.com --- Created attachment 49216 --> http://bugs.winehq.org/attachment.cgi?id=49216 Patch that fixes WINE_DOS segment allocation under ASLR properly -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37034 planetbeing(a)gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.winehq.org/sho | |w_bug.cgi?id=36367 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37034 Austin English <austinenglish(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch --- Comment #4 from Austin English <austinenglish(a)gmail.com> --- Patches should be sent to wine-patches(a)winehq.org. See http://wiki.winehq.org/SubmittingPatches for more info. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=37034 --- Comment #5 from Ken Thomases <ken(a)codeweavers.com> --- (In reply to planetbeing from comment #2)
Found the real reason for this. The space was reserved by the WINE_DOS segment. However, due to ASLR on Mac, that segment is always put into a random location and does not start at 0x1000 as expected.
If those segments are not ending up in the correct place, then the solution is to fix that. We may need to add "-no_pie" to the linker flags, perhaps instead of "-macosx_version_min 10.6". See the setting of LDEXECFLAGS in configure.ac.
It seems to me that the correction solution is to reserve those segments correcting for the ASLR slide. The previous patch helped but was unreliable. This new patch seems to have solved the problem for good.
I don't understand how making reserve_area() reserve a different area then it was asked to helps anything. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=37034 --- Comment #6 from planetbeing(a)gmail.com --- (In reply to Ken Thomases from comment #5)
(In reply to planetbeing from comment #2)
Found the real reason for this. The space was reserved by the WINE_DOS segment. However, due to ASLR on Mac, that segment is always put into a random location and does not start at 0x1000 as expected.
If those segments are not ending up in the correct place, then the solution is to fix that. We may need to add "-no_pie" to the linker flags, perhaps instead of "-macosx_version_min 10.6". See the setting of LDEXECFLAGS in configure.ac.
I think it's preferable not to disable security features if possible when the application could be made to work without disabling them.
It seems to me that the correction solution is to reserve those segments correcting for the ASLR slide. The previous patch helped but was unreliable. This new patch seems to have solved the problem for good.
I don't understand how making reserve_area() reserve a different area then it was asked to helps anything.
The linker creates an executable with the WINE_DOS segment at 0x1000. On load of the executable, dyld adds a slide to executable marked MH_PIE. The __wine_dos symbol is relocated to 0x1000 + slide. When reserve_area is called, it is called with 0x1000 + random slide. At least for WINE_DOS, asking for a reservation at 0x1000 + random slide is not useful. There's no way to prevent that while still referring to the area to be reserved through a symbol whose location is defined by the compiler flag "-Wl,-segaddr" without subtracting the slide first before calling reserve_area or else subtracting the slide in reserve_area itself. It's still useful to define the linker segments to prevent the rest of the code from ever being placed too low, but the actual reservation must happen at non-slid addresses (at least for WINE_DOS, not sure if WINE_SHAREDHEAP has specific address requirements). It's possible to fix this with -Wl,-no_pie but that seems to me to be needlessly removing a security mitigation. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=37034 --- Comment #7 from Ken Thomases <ken(a)codeweavers.com> --- (In reply to planetbeing from comment #6)
The linker creates an executable with the WINE_DOS segment at 0x1000. On load of the executable, dyld adds a slide to executable marked MH_PIE.
It's still useful to define the linker segments to prevent the rest of the code from ever being placed too low, but the actual reservation must happen at non-slid addresses (at least for WINE_DOS, not sure if WINE_SHAREDHEAP has specific address requirements).
No, it's not useful. The WINE_DOS segment is not just used to make sure no other part of the wineloader gets put too low, but that nothing else that gets loaded or initialized before main() gets put there, either. The WINE_DOS segment really has to be put at 0x1000 or we might as well not have it at all.
It's possible to fix this with -Wl,-no_pie but that seems to me to be needlessly removing a security mitigation.
Wine needs to be loaded at a specific address. That's why, for example, it uses "-image_base 0x7bf00000". I don't think ASLR is acceptable for Wine. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=37034 --- Comment #8 from planetbeing(a)gmail.com --- (In reply to Ken Thomases from comment #7)
(In reply to planetbeing from comment #6)
The linker creates an executable with the WINE_DOS segment at 0x1000. On load of the executable, dyld adds a slide to executable marked MH_PIE.
It's still useful to define the linker segments to prevent the rest of the code from ever being placed too low, but the actual reservation must happen at non-slid addresses (at least for WINE_DOS, not sure if WINE_SHAREDHEAP has specific address requirements).
No, it's not useful. The WINE_DOS segment is not just used to make sure no other part of the wineloader gets put too low, but that nothing else that gets loaded or initialized before main() gets put there, either. The WINE_DOS segment really has to be put at 0x1000 or we might as well not have it at all.
It's possible to fix this with -Wl,-no_pie but that seems to me to be needlessly removing a security mitigation.
Wine needs to be loaded at a specific address. That's why, for example, it uses "-image_base 0x7bf00000". I don't think ASLR is acceptable for Wine.
In light of the fact there are other code dependencies on Wine being loaded into a specific address, then -no_pie is definitely the right way to go. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37034 --- Comment #9 from Ken Thomases <ken(a)codeweavers.com> --- I've submitted a patch to use -no_pie when linking the wineloader: http://source.winehq.org/patches/data/107101 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37034 --- Comment #10 from Ken Thomases <ken(a)codeweavers.com> --- The -no_pie change was committed: http://source.winehq.org/git/wine.git/?a=commit;h=5ddaf34d69fe9608e45abb65a4... Please retest with Wine 1.7.29. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37034 Bruno Jesus <00cpxxx(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |5ddaf34d69fe9608e45abb65a48 | |854b22fa3610a Resolution|--- |FIXED Status|UNCONFIRMED |RESOLVED --- Comment #11 from Bruno Jesus <00cpxxx(a)gmail.com> --- Assuming fixed after 2 years. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37034 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #12 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 2.1. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org