[PATCH] loader: Check if the preloader overlaps the reserved range on the Mac.
Signed-off-by: Ken Thomases <ken(a)codeweavers.com> --- loader/preloader_mac.c | 48 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/loader/preloader_mac.c b/loader/preloader_mac.c index 1d88062..83d24af 100644 --- a/loader/preloader_mac.c +++ b/loader/preloader_mac.c @@ -48,6 +48,7 @@ #ifdef HAVE_MACH_O_LOADER_H #include <mach/thread_status.h> #include <mach-o/loader.h> +#include <mach-o/ldsyms.h> #endif #include "main.h" @@ -98,6 +99,8 @@ void __stack_chk_fail(void) { return; } static const size_t page_size = 0x1000; static const size_t page_mask = 0xfff; #define target_mach_header mach_header +#define target_segment_command segment_command +#define TARGET_LC_SEGMENT LC_SEGMENT #define target_thread_state_t i386_thread_state_t #ifdef __DARWIN_UNIX03 #define target_thread_ip(x) (x)->__eip @@ -184,6 +187,8 @@ __ASM_GLOBAL_FUNC( start, static const size_t page_size = 0x1000; static const size_t page_mask = 0xfff; #define target_mach_header mach_header_64 +#define target_segment_command segment_command_64 +#define TARGET_LC_SEGMENT LC_SEGMENT_64 #define target_thread_state_t x86_thread_state64_t #ifdef __DARWIN_UNIX03 #define target_thread_ip(x) (x)->__rip @@ -376,6 +381,36 @@ static __attribute__((noreturn,format(printf,1,2))) void fatal_error(const char wld_exit(1); } +static int preloader_overlaps_range( const void *start, const void *end ) +{ + intptr_t slide = p_dyld_get_image_slide(&_mh_execute_header); + struct load_command *cmd = (struct load_command*)(&_mh_execute_header + 1); + int i; + + for (i = 0; i < _mh_execute_header.ncmds; ++i) + { + if (cmd->cmd == TARGET_LC_SEGMENT) + { + struct target_segment_command *seg = (struct target_segment_command*)cmd; + const void *seg_start = (const void*)(seg->vmaddr + slide); + const void *seg_end = (const char*)seg_start + seg->vmsize; + + if (end > seg_start && start <= seg_end) + { + char segname[sizeof(seg->segname) + 1]; + memcpy(segname, seg->segname, sizeof(seg->segname)); + segname[sizeof(segname) - 1] = 0; + wld_printf( "WINEPRELOADRESERVE range %p-%p overlaps preloader %s segment %p-%p\n", + start, end, segname, seg_start, seg_end ); + return 1; + } + } + cmd = (struct load_command*)((char*)cmd + cmd->cmdsize); + } + + return 0; +} + /* * preload_reserve * @@ -406,7 +441,8 @@ static void preload_reserve( const char *str ) else if (result) goto error; /* single value '0' is allowed */ /* sanity checks */ - if (end <= start) start = end = NULL; + if (end <= start || preloader_overlaps_range(start, end)) + start = end = NULL; /* check for overlap with low memory areas */ for (i = 0; preload_info[i].size; i++) @@ -562,6 +598,11 @@ void *wld_start( void *stack, int *is_unix_thread ) p++; } + LOAD_POSIX_DYLD_FUNC( dlopen ); + LOAD_POSIX_DYLD_FUNC( dlsym ); + LOAD_POSIX_DYLD_FUNC( dladdr ); + LOAD_MACHO_DYLD_FUNC( _dyld_get_image_slide ); + /* reserve memory that Wine needs */ if (reserve) preload_reserve( reserve ); for (i = 0; preload_info[i].size; i++) @@ -576,11 +617,6 @@ void *wld_start( void *stack, int *is_unix_thread ) if (!map_region( &builtin_dlls )) builtin_dlls.size = 0; - LOAD_POSIX_DYLD_FUNC( dlopen ); - LOAD_POSIX_DYLD_FUNC( dlsym ); - LOAD_POSIX_DYLD_FUNC( dladdr ); - LOAD_MACHO_DYLD_FUNC( _dyld_get_image_slide ); - /* load the main binary */ if (!(mod = pdlopen( argv[1], RTLD_NOW ))) fatal_error( "%s: could not load binary\n", argv[1] ); -- 2.10.2
Am Mi., 5. Dez. 2018 um 23:22 Uhr schrieb Ken Thomases <ken(a)codeweavers.com>:
@@ -562,6 +598,11 @@ void *wld_start( void *stack, int *is_unix_thread ) p++; }
+ LOAD_POSIX_DYLD_FUNC( dlopen ); + LOAD_POSIX_DYLD_FUNC( dlsym ); + LOAD_POSIX_DYLD_FUNC( dladdr ); + LOAD_MACHO_DYLD_FUNC( _dyld_get_image_slide ); + /* reserve memory that Wine needs */ if (reserve) preload_reserve( reserve ); for (i = 0; preload_info[i].size; i++) @@ -576,11 +617,6 @@ void *wld_start( void *stack, int *is_unix_thread ) if (!map_region( &builtin_dlls )) builtin_dlls.size = 0;
- LOAD_POSIX_DYLD_FUNC( dlopen ); - LOAD_POSIX_DYLD_FUNC( dlsym ); - LOAD_POSIX_DYLD_FUNC( dladdr ); - LOAD_MACHO_DYLD_FUNC( _dyld_get_image_slide ); - /* load the main binary */ if (!(mod = pdlopen( argv[1], RTLD_NOW ))) fatal_error( "%s: could not load binary\n", argv[1] ); -- 2.10.2
Hello Ken, I am not sure anymore if the system libs (dlopen, etc.) are already loaded by the kernel, or if they are loaded on the first _dyld_func_lookup() call. Could you maybe check that? In the second case, changing the order means that there is a higher risk of running into some address space conflicts. Best regards, Sebastian
Hi Sebastian, On Dec 6, 2018, at 9:53 AM, Sebastian Lackner <sebastian(a)fds-team.de> wrote:
Am Mi., 5. Dez. 2018 um 23:22 Uhr schrieb Ken Thomases <ken(a)codeweavers.com>:
@@ -562,6 +598,11 @@ void *wld_start( void *stack, int *is_unix_thread ) p++; }
+ LOAD_POSIX_DYLD_FUNC( dlopen ); + LOAD_POSIX_DYLD_FUNC( dlsym ); + LOAD_POSIX_DYLD_FUNC( dladdr ); + LOAD_MACHO_DYLD_FUNC( _dyld_get_image_slide ); + /* reserve memory that Wine needs */ if (reserve) preload_reserve( reserve ); for (i = 0; preload_info[i].size; i++) @@ -576,11 +617,6 @@ void *wld_start( void *stack, int *is_unix_thread ) if (!map_region( &builtin_dlls )) builtin_dlls.size = 0;
- LOAD_POSIX_DYLD_FUNC( dlopen ); - LOAD_POSIX_DYLD_FUNC( dlsym ); - LOAD_POSIX_DYLD_FUNC( dladdr ); - LOAD_MACHO_DYLD_FUNC( _dyld_get_image_slide ); - /* load the main binary */ if (!(mod = pdlopen( argv[1], RTLD_NOW ))) fatal_error( "%s: could not load binary\n", argv[1] );
I am not sure anymore if the system libs (dlopen, etc.) are already loaded by the kernel, or if they are loaded on the first _dyld_func_lookup() call. Could you maybe check that? In the second case, changing the order means that there is a higher risk of running into some address space conflicts.
Thanks for reviewing! I considered the question of whether looking up the functions earlier might load additional images, but it shouldn't. Those functions don't come from libSystem, they come from dyld itself (which is, of course, already loaded). I just confirmed by forcing a crash just before dlopen-ing the main executable and looking at the macOS crash report. It shows the loaded images and it's just wine-preloader and dyld. -Ken
Am Do., 6. Dez. 2018 um 17:09 Uhr schrieb Ken Thomases <ken(a)codeweavers.com>:
Hi Sebastian,
On Dec 6, 2018, at 9:53 AM, Sebastian Lackner <sebastian(a)fds-team.de> wrote:
Am Mi., 5. Dez. 2018 um 23:22 Uhr schrieb Ken Thomases <ken(a)codeweavers.com>:
@@ -562,6 +598,11 @@ void *wld_start( void *stack, int *is_unix_thread ) p++; }
+ LOAD_POSIX_DYLD_FUNC( dlopen ); + LOAD_POSIX_DYLD_FUNC( dlsym ); + LOAD_POSIX_DYLD_FUNC( dladdr ); + LOAD_MACHO_DYLD_FUNC( _dyld_get_image_slide ); + /* reserve memory that Wine needs */ if (reserve) preload_reserve( reserve ); for (i = 0; preload_info[i].size; i++) @@ -576,11 +617,6 @@ void *wld_start( void *stack, int *is_unix_thread ) if (!map_region( &builtin_dlls )) builtin_dlls.size = 0;
- LOAD_POSIX_DYLD_FUNC( dlopen ); - LOAD_POSIX_DYLD_FUNC( dlsym ); - LOAD_POSIX_DYLD_FUNC( dladdr ); - LOAD_MACHO_DYLD_FUNC( _dyld_get_image_slide ); - /* load the main binary */ if (!(mod = pdlopen( argv[1], RTLD_NOW ))) fatal_error( "%s: could not load binary\n", argv[1] );
I am not sure anymore if the system libs (dlopen, etc.) are already loaded by the kernel, or if they are loaded on the first _dyld_func_lookup() call. Could you maybe check that? In the second case, changing the order means that there is a higher risk of running into some address space conflicts.
Thanks for reviewing!
I considered the question of whether looking up the functions earlier might load additional images, but it shouldn't. Those functions don't come from libSystem, they come from dyld itself (which is, of course, already loaded).
I just confirmed by forcing a crash just before dlopen-ing the main executable and looking at the macOS crash report. It shows the loaded images and it's just wine-preloader and dyld.
-Ken
Hello Ken, since it seems pretty easy to check, I would also recommend to verify that dyld is indeed already loaded before calling any of the lookup functions. We only link against a small *.o file, so in theory, the kernel could still use lazy-loading for the dylib. If this shows that dyld is already loaded, then it indeed doesn't make any difference. BTW: Thanks for your effort to get this upstream. Best regards, Sebastian
December 6, 2018 10:20 AM, "Sebastian Lackner" <sebastian(a)fds-team.de> wrote:
since it seems pretty easy to check, I would also recommend to verify that dyld is indeed already loaded before calling any of the lookup functions.
But dyld *must* always be mapped in by the kernel. Otherwise, it would not be possible to load any other images at all. Therefore, I think we can safely assume that dyld is loaded. Chip
On Dec 6, 2018, at 10:20 AM, Sebastian Lackner <sebastian(a)fds-team.de> wrote:
Am Do., 6. Dez. 2018 um 17:09 Uhr schrieb Ken Thomases <ken(a)codeweavers.com>:
Hi Sebastian,
On Dec 6, 2018, at 9:53 AM, Sebastian Lackner <sebastian(a)fds-team.de> wrote:
Am Mi., 5. Dez. 2018 um 23:22 Uhr schrieb Ken Thomases <ken(a)codeweavers.com>:
@@ -562,6 +598,11 @@ void *wld_start( void *stack, int *is_unix_thread ) p++; }
+ LOAD_POSIX_DYLD_FUNC( dlopen ); + LOAD_POSIX_DYLD_FUNC( dlsym ); + LOAD_POSIX_DYLD_FUNC( dladdr ); + LOAD_MACHO_DYLD_FUNC( _dyld_get_image_slide ); + /* reserve memory that Wine needs */ if (reserve) preload_reserve( reserve ); for (i = 0; preload_info[i].size; i++) @@ -576,11 +617,6 @@ void *wld_start( void *stack, int *is_unix_thread ) if (!map_region( &builtin_dlls )) builtin_dlls.size = 0;
- LOAD_POSIX_DYLD_FUNC( dlopen ); - LOAD_POSIX_DYLD_FUNC( dlsym ); - LOAD_POSIX_DYLD_FUNC( dladdr ); - LOAD_MACHO_DYLD_FUNC( _dyld_get_image_slide ); - /* load the main binary */ if (!(mod = pdlopen( argv[1], RTLD_NOW ))) fatal_error( "%s: could not load binary\n", argv[1] );
I am not sure anymore if the system libs (dlopen, etc.) are already loaded by the kernel, or if they are loaded on the first _dyld_func_lookup() call. Could you maybe check that? In the second case, changing the order means that there is a higher risk of running into some address space conflicts.
Thanks for reviewing!
I considered the question of whether looking up the functions earlier might load additional images, but it shouldn't. Those functions don't come from libSystem, they come from dyld itself (which is, of course, already loaded).
I just confirmed by forcing a crash just before dlopen-ing the main executable and looking at the macOS crash report. It shows the loaded images and it's just wine-preloader and dyld.
-Ken
Hello Ken,
since it seems pretty easy to check, I would also recommend to verify that dyld is indeed already loaded before calling any of the lookup functions. We only link against a small *.o file, so in theory, the kernel could still use lazy-loading for the dylib. If this shows that dyld is already loaded, then it indeed doesn't make any difference.
Dyld is definitely loaded before our code is run. The kernel loads the executable (but not its dependencies) and dyld and then starts the process in dyld, giving it a pointer to where the executable is. Without dyld already loaded, there'd be no way to load it or anything else (even lazy-loaded symbols). It's the loader, the thing that loads. ;) But, just to confirm, I forced the crash at the very beginning of wld_start() and dyld is loaded.
BTW: Thanks for your effort to get this upstream.
Happy to do it. And thank you and Michael for the original work. -Ken
participants (3)
-
Chip Davis -
Ken Thomases -
Sebastian Lackner