Re: [PATCH] loader: Check if the preloader overlaps the reserved range on the Mac.

[Sebastian Lackner]

Am Do., 6. Dez. 2018 um 17:09 Uhr schrieb Ken Thomases :
> Hi Sebastian,
>
> On Dec 6, 2018, at 9:53 AM, Sebastian Lackner  wrote:
> > Am Mi., 5. Dez. 2018 um 23:22 Uhr schrieb Ken Thomases :
> > > @@ -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